Here's a v4 that is actually more or less feature-complete:
it removes no-longer-needed complexity such as PG_TRY blocks.
I've checked that Valgrind shows no leaks in the postgres_fdw
and dblink tests after applying this on top of my other
patch series.
0001 is like the previous version except that I took out some
inessential simplifications to get to the minimum possible
patch. Then 0002 does all the simplifications. Removal of
PG_TRY blocks implies reindenting a lot of code, but I made
that a separate patch 0003 for ease of review. (0003 would
be a candidate for adding to .git-blame-ignore-revs, perhaps.)
0004 is the old 0002 (still unmodified) and then 0005 cleans
up one remaining leakage observed by Valgrind.
regards, tom lane
From 47d10046b2594358a1724a1c3f02562eb2d53e30 Mon Sep 17 00:00:00 2001
From: Tom Lane <t...@sss.pgh.pa.us>
Date: Mon, 26 May 2025 12:39:06 -0400
Subject: [PATCH v4 1/5] Fix memory leakage in postgres_fdw's DirectModify code
path.
postgres_fdw tries to use PG_TRY blocks to ensure that it will
eventually free the PGresult created by the remote modify command.
However, it's fundamentally impossible for this scheme to work
reliably when there's RETURNING data, because the query could fail
in between invocations of postgres_fdw's DirectModify methods.
There is at least one instance of exactly this situation in the
regression tests, and the ensuing session-lifespan leak is visible
under Valgrind.
We can improve matters by using a memory context reset callback
attached to the ExecutorState context. That ensures that the
PGresult will be freed when the ExecutorState context is torn
down, even if control never reaches postgresEndDirectModify.
This patch adds infrastructure that wraps each PGresult in a
"libpqsrv_PGresult" that provides the reset callback. Code using
this abstraction is inherently memory-safe (so long as it attaches
the reset callbacks to an appropriate memory context). Furthermore,
we add some macros that automatically redirect calls of the libpq
functions concerned with PGresults to use this infrastructure,
so that almost no source-code changes are needed to wheel this
infrastructure into place in all the backend code that uses libpq.
This patch just creates the infrastructure and makes relevant code
use it, which is enough to fix the DirectModify leak (and any others
that may exist). A good deal of follow-on simplification is possible
now that we don't have to be so cautious about freeing PGresults,
but I'll put that in a separate patch.
Author: Tom Lane <t...@sss.pgh.pa.us>
Discussion: https://postgr.es/m/2976982.1748049...@sss.pgh.pa.us
---
contrib/postgres_fdw/postgres_fdw.c | 7 +
contrib/postgres_fdw/postgres_fdw.h | 2 +-
src/backend/utils/mmgr/mcxt.c | 39 +++-
src/include/libpq/libpq-be-fe-helpers.h | 12 +-
src/include/libpq/libpq-be-fe.h | 259 ++++++++++++++++++++++++
src/include/utils/palloc.h | 2 +
src/tools/pgindent/typedefs.list | 1 +
7 files changed, 307 insertions(+), 15 deletions(-)
create mode 100644 src/include/libpq/libpq-be-fe.h
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 331f3fc088d..12bdecb32f4 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -4600,6 +4600,13 @@ execute_dml_stmt(ForeignScanState *node)
pgfdw_report_error(ERROR, dmstate->result, dmstate->conn, true,
dmstate->query);
+ /*
+ * The result potentially needs to survive across multiple executor row
+ * cycles, so move it to the context where the dmstate is.
+ */
+ dmstate->result = libpqsrv_PGresultSetParent(dmstate->result,
+ GetMemoryChunkContext(dmstate));
+
/* Get the number of rows affected. */
if (dmstate->has_returning)
dmstate->num_tuples = PQntuples(dmstate->result);
diff --git a/contrib/postgres_fdw/postgres_fdw.h b/contrib/postgres_fdw/postgres_fdw.h
index 81358f3bde7..9cb4ee84139 100644
--- a/contrib/postgres_fdw/postgres_fdw.h
+++ b/contrib/postgres_fdw/postgres_fdw.h
@@ -15,7 +15,7 @@
#include "foreign/foreign.h"
#include "lib/stringinfo.h"
-#include "libpq-fe.h"
+#include "libpq/libpq-be-fe.h"
#include "nodes/execnodes.h"
#include "nodes/pathnodes.h"
#include "utils/relcache.h"
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 15fa4d0a55e..ce01dce9861 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -560,9 +560,7 @@ MemoryContextDeleteChildren(MemoryContext context)
* the specified context, since that means it will automatically be freed
* when no longer needed.
*
- * There is no API for deregistering a callback once registered. If you
- * want it to not do anything anymore, adjust the state pointed to by its
- * "arg" to indicate that.
+ * Note that callers can assume this cannot fail.
*/
void
MemoryContextRegisterResetCallback(MemoryContext context,
@@ -577,6 +575,41 @@ MemoryContextRegisterResetCallback(MemoryContext context,
context->isReset = false;
}
+/*
+ * MemoryContextUnregisterResetCallback
+ * Undo the effects of MemoryContextRegisterResetCallback.
+ *
+ * This can be used if a callback's effects are no longer required
+ * at some point before the context has been reset/deleted. It is the
+ * caller's responsibility to pfree the callback struct (if needed).
+ *
+ * An assertion failure occurs if the callback was not registered.
+ * We could alternatively define that case as a no-op, but that seems too
+ * likely to mask programming errors such as passing the wrong context.
+ */
+void
+MemoryContextUnregisterResetCallback(MemoryContext context,
+ MemoryContextCallback *cb)
+{
+ MemoryContextCallback *prev,
+ *cur;
+
+ Assert(MemoryContextIsValid(context));
+
+ for (prev = NULL, cur = context->reset_cbs; cur != NULL;
+ prev = cur, cur = cur->next)
+ {
+ if (cur != cb)
+ continue;
+ if (prev)
+ prev->next = cur->next;
+ else
+ context->reset_cbs = cur->next;
+ return;
+ }
+ Assert(false);
+}
+
/*
* MemoryContextCallResetCallbacks
* Internal function to call all registered callbacks for context.
diff --git a/src/include/libpq/libpq-be-fe-helpers.h b/src/include/libpq/libpq-be-fe-helpers.h
index 16205b824fa..9a0373212da 100644
--- a/src/include/libpq/libpq-be-fe-helpers.h
+++ b/src/include/libpq/libpq-be-fe-helpers.h
@@ -30,17 +30,7 @@
#ifndef LIBPQ_BE_FE_HELPERS_H
#define LIBPQ_BE_FE_HELPERS_H
-/*
- * Despite the name, BUILDING_DLL is set only when building code directly part
- * of the backend. Which also is where libpq isn't allowed to be
- * used. Obviously this doesn't protect against libpq-fe.h getting included
- * otherwise, but perhaps still protects against a few mistakes...
- */
-#ifdef BUILDING_DLL
-#error "libpq may not be used code directly built into the backend"
-#endif
-
-#include "libpq-fe.h"
+#include "libpq/libpq-be-fe.h"
#include "miscadmin.h"
#include "storage/fd.h"
#include "storage/latch.h"
diff --git a/src/include/libpq/libpq-be-fe.h b/src/include/libpq/libpq-be-fe.h
new file mode 100644
index 00000000000..e3f796b0230
--- /dev/null
+++ b/src/include/libpq/libpq-be-fe.h
@@ -0,0 +1,259 @@
+/*-------------------------------------------------------------------------
+ *
+ * libpq-be-fe.h
+ * Wrapper functions for using libpq in extensions
+ *
+ * Code built directly into the backend is not allowed to link to libpq
+ * directly. Extension code is allowed to use libpq however. One of the
+ * main risks in doing so is leaking the malloc-allocated structures
+ * returned by libpq, causing a process-lifespan memory leak.
+ *
+ * This file provides wrapper objects to help in building memory-safe code.
+ * A PGresult object wrapped this way acts much as if it were palloc'd:
+ * it will go away when the specified context is reset or deleted.
+ * We might later extend the concept to other objects such as PGconns.
+ *
+ * See also the libpq-be-fe-helpers.h file, which provides additional
+ * facilities built on top of this one.
+ *
+ * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/libpq/libpq-be-fe.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef LIBPQ_BE_FE_H
+#define LIBPQ_BE_FE_H
+
+/*
+ * Despite the name, BUILDING_DLL is set only when building code directly part
+ * of the backend. Which also is where libpq isn't allowed to be
+ * used. Obviously this doesn't protect against libpq-fe.h getting included
+ * otherwise, but perhaps still protects against a few mistakes...
+ */
+#ifdef BUILDING_DLL
+#error "libpq may not be used in code directly built into the backend"
+#endif
+
+#include "libpq-fe.h"
+
+/*
+ * Memory-context-safe wrapper object for a PGresult.
+ */
+typedef struct libpqsrv_PGresult
+{
+ PGresult *res; /* the wrapped PGresult */
+ MemoryContext ctx; /* the MemoryContext it's attached to */
+ MemoryContextCallback cb; /* the callback that implements freeing */
+} libpqsrv_PGresult;
+
+
+/*
+ * Wrap the given PGresult in a libpqsrv_PGresult object, so that it will
+ * go away automatically if the current memory context is reset or deleted.
+ *
+ * To avoid potential memory leaks, backend code must always apply this
+ * immediately to the output of any PGresult-yielding libpq function.
+ */
+static inline libpqsrv_PGresult *
+libpqsrv_PQwrap(PGresult *res)
+{
+ libpqsrv_PGresult *bres;
+ MemoryContext ctx = CurrentMemoryContext;
+
+ /* We pass through a NULL result as-is, since there's nothing to free */
+ if (res == NULL)
+ return NULL;
+ /* Attempt to allocate the wrapper ... this had better not throw error */
+ bres = (libpqsrv_PGresult *)
+ MemoryContextAllocExtended(ctx,
+ sizeof(libpqsrv_PGresult),
+ MCXT_ALLOC_NO_OOM);
+ /* If we failed to allocate a wrapper, free the PGresult before failing */
+ if (bres == NULL)
+ {
+ PQclear(res);
+ ereport(ERROR,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("out of memory")));
+ }
+ /* Okay, set up the wrapper */
+ bres->res = res;
+ bres->ctx = ctx;
+ bres->cb.func = (MemoryContextCallbackFunction) PQclear;
+ bres->cb.arg = res;
+ MemoryContextRegisterResetCallback(ctx, &bres->cb);
+ return bres;
+}
+
+/*
+ * Free a wrapped PGresult, after detaching it from the memory context.
+ * Like PQclear(), allow the argument to be NULL.
+ */
+static inline void
+libpqsrv_PQclear(libpqsrv_PGresult *bres)
+{
+ if (bres)
+ {
+ MemoryContextUnregisterResetCallback(bres->ctx, &bres->cb);
+ PQclear(bres->res);
+ pfree(bres);
+ }
+}
+
+/*
+ * Move a wrapped PGresult to have a different parent context.
+ */
+static inline libpqsrv_PGresult *
+libpqsrv_PGresultSetParent(libpqsrv_PGresult *bres, MemoryContext ctx)
+{
+ libpqsrv_PGresult *newres;
+
+ /* We pass through a NULL result as-is */
+ if (bres == NULL)
+ return NULL;
+ /* Make a new wrapper in the target context, raising error on OOM */
+ newres = (libpqsrv_PGresult *)
+ MemoryContextAlloc(ctx, sizeof(libpqsrv_PGresult));
+ /* Okay, set up the new wrapper */
+ newres->res = bres->res;
+ newres->ctx = ctx;
+ newres->cb.func = (MemoryContextCallbackFunction) PQclear;
+ newres->cb.arg = bres->res;
+ MemoryContextRegisterResetCallback(ctx, &newres->cb);
+ /* Disarm and delete the old wrapper */
+ MemoryContextUnregisterResetCallback(bres->ctx, &bres->cb);
+ pfree(bres);
+ return newres;
+}
+
+/*
+ * Convenience wrapper for PQgetResult.
+ *
+ * We could supply wrappers for other PGresult-returning functions too,
+ * but at present there's no need.
+ */
+static inline libpqsrv_PGresult *
+libpqsrv_PQgetResult(PGconn *conn)
+{
+ return libpqsrv_PQwrap(PQgetResult(conn));
+}
+
+/*
+ * Accessor functions for libpqsrv_PGresult. While it's not necessary to use
+ * these, they emulate the behavior of the underlying libpq functions when
+ * passed a NULL pointer. This is particularly important for PQresultStatus,
+ * which is often the first check on a result.
+ */
+
+static inline ExecStatusType
+libpqsrv_PQresultStatus(const libpqsrv_PGresult *res)
+{
+ if (!res)
+ return PGRES_FATAL_ERROR;
+ return PQresultStatus(res->res);
+}
+
+static inline const char *
+libpqsrv_PQresultErrorMessage(const libpqsrv_PGresult *res)
+{
+ if (!res)
+ return "";
+ return PQresultErrorMessage(res->res);
+}
+
+static inline char *
+libpqsrv_PQresultErrorField(const libpqsrv_PGresult *res, int fieldcode)
+{
+ if (!res)
+ return NULL;
+ return PQresultErrorField(res->res, fieldcode);
+}
+
+static inline char *
+libpqsrv_PQcmdStatus(const libpqsrv_PGresult *res)
+{
+ if (!res)
+ return NULL;
+ return PQcmdStatus(res->res);
+}
+
+static inline int
+libpqsrv_PQntuples(const libpqsrv_PGresult *res)
+{
+ if (!res)
+ return 0;
+ return PQntuples(res->res);
+}
+
+static inline int
+libpqsrv_PQnfields(const libpqsrv_PGresult *res)
+{
+ if (!res)
+ return 0;
+ return PQnfields(res->res);
+}
+
+static inline char *
+libpqsrv_PQgetvalue(const libpqsrv_PGresult *res, int tup_num, int field_num)
+{
+ if (!res)
+ return NULL;
+ return PQgetvalue(res->res, tup_num, field_num);
+}
+
+static inline int
+libpqsrv_PQgetlength(const libpqsrv_PGresult *res, int tup_num, int field_num)
+{
+ if (!res)
+ return 0;
+ return PQgetlength(res->res, tup_num, field_num);
+}
+
+static inline int
+libpqsrv_PQgetisnull(const libpqsrv_PGresult *res, int tup_num, int field_num)
+{
+ if (!res)
+ return 1; /* pretend it is null */
+ return PQgetisnull(res->res, tup_num, field_num);
+}
+
+static inline char *
+libpqsrv_PQfname(const libpqsrv_PGresult *res, int field_num)
+{
+ if (!res)
+ return NULL;
+ return PQfname(res->res, field_num);
+}
+
+static inline const char *
+libpqsrv_PQcmdTuples(const libpqsrv_PGresult *res)
+{
+ if (!res)
+ return "";
+ return PQcmdTuples(res->res);
+}
+
+/*
+ * Redefine these libpq entry point names concerned with PGresults so that
+ * they will operate on libpqsrv_PGresults instead. This avoids needing to
+ * convert a lot of pre-existing code, and reduces the notational differences
+ * between frontend and backend libpq-using code.
+ */
+#define PGresult libpqsrv_PGresult
+#define PQclear libpqsrv_PQclear
+#define PQgetResult libpqsrv_PQgetResult
+#define PQresultStatus libpqsrv_PQresultStatus
+#define PQresultErrorMessage libpqsrv_PQresultErrorMessage
+#define PQresultErrorField libpqsrv_PQresultErrorField
+#define PQcmdStatus libpqsrv_PQcmdStatus
+#define PQntuples libpqsrv_PQntuples
+#define PQnfields libpqsrv_PQnfields
+#define PQgetvalue libpqsrv_PQgetvalue
+#define PQgetlength libpqsrv_PQgetlength
+#define PQgetisnull libpqsrv_PQgetisnull
+#define PQfname libpqsrv_PQfname
+#define PQcmdTuples libpqsrv_PQcmdTuples
+
+#endif /* LIBPQ_BE_FE_H */
diff --git a/src/include/utils/palloc.h b/src/include/utils/palloc.h
index e1b42267b22..039b9cba61a 100644
--- a/src/include/utils/palloc.h
+++ b/src/include/utils/palloc.h
@@ -133,6 +133,8 @@ MemoryContextSwitchTo(MemoryContext context)
/* Registration of memory context reset/delete callbacks */
extern void MemoryContextRegisterResetCallback(MemoryContext context,
MemoryContextCallback *cb);
+extern void MemoryContextUnregisterResetCallback(MemoryContext context,
+ MemoryContextCallback *cb);
/*
* These are like standard strdup() except the copied string is
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index a8346cda633..0b1a8f71af6 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -3714,6 +3714,7 @@ lclTocEntry
leafSegmentInfo
leaf_item
libpq_source
+libpqsrv_PGresult
line_t
lineno_t
list_sort_comparator
--
2.43.5
From cfa77394a34dbd826f48d398c6e5c3ba0fdadef2 Mon Sep 17 00:00:00 2001
From: Tom Lane <t...@sss.pgh.pa.us>
Date: Mon, 26 May 2025 14:10:23 -0400
Subject: [PATCH v4 2/5] Reap the benefits of not having to avoid leaking
PGresults.
Remove a bunch of PG_TRY constructs, de-volatilize related
variables, remove some PQclear calls in error paths.
Aside from making the code simpler and shorter, this should
provide some marginal performance gains.
For ease of review, I did not re-indent code within the removed
PG_TRY constructs. That'll be done in a separate patch.
Author: Tom Lane <t...@sss.pgh.pa.us>
Discussion: https://postgr.es/m/2976982.1748049...@sss.pgh.pa.us
---
contrib/dblink/dblink.c | 96 +++-------
contrib/postgres_fdw/connection.c | 52 ++---
contrib/postgres_fdw/postgres_fdw.c | 179 ++++--------------
contrib/postgres_fdw/postgres_fdw.h | 2 +-
.../libpqwalreceiver/libpqwalreceiver.c | 31 +--
src/include/libpq/libpq-be-fe-helpers.h | 13 +-
6 files changed, 83 insertions(+), 290 deletions(-)
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index 98d4e3d7dac..e43e446e818 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -101,8 +101,8 @@ static void materializeQueryResult(FunctionCallInfo fcinfo,
const char *conname,
const char *sql,
bool fail);
-static PGresult *storeQueryResult(volatile storeInfo *sinfo, PGconn *conn, const char *sql);
-static void storeRow(volatile storeInfo *sinfo, PGresult *res, bool first);
+static PGresult *storeQueryResult(storeInfo *sinfo, PGconn *conn, const char *sql);
+static void storeRow(storeInfo *sinfo, PGresult *res, bool first);
static remoteConn *getConnectionByName(const char *name);
static HTAB *createConnHash(void);
static void createNewConnection(const char *name, remoteConn *rconn);
@@ -162,14 +162,6 @@ typedef struct remoteConnHashEnt
/* initial number of connection hashes */
#define NUMCONN 16
-static char *
-xpstrdup(const char *in)
-{
- if (in == NULL)
- return NULL;
- return pstrdup(in);
-}
-
pg_noreturn static void
dblink_res_internalerror(PGconn *conn, PGresult *res, const char *p2)
{
@@ -861,17 +853,14 @@ static void
materializeResult(FunctionCallInfo fcinfo, PGconn *conn, PGresult *res)
{
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+ TupleDesc tupdesc;
+ bool is_sql_cmd;
+ int ntuples;
+ int nfields;
/* prepTuplestoreResult must have been called previously */
Assert(rsinfo->returnMode == SFRM_Materialize);
- PG_TRY();
- {
- TupleDesc tupdesc;
- bool is_sql_cmd;
- int ntuples;
- int nfields;
-
if (PQresultStatus(res) == PGRES_COMMAND_OK)
{
is_sql_cmd = true;
@@ -979,13 +968,8 @@ materializeResult(FunctionCallInfo fcinfo, PGconn *conn, PGresult *res)
/* clean up GUC settings, if we changed any */
restoreLocalGucs(nestlevel);
}
- }
- PG_FINALLY();
- {
- /* be sure to release the libpq result */
+
PQclear(res);
- }
- PG_END_TRY();
}
/*
@@ -1004,16 +988,17 @@ materializeQueryResult(FunctionCallInfo fcinfo,
bool fail)
{
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
- PGresult *volatile res = NULL;
- volatile storeInfo sinfo = {0};
/* prepTuplestoreResult must have been called previously */
Assert(rsinfo->returnMode == SFRM_Materialize);
- sinfo.fcinfo = fcinfo;
-
+ /* Use a PG_TRY block to ensure we pump libpq dry of results */
PG_TRY();
{
+ storeInfo sinfo = {0};
+ PGresult *res;
+
+ sinfo.fcinfo = fcinfo;
/* Create short-lived memory context for data conversions */
sinfo.tmpcontext = AllocSetContextCreate(CurrentMemoryContext,
"dblink temporary context",
@@ -1026,14 +1011,7 @@ materializeQueryResult(FunctionCallInfo fcinfo,
(PQresultStatus(res) != PGRES_COMMAND_OK &&
PQresultStatus(res) != PGRES_TUPLES_OK))
{
- /*
- * dblink_res_error will clear the passed PGresult, so we need
- * this ugly dance to avoid doing so twice during error exit
- */
- PGresult *res1 = res;
-
- res = NULL;
- dblink_res_error(conn, conname, res1, fail,
+ dblink_res_error(conn, conname, res, fail,
"while executing query");
/* if fail isn't set, we'll return an empty query result */
}
@@ -1072,7 +1050,6 @@ materializeQueryResult(FunctionCallInfo fcinfo,
tuplestore_puttuple(tupstore, tuple);
PQclear(res);
- res = NULL;
}
else
{
@@ -1081,26 +1058,20 @@ materializeQueryResult(FunctionCallInfo fcinfo,
Assert(rsinfo->setResult != NULL);
PQclear(res);
- res = NULL;
}
/* clean up data conversion short-lived memory context */
if (sinfo.tmpcontext != NULL)
MemoryContextDelete(sinfo.tmpcontext);
- sinfo.tmpcontext = NULL;
PQclear(sinfo.last_res);
- sinfo.last_res = NULL;
PQclear(sinfo.cur_res);
- sinfo.cur_res = NULL;
}
PG_CATCH();
{
- /* be sure to release any libpq result we collected */
- PQclear(res);
- PQclear(sinfo.last_res);
- PQclear(sinfo.cur_res);
- /* and clear out any pending data in libpq */
+ PGresult *res;
+
+ /* be sure to clear out any pending data in libpq */
while ((res = libpqsrv_get_result(conn, dblink_we_get_result)) !=
NULL)
PQclear(res);
@@ -1113,7 +1084,7 @@ materializeQueryResult(FunctionCallInfo fcinfo,
* Execute query, and send any result rows to sinfo->tuplestore.
*/
static PGresult *
-storeQueryResult(volatile storeInfo *sinfo, PGconn *conn, const char *sql)
+storeQueryResult(storeInfo *sinfo, PGconn *conn, const char *sql)
{
bool first = true;
int nestlevel = -1;
@@ -1181,7 +1152,7 @@ storeQueryResult(volatile storeInfo *sinfo, PGconn *conn, const char *sql)
* (in this case the PGresult might contain either zero or one row).
*/
static void
-storeRow(volatile storeInfo *sinfo, PGresult *res, bool first)
+storeRow(storeInfo *sinfo, PGresult *res, bool first)
{
int nfields = PQnfields(res);
HeapTuple tuple;
@@ -2782,10 +2753,13 @@ dblink_connstr_check(const char *connstr)
/*
* Report an error received from the remote server
*
- * res: the received error result (will be freed)
+ * res: the received error result
* fail: true for ERROR ereport, false for NOTICE
* fmt and following args: sprintf-style format and values for errcontext;
* the resulting string should be worded like "while <some action>"
+ *
+ * If "res" is not NULL, it'll be PQclear'ed here (unless we throw error,
+ * in which case memory context cleanup will clear it eventually).
*/
static void
dblink_res_error(PGconn *conn, const char *conname, PGresult *res,
@@ -2793,15 +2767,11 @@ dblink_res_error(PGconn *conn, const char *conname, PGresult *res,
{
int level;
char *pg_diag_sqlstate = PQresultErrorField(res, PG_DIAG_SQLSTATE);
- char *pg_diag_message_primary = PQresultErrorField(res, PG_DIAG_MESSAGE_PRIMARY);
- char *pg_diag_message_detail = PQresultErrorField(res, PG_DIAG_MESSAGE_DETAIL);
- char *pg_diag_message_hint = PQresultErrorField(res, PG_DIAG_MESSAGE_HINT);
- char *pg_diag_context = PQresultErrorField(res, PG_DIAG_CONTEXT);
+ char *message_primary = PQresultErrorField(res, PG_DIAG_MESSAGE_PRIMARY);
+ char *message_detail = PQresultErrorField(res, PG_DIAG_MESSAGE_DETAIL);
+ char *message_hint = PQresultErrorField(res, PG_DIAG_MESSAGE_HINT);
+ char *message_context = PQresultErrorField(res, PG_DIAG_CONTEXT);
int sqlstate;
- char *message_primary;
- char *message_detail;
- char *message_hint;
- char *message_context;
va_list ap;
char dblink_context_msg[512];
@@ -2819,11 +2789,6 @@ dblink_res_error(PGconn *conn, const char *conname, PGresult *res,
else
sqlstate = ERRCODE_CONNECTION_FAILURE;
- message_primary = xpstrdup(pg_diag_message_primary);
- message_detail = xpstrdup(pg_diag_message_detail);
- message_hint = xpstrdup(pg_diag_message_hint);
- message_context = xpstrdup(pg_diag_context);
-
/*
* If we don't get a message from the PGresult, try the PGconn. This is
* needed because for connection-level failures, PQgetResult may just
@@ -2832,14 +2797,6 @@ dblink_res_error(PGconn *conn, const char *conname, PGresult *res,
if (message_primary == NULL)
message_primary = pchomp(PQerrorMessage(conn));
- /*
- * Now that we've copied all the data we need out of the PGresult, it's
- * safe to free it. We must do this to avoid PGresult leakage. We're
- * leaking all the strings too, but those are in palloc'd memory that will
- * get cleaned up eventually.
- */
- PQclear(res);
-
/*
* Format the basic errcontext string. Below, we'll add on something
* about the connection name. That's a violation of the translatability
@@ -2864,6 +2821,7 @@ dblink_res_error(PGconn *conn, const char *conname, PGresult *res,
dblink_context_msg, conname)) :
(errcontext("%s on unnamed dblink connection",
dblink_context_msg))));
+ PQclear(res);
}
/*
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index 304f3c20f83..2082c0b2ff8 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -812,7 +812,7 @@ static void
do_sql_command_begin(PGconn *conn, const char *sql)
{
if (!PQsendQuery(conn, sql))
- pgfdw_report_error(ERROR, NULL, conn, false, sql);
+ pgfdw_report_error(ERROR, NULL, conn, sql);
}
static void
@@ -827,10 +827,10 @@ do_sql_command_end(PGconn *conn, const char *sql, bool consume_input)
* would be large compared to the overhead of PQconsumeInput.)
*/
if (consume_input && !PQconsumeInput(conn))
- pgfdw_report_error(ERROR, NULL, conn, false, sql);
+ pgfdw_report_error(ERROR, NULL, conn, sql);
res = pgfdw_get_result(conn);
if (PQresultStatus(res) != PGRES_COMMAND_OK)
- pgfdw_report_error(ERROR, res, conn, true, sql);
+ pgfdw_report_error(ERROR, res, conn, sql);
PQclear(res);
}
@@ -964,22 +964,21 @@ pgfdw_get_result(PGconn *conn)
* Report an error we got from the remote server.
*
* elevel: error level to use (typically ERROR, but might be less)
- * res: PGresult containing the error
+ * res: PGresult containing the error (might be NULL)
* conn: connection we did the query on
- * clear: if true, PQclear the result (otherwise caller will handle it)
* sql: NULL, or text of remote command we tried to execute
*
+ * If "res" is not NULL, it'll be PQclear'ed here (unless we throw error,
+ * in which case memory context cleanup will clear it eventually).
+ *
* Note: callers that choose not to throw ERROR for a remote error are
* responsible for making sure that the associated ConnCacheEntry gets
* marked with have_error = true.
*/
void
pgfdw_report_error(int elevel, PGresult *res, PGconn *conn,
- bool clear, const char *sql)
+ const char *sql)
{
- /* If requested, PGresult must be released before leaving this function. */
- PG_TRY();
- {
char *diag_sqlstate = PQresultErrorField(res, PG_DIAG_SQLSTATE);
char *message_primary = PQresultErrorField(res, PG_DIAG_MESSAGE_PRIMARY);
char *message_detail = PQresultErrorField(res, PG_DIAG_MESSAGE_DETAIL);
@@ -1013,13 +1012,7 @@ pgfdw_report_error(int elevel, PGresult *res, PGconn *conn,
message_hint ? errhint("%s", message_hint) : 0,
message_context ? errcontext("%s", message_context) : 0,
sql ? errcontext("remote SQL command: %s", sql) : 0));
- }
- PG_FINALLY();
- {
- if (clear)
PQclear(res);
- }
- PG_END_TRY();
}
/*
@@ -1542,7 +1535,7 @@ pgfdw_exec_cleanup_query_begin(PGconn *conn, const char *query)
*/
if (!PQsendQuery(conn, query))
{
- pgfdw_report_error(WARNING, NULL, conn, false, query);
+ pgfdw_report_error(WARNING, NULL, conn, query);
return false;
}
@@ -1567,7 +1560,7 @@ pgfdw_exec_cleanup_query_end(PGconn *conn, const char *query,
*/
if (consume_input && !PQconsumeInput(conn))
{
- pgfdw_report_error(WARNING, NULL, conn, false, query);
+ pgfdw_report_error(WARNING, NULL, conn, query);
return false;
}
@@ -1579,7 +1572,7 @@ pgfdw_exec_cleanup_query_end(PGconn *conn, const char *query,
(errmsg("could not get query result due to timeout"),
errcontext("remote SQL command: %s", query)));
else
- pgfdw_report_error(WARNING, NULL, conn, false, query);
+ pgfdw_report_error(WARNING, NULL, conn, query);
return false;
}
@@ -1587,7 +1580,7 @@ pgfdw_exec_cleanup_query_end(PGconn *conn, const char *query,
/* Issue a warning if not successful. */
if (PQresultStatus(result) != PGRES_COMMAND_OK)
{
- pgfdw_report_error(WARNING, result, conn, true, query);
+ pgfdw_report_error(WARNING, result, conn, query);
return ignore_errors;
}
PQclear(result);
@@ -1615,17 +1608,12 @@ pgfdw_get_cleanup_result(PGconn *conn, TimestampTz endtime,
PGresult **result,
bool *timed_out)
{
- volatile bool failed = false;
- PGresult *volatile last_res = NULL;
+ bool failed = false;
+ PGresult *last_res = NULL;
+ int canceldelta = RETRY_CANCEL_TIMEOUT * 2;
*result = NULL;
*timed_out = false;
-
- /* In what follows, do not leak any PGresults on an error. */
- PG_TRY();
- {
- int canceldelta = RETRY_CANCEL_TIMEOUT * 2;
-
for (;;)
{
PGresult *res;
@@ -1703,15 +1691,7 @@ pgfdw_get_cleanup_result(PGconn *conn, TimestampTz endtime,
PQclear(last_res);
last_res = res;
}
-exit: ;
- }
- PG_CATCH();
- {
- PQclear(last_res);
- PG_RE_THROW();
- }
- PG_END_TRY();
-
+exit:
if (failed)
PQclear(last_res);
else
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 12bdecb32f4..610ea779a87 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -1702,13 +1702,9 @@ postgresReScanForeignScan(ForeignScanState *node)
return;
}
- /*
- * We don't use a PG_TRY block here, so be careful not to throw error
- * without releasing the PGresult.
- */
res = pgfdw_exec_query(fsstate->conn, sql, fsstate->conn_state);
if (PQresultStatus(res) != PGRES_COMMAND_OK)
- pgfdw_report_error(ERROR, res, fsstate->conn, true, sql);
+ pgfdw_report_error(ERROR, res, fsstate->conn, sql);
PQclear(res);
/* Now force a fresh FETCH. */
@@ -3601,11 +3597,7 @@ get_remote_estimate(const char *sql, PGconn *conn,
double *rows, int *width,
Cost *startup_cost, Cost *total_cost)
{
- PGresult *volatile res = NULL;
-
- /* PGresult must be released before leaving this function. */
- PG_TRY();
- {
+ PGresult *res;
char *line;
char *p;
int n;
@@ -3615,7 +3607,7 @@ get_remote_estimate(const char *sql, PGconn *conn,
*/
res = pgfdw_exec_query(conn, sql, NULL);
if (PQresultStatus(res) != PGRES_TUPLES_OK)
- pgfdw_report_error(ERROR, res, conn, false, sql);
+ pgfdw_report_error(ERROR, res, conn, sql);
/*
* Extract cost numbers for topmost plan node. Note we search for a
@@ -3630,12 +3622,7 @@ get_remote_estimate(const char *sql, PGconn *conn,
startup_cost, total_cost, rows, width);
if (n != 4)
elog(ERROR, "could not interpret EXPLAIN output: \"%s\"", line);
- }
- PG_FINALLY();
- {
PQclear(res);
- }
- PG_END_TRY();
}
/*
@@ -3775,17 +3762,14 @@ create_cursor(ForeignScanState *node)
*/
if (!PQsendQueryParams(conn, buf.data, numParams,
NULL, values, NULL, NULL, 0))
- pgfdw_report_error(ERROR, NULL, conn, false, buf.data);
+ pgfdw_report_error(ERROR, NULL, conn, buf.data);
/*
* Get the result, and check for success.
- *
- * We don't use a PG_TRY block here, so be careful not to throw error
- * without releasing the PGresult.
*/
res = pgfdw_get_result(conn);
if (PQresultStatus(res) != PGRES_COMMAND_OK)
- pgfdw_report_error(ERROR, res, conn, true, fsstate->query);
+ pgfdw_report_error(ERROR, res, conn, fsstate->query);
PQclear(res);
/* Mark the cursor as created, and show no tuples have been retrieved */
@@ -3807,7 +3791,10 @@ static void
fetch_more_data(ForeignScanState *node)
{
PgFdwScanState *fsstate = (PgFdwScanState *) node->fdw_state;
- PGresult *volatile res = NULL;
+ PGconn *conn = fsstate->conn;
+ PGresult *res;
+ int numrows;
+ int i;
MemoryContext oldcontext;
/*
@@ -3818,13 +3805,6 @@ fetch_more_data(ForeignScanState *node)
MemoryContextReset(fsstate->batch_cxt);
oldcontext = MemoryContextSwitchTo(fsstate->batch_cxt);
- /* PGresult must be released before leaving this function. */
- PG_TRY();
- {
- PGconn *conn = fsstate->conn;
- int numrows;
- int i;
-
if (fsstate->async_capable)
{
Assert(fsstate->conn_state->pendingAreq);
@@ -3836,7 +3816,7 @@ fetch_more_data(ForeignScanState *node)
res = pgfdw_get_result(conn);
/* On error, report the original query, not the FETCH. */
if (PQresultStatus(res) != PGRES_TUPLES_OK)
- pgfdw_report_error(ERROR, res, conn, false, fsstate->query);
+ pgfdw_report_error(ERROR, res, conn, fsstate->query);
/* Reset per-connection state */
fsstate->conn_state->pendingAreq = NULL;
@@ -3852,7 +3832,7 @@ fetch_more_data(ForeignScanState *node)
res = pgfdw_exec_query(conn, sql, fsstate->conn_state);
/* On error, report the original query, not the FETCH. */
if (PQresultStatus(res) != PGRES_TUPLES_OK)
- pgfdw_report_error(ERROR, res, conn, false, fsstate->query);
+ pgfdw_report_error(ERROR, res, conn, fsstate->query);
}
/* Convert the data into HeapTuples */
@@ -3880,12 +3860,8 @@ fetch_more_data(ForeignScanState *node)
/* Must be EOF if we didn't get as many tuples as we asked for. */
fsstate->eof_reached = (numrows < fsstate->fetch_size);
- }
- PG_FINALLY();
- {
+
PQclear(res);
- }
- PG_END_TRY();
MemoryContextSwitchTo(oldcontext);
}
@@ -3959,14 +3935,9 @@ close_cursor(PGconn *conn, unsigned int cursor_number,
PGresult *res;
snprintf(sql, sizeof(sql), "CLOSE c%u", cursor_number);
-
- /*
- * We don't use a PG_TRY block here, so be careful not to throw error
- * without releasing the PGresult.
- */
res = pgfdw_exec_query(conn, sql, conn_state);
if (PQresultStatus(res) != PGRES_COMMAND_OK)
- pgfdw_report_error(ERROR, res, conn, true, sql);
+ pgfdw_report_error(ERROR, res, conn, sql);
PQclear(res);
}
@@ -4174,18 +4145,15 @@ execute_foreign_modify(EState *estate,
NULL,
NULL,
0))
- pgfdw_report_error(ERROR, NULL, fmstate->conn, false, fmstate->query);
+ pgfdw_report_error(ERROR, NULL, fmstate->conn, fmstate->query);
/*
* Get the result, and check for success.
- *
- * We don't use a PG_TRY block here, so be careful not to throw error
- * without releasing the PGresult.
*/
res = pgfdw_get_result(fmstate->conn);
if (PQresultStatus(res) !=
(fmstate->has_returning ? PGRES_TUPLES_OK : PGRES_COMMAND_OK))
- pgfdw_report_error(ERROR, res, fmstate->conn, true, fmstate->query);
+ pgfdw_report_error(ERROR, res, fmstate->conn, fmstate->query);
/* Check number of rows affected, and fetch RETURNING tuple if any */
if (fmstate->has_returning)
@@ -4244,17 +4212,14 @@ prepare_foreign_modify(PgFdwModifyState *fmstate)
fmstate->query,
0,
NULL))
- pgfdw_report_error(ERROR, NULL, fmstate->conn, false, fmstate->query);
+ pgfdw_report_error(ERROR, NULL, fmstate->conn, fmstate->query);
/*
* Get the result, and check for success.
- *
- * We don't use a PG_TRY block here, so be careful not to throw error
- * without releasing the PGresult.
*/
res = pgfdw_get_result(fmstate->conn);
if (PQresultStatus(res) != PGRES_COMMAND_OK)
- pgfdw_report_error(ERROR, res, fmstate->conn, true, fmstate->query);
+ pgfdw_report_error(ERROR, res, fmstate->conn, fmstate->query);
PQclear(res);
/* This action shows that the prepare has been done. */
@@ -4345,16 +4310,11 @@ convert_prep_stmt_params(PgFdwModifyState *fmstate,
/*
* store_returning_result
* Store the result of a RETURNING clause
- *
- * On error, be sure to release the PGresult on the way out. Callers do not
- * have PG_TRY blocks to ensure this happens.
*/
static void
store_returning_result(PgFdwModifyState *fmstate,
TupleTableSlot *slot, PGresult *res)
{
- PG_TRY();
- {
HeapTuple newtup;
newtup = make_tuple_from_result_row(res, 0,
@@ -4369,13 +4329,6 @@ store_returning_result(PgFdwModifyState *fmstate,
* heaptuples directly, so allow for conversion.
*/
ExecForceStoreHeapTuple(newtup, slot, true);
- }
- PG_CATCH();
- {
- PQclear(res);
- PG_RE_THROW();
- }
- PG_END_TRY();
}
/*
@@ -4411,14 +4364,9 @@ deallocate_query(PgFdwModifyState *fmstate)
return;
snprintf(sql, sizeof(sql), "DEALLOCATE %s", fmstate->p_name);
-
- /*
- * We don't use a PG_TRY block here, so be careful not to throw error
- * without releasing the PGresult.
- */
res = pgfdw_exec_query(fmstate->conn, sql, fmstate->conn_state);
if (PQresultStatus(res) != PGRES_COMMAND_OK)
- pgfdw_report_error(ERROR, res, fmstate->conn, true, sql);
+ pgfdw_report_error(ERROR, res, fmstate->conn, sql);
PQclear(res);
pfree(fmstate->p_name);
fmstate->p_name = NULL;
@@ -4586,18 +4534,15 @@ execute_dml_stmt(ForeignScanState *node)
*/
if (!PQsendQueryParams(dmstate->conn, dmstate->query, numParams,
NULL, values, NULL, NULL, 0))
- pgfdw_report_error(ERROR, NULL, dmstate->conn, false, dmstate->query);
+ pgfdw_report_error(ERROR, NULL, dmstate->conn, dmstate->query);
/*
* Get the result, and check for success.
- *
- * We don't use a PG_TRY block here, so be careful not to throw error
- * without releasing the PGresult.
*/
dmstate->result = pgfdw_get_result(dmstate->conn);
if (PQresultStatus(dmstate->result) !=
(dmstate->has_returning ? PGRES_TUPLES_OK : PGRES_COMMAND_OK))
- pgfdw_report_error(ERROR, dmstate->result, dmstate->conn, true,
+ pgfdw_report_error(ERROR, dmstate->result, dmstate->conn,
dmstate->query);
/*
@@ -4648,12 +4593,6 @@ get_returning_data(ForeignScanState *node)
}
else
{
- /*
- * On error, be sure to release the PGresult on the way out. Callers
- * do not have PG_TRY blocks to ensure this happens.
- */
- PG_TRY();
- {
HeapTuple newtup;
newtup = make_tuple_from_result_row(dmstate->result,
@@ -4664,13 +4603,6 @@ get_returning_data(ForeignScanState *node)
node,
dmstate->temp_cxt);
ExecStoreHeapTuple(newtup, slot, false);
- }
- PG_CATCH();
- {
- PQclear(dmstate->result);
- PG_RE_THROW();
- }
- PG_END_TRY();
/* Get the updated/deleted tuple. */
if (dmstate->rel)
@@ -4957,7 +4889,7 @@ postgresAnalyzeForeignTable(Relation relation,
UserMapping *user;
PGconn *conn;
StringInfoData sql;
- PGresult *volatile res = NULL;
+ PGresult *res;
/* Return the row-analysis function pointer */
*func = postgresAcquireSampleRowsFunc;
@@ -4983,22 +4915,14 @@ postgresAnalyzeForeignTable(Relation relation,
initStringInfo(&sql);
deparseAnalyzeSizeSql(&sql, relation);
- /* In what follows, do not risk leaking any PGresults. */
- PG_TRY();
- {
res = pgfdw_exec_query(conn, sql.data, NULL);
if (PQresultStatus(res) != PGRES_TUPLES_OK)
- pgfdw_report_error(ERROR, res, conn, false, sql.data);
+ pgfdw_report_error(ERROR, res, conn, sql.data);
if (PQntuples(res) != 1 || PQnfields(res) != 1)
elog(ERROR, "unexpected result from deparseAnalyzeSizeSql query");
*totalpages = strtoul(PQgetvalue(res, 0, 0), NULL, 10);
- }
- PG_FINALLY();
- {
PQclear(res);
- }
- PG_END_TRY();
ReleaseConnection(conn);
@@ -5019,9 +4943,9 @@ postgresGetAnalyzeInfoForForeignTable(Relation relation, bool *can_tablesample)
UserMapping *user;
PGconn *conn;
StringInfoData sql;
- PGresult *volatile res = NULL;
- volatile double reltuples = -1;
- volatile char relkind = 0;
+ PGresult *res;
+ double reltuples;
+ char relkind;
/* assume the remote relation does not support TABLESAMPLE */
*can_tablesample = false;
@@ -5040,24 +4964,15 @@ postgresGetAnalyzeInfoForForeignTable(Relation relation, bool *can_tablesample)
initStringInfo(&sql);
deparseAnalyzeInfoSql(&sql, relation);
- /* In what follows, do not risk leaking any PGresults. */
- PG_TRY();
- {
res = pgfdw_exec_query(conn, sql.data, NULL);
if (PQresultStatus(res) != PGRES_TUPLES_OK)
- pgfdw_report_error(ERROR, res, conn, false, sql.data);
+ pgfdw_report_error(ERROR, res, conn, sql.data);
if (PQntuples(res) != 1 || PQnfields(res) != 2)
elog(ERROR, "unexpected result from deparseAnalyzeInfoSql query");
reltuples = strtod(PQgetvalue(res, 0, 0), NULL);
relkind = *(PQgetvalue(res, 0, 1));
- }
- PG_FINALLY();
- {
- if (res)
PQclear(res);
- }
- PG_END_TRY();
ReleaseConnection(conn);
@@ -5100,7 +5015,9 @@ postgresAcquireSampleRowsFunc(Relation relation, int elevel,
double reltuples;
unsigned int cursor_number;
StringInfoData sql;
- PGresult *volatile res = NULL;
+ PGresult *res;
+ char fetch_sql[64];
+ int fetch_size;
ListCell *lc;
/* Initialize workspace state */
@@ -5277,17 +5194,10 @@ postgresAcquireSampleRowsFunc(Relation relation, int elevel,
deparseAnalyzeSql(&sql, relation, method, sample_frac, &astate.retrieved_attrs);
- /* In what follows, do not risk leaking any PGresults. */
- PG_TRY();
- {
- char fetch_sql[64];
- int fetch_size;
-
res = pgfdw_exec_query(conn, sql.data, NULL);
if (PQresultStatus(res) != PGRES_COMMAND_OK)
- pgfdw_report_error(ERROR, res, conn, false, sql.data);
+ pgfdw_report_error(ERROR, res, conn, sql.data);
PQclear(res);
- res = NULL;
/*
* Determine the fetch size. The default is arbitrary, but shouldn't
@@ -5338,7 +5248,7 @@ postgresAcquireSampleRowsFunc(Relation relation, int elevel,
res = pgfdw_exec_query(conn, fetch_sql, NULL);
/* On error, report the original query, not the FETCH. */
if (PQresultStatus(res) != PGRES_TUPLES_OK)
- pgfdw_report_error(ERROR, res, conn, false, sql.data);
+ pgfdw_report_error(ERROR, res, conn, sql.data);
/* Process whatever we got. */
numrows = PQntuples(res);
@@ -5346,7 +5256,6 @@ postgresAcquireSampleRowsFunc(Relation relation, int elevel,
analyze_row_processor(res, i, &astate);
PQclear(res);
- res = NULL;
/* Must be EOF if we didn't get all the rows requested. */
if (numrows < fetch_size)
@@ -5355,13 +5264,6 @@ postgresAcquireSampleRowsFunc(Relation relation, int elevel,
/* Close the cursor, just to be tidy. */
close_cursor(conn, cursor_number, NULL);
- }
- PG_CATCH();
- {
- PQclear(res);
- PG_RE_THROW();
- }
- PG_END_TRY();
ReleaseConnection(conn);
@@ -5473,7 +5375,7 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
UserMapping *mapping;
PGconn *conn;
StringInfoData buf;
- PGresult *volatile res = NULL;
+ PGresult *res;
int numrows,
i;
ListCell *lc;
@@ -5512,16 +5414,13 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
/* Create workspace for strings */
initStringInfo(&buf);
- /* In what follows, do not risk leaking any PGresults. */
- PG_TRY();
- {
/* Check that the schema really exists */
appendStringInfoString(&buf, "SELECT 1 FROM pg_catalog.pg_namespace WHERE nspname = ");
deparseStringLiteral(&buf, stmt->remote_schema);
res = pgfdw_exec_query(conn, buf.data, NULL);
if (PQresultStatus(res) != PGRES_TUPLES_OK)
- pgfdw_report_error(ERROR, res, conn, false, buf.data);
+ pgfdw_report_error(ERROR, res, conn, buf.data);
if (PQntuples(res) != 1)
ereport(ERROR,
@@ -5530,7 +5429,6 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
stmt->remote_schema, server->servername)));
PQclear(res);
- res = NULL;
resetStringInfo(&buf);
/*
@@ -5638,7 +5536,7 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
/* Fetch the data */
res = pgfdw_exec_query(conn, buf.data, NULL);
if (PQresultStatus(res) != PGRES_TUPLES_OK)
- pgfdw_report_error(ERROR, res, conn, false, buf.data);
+ pgfdw_report_error(ERROR, res, conn, buf.data);
/* Process results */
numrows = PQntuples(res);
@@ -5743,12 +5641,7 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
commands = lappend(commands, pstrdup(buf.data));
}
- }
- PG_FINALLY();
- {
PQclear(res);
- }
- PG_END_TRY();
ReleaseConnection(conn);
@@ -7416,7 +7309,7 @@ postgresForeignAsyncNotify(AsyncRequest *areq)
/* On error, report the original query, not the FETCH. */
if (!PQconsumeInput(fsstate->conn))
- pgfdw_report_error(ERROR, NULL, fsstate->conn, false, fsstate->query);
+ pgfdw_report_error(ERROR, NULL, fsstate->conn, fsstate->query);
fetch_more_data(node);
@@ -7515,7 +7408,7 @@ fetch_more_data_begin(AsyncRequest *areq)
fsstate->fetch_size, fsstate->cursor_number);
if (!PQsendQuery(fsstate->conn, sql))
- pgfdw_report_error(ERROR, NULL, fsstate->conn, false, fsstate->query);
+ pgfdw_report_error(ERROR, NULL, fsstate->conn, fsstate->query);
/* Remember that the request is in process */
fsstate->conn_state->pendingAreq = areq;
diff --git a/contrib/postgres_fdw/postgres_fdw.h b/contrib/postgres_fdw/postgres_fdw.h
index 9cb4ee84139..38e1a885941 100644
--- a/contrib/postgres_fdw/postgres_fdw.h
+++ b/contrib/postgres_fdw/postgres_fdw.h
@@ -167,7 +167,7 @@ extern PGresult *pgfdw_get_result(PGconn *conn);
extern PGresult *pgfdw_exec_query(PGconn *conn, const char *query,
PgFdwConnState *state);
extern void pgfdw_report_error(int elevel, PGresult *res, PGconn *conn,
- bool clear, const char *sql);
+ const char *sql);
/* in option.c */
extern int ExtractConnectionOptions(List *defelems,
diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index 7b4ddf7a8f5..d615f340ac7 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -418,31 +418,22 @@ libpqrcv_identify_system(WalReceiverConn *conn, TimeLineID *primary_tli)
"IDENTIFY_SYSTEM",
WAIT_EVENT_LIBPQWALRECEIVER_RECEIVE);
if (PQresultStatus(res) != PGRES_TUPLES_OK)
- {
- PQclear(res);
ereport(ERROR,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("could not receive database system identifier and timeline ID from "
"the primary server: %s",
pchomp(PQerrorMessage(conn->streamConn)))));
- }
/*
* IDENTIFY_SYSTEM returns 3 columns in 9.3 and earlier, and 4 columns in
* 9.4 and onwards.
*/
if (PQnfields(res) < 3 || PQntuples(res) != 1)
- {
- int ntuples = PQntuples(res);
- int nfields = PQnfields(res);
-
- PQclear(res);
ereport(ERROR,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("invalid response from primary server"),
errdetail("Could not identify system: got %d rows and %d fields, expected %d rows and %d or more fields.",
- ntuples, nfields, 1, 3)));
- }
+ PQntuples(res), PQnfields(res), 1, 3)));
primary_sysid = pstrdup(PQgetvalue(res, 0, 0));
*primary_tli = pg_strtoint32(PQgetvalue(res, 0, 1));
PQclear(res);
@@ -604,13 +595,10 @@ libpqrcv_startstreaming(WalReceiverConn *conn,
return false;
}
else if (PQresultStatus(res) != PGRES_COPY_BOTH)
- {
- PQclear(res);
ereport(ERROR,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("could not start WAL streaming: %s",
pchomp(PQerrorMessage(conn->streamConn)))));
- }
PQclear(res);
return true;
}
@@ -718,26 +706,17 @@ libpqrcv_readtimelinehistoryfile(WalReceiverConn *conn,
cmd,
WAIT_EVENT_LIBPQWALRECEIVER_RECEIVE);
if (PQresultStatus(res) != PGRES_TUPLES_OK)
- {
- PQclear(res);
ereport(ERROR,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("could not receive timeline history file from "
"the primary server: %s",
pchomp(PQerrorMessage(conn->streamConn)))));
- }
if (PQnfields(res) != 2 || PQntuples(res) != 1)
- {
- int ntuples = PQntuples(res);
- int nfields = PQnfields(res);
-
- PQclear(res);
ereport(ERROR,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("invalid response from primary server"),
errdetail("Expected 1 tuple with 2 fields, got %d tuples with %d fields.",
- ntuples, nfields)));
- }
+ PQntuples(res), PQnfields(res))));
*filename = pstrdup(PQgetvalue(res, 0, 0));
*len = PQgetlength(res, 0, 1);
@@ -841,13 +820,10 @@ libpqrcv_receive(WalReceiverConn *conn, char **buffer,
return -1;
}
else
- {
- PQclear(res);
ereport(ERROR,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("could not receive data from WAL stream: %s",
pchomp(PQerrorMessage(conn->streamConn)))));
- }
}
if (rawlen < -1)
ereport(ERROR,
@@ -971,13 +947,10 @@ libpqrcv_create_slot(WalReceiverConn *conn, const char *slotname,
pfree(cmd.data);
if (PQresultStatus(res) != PGRES_TUPLES_OK)
- {
- PQclear(res);
ereport(ERROR,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("could not create replication slot \"%s\": %s",
slotname, pchomp(PQerrorMessage(conn->streamConn)))));
- }
if (lsn)
*lsn = DatumGetLSN(DirectFunctionCall1Coll(pg_lsn_in, InvalidOid,
diff --git a/src/include/libpq/libpq-be-fe-helpers.h b/src/include/libpq/libpq-be-fe-helpers.h
index 9a0373212da..e9fc59716ad 100644
--- a/src/include/libpq/libpq-be-fe-helpers.h
+++ b/src/include/libpq/libpq-be-fe-helpers.h
@@ -279,11 +279,8 @@ libpqsrv_exec_params(PGconn *conn,
static inline PGresult *
libpqsrv_get_result_last(PGconn *conn, uint32 wait_event_info)
{
- PGresult *volatile lastResult = NULL;
+ PGresult *lastResult = NULL;
- /* In what follows, do not leak any PGresults on an error. */
- PG_TRY();
- {
for (;;)
{
/* Wait for, and collect, the next PGresult. */
@@ -306,14 +303,6 @@ libpqsrv_get_result_last(PGconn *conn, uint32 wait_event_info)
PQstatus(conn) == CONNECTION_BAD)
break;
}
- }
- PG_CATCH();
- {
- PQclear(lastResult);
- PG_RE_THROW();
- }
- PG_END_TRY();
-
return lastResult;
}
--
2.43.5
From 34b49be9ba8080f03a630727df7561644ea8882a Mon Sep 17 00:00:00 2001
From: Tom Lane <t...@sss.pgh.pa.us>
Date: Mon, 26 May 2025 14:15:28 -0400
Subject: [PATCH v4 3/5] Run pgindent on the changes of the previous patch.
This step can be checked mechanically.
Author: Tom Lane <t...@sss.pgh.pa.us>
Discussion: https://postgr.es/m/2976982.1748049...@sss.pgh.pa.us
---
contrib/dblink/dblink.c | 180 +++---
contrib/postgres_fdw/connection.c | 188 +++---
contrib/postgres_fdw/postgres_fdw.c | 721 ++++++++++++------------
src/include/libpq/libpq-be-fe-helpers.h | 38 +-
4 files changed, 562 insertions(+), 565 deletions(-)
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index e43e446e818..1e1baf68635 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -861,115 +861,115 @@ materializeResult(FunctionCallInfo fcinfo, PGconn *conn, PGresult *res)
/* prepTuplestoreResult must have been called previously */
Assert(rsinfo->returnMode == SFRM_Materialize);
- if (PQresultStatus(res) == PGRES_COMMAND_OK)
- {
- is_sql_cmd = true;
-
- /*
- * need a tuple descriptor representing one TEXT column to return
- * the command status string as our result tuple
- */
- tupdesc = CreateTemplateTupleDesc(1);
- TupleDescInitEntry(tupdesc, (AttrNumber) 1, "status",
- TEXTOID, -1, 0);
- ntuples = 1;
- nfields = 1;
- }
- else
- {
- Assert(PQresultStatus(res) == PGRES_TUPLES_OK);
+ if (PQresultStatus(res) == PGRES_COMMAND_OK)
+ {
+ is_sql_cmd = true;
- is_sql_cmd = false;
+ /*
+ * need a tuple descriptor representing one TEXT column to return the
+ * command status string as our result tuple
+ */
+ tupdesc = CreateTemplateTupleDesc(1);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 1, "status",
+ TEXTOID, -1, 0);
+ ntuples = 1;
+ nfields = 1;
+ }
+ else
+ {
+ Assert(PQresultStatus(res) == PGRES_TUPLES_OK);
- /* get a tuple descriptor for our result type */
- switch (get_call_result_type(fcinfo, NULL, &tupdesc))
- {
- case TYPEFUNC_COMPOSITE:
- /* success */
- break;
- case TYPEFUNC_RECORD:
- /* failed to determine actual type of RECORD */
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("function returning record called in context "
- "that cannot accept type record")));
- break;
- default:
- /* result type isn't composite */
- elog(ERROR, "return type must be a row type");
- break;
- }
+ is_sql_cmd = false;
- /* make sure we have a persistent copy of the tupdesc */
- tupdesc = CreateTupleDescCopy(tupdesc);
- ntuples = PQntuples(res);
- nfields = PQnfields(res);
+ /* get a tuple descriptor for our result type */
+ switch (get_call_result_type(fcinfo, NULL, &tupdesc))
+ {
+ case TYPEFUNC_COMPOSITE:
+ /* success */
+ break;
+ case TYPEFUNC_RECORD:
+ /* failed to determine actual type of RECORD */
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("function returning record called in context "
+ "that cannot accept type record")));
+ break;
+ default:
+ /* result type isn't composite */
+ elog(ERROR, "return type must be a row type");
+ break;
}
- /*
- * check result and tuple descriptor have the same number of columns
- */
- if (nfields != tupdesc->natts)
- ereport(ERROR,
- (errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("remote query result rowtype does not match "
- "the specified FROM clause rowtype")));
+ /* make sure we have a persistent copy of the tupdesc */
+ tupdesc = CreateTupleDescCopy(tupdesc);
+ ntuples = PQntuples(res);
+ nfields = PQnfields(res);
+ }
- if (ntuples > 0)
- {
- AttInMetadata *attinmeta;
- int nestlevel = -1;
- Tuplestorestate *tupstore;
- MemoryContext oldcontext;
- int row;
- char **values;
+ /*
+ * check result and tuple descriptor have the same number of columns
+ */
+ if (nfields != tupdesc->natts)
+ ereport(ERROR,
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("remote query result rowtype does not match "
+ "the specified FROM clause rowtype")));
- attinmeta = TupleDescGetAttInMetadata(tupdesc);
+ if (ntuples > 0)
+ {
+ AttInMetadata *attinmeta;
+ int nestlevel = -1;
+ Tuplestorestate *tupstore;
+ MemoryContext oldcontext;
+ int row;
+ char **values;
- /* Set GUCs to ensure we read GUC-sensitive data types correctly */
- if (!is_sql_cmd)
- nestlevel = applyRemoteGucs(conn);
+ attinmeta = TupleDescGetAttInMetadata(tupdesc);
- oldcontext = MemoryContextSwitchTo(rsinfo->econtext->ecxt_per_query_memory);
- tupstore = tuplestore_begin_heap(true, false, work_mem);
- rsinfo->setResult = tupstore;
- rsinfo->setDesc = tupdesc;
- MemoryContextSwitchTo(oldcontext);
+ /* Set GUCs to ensure we read GUC-sensitive data types correctly */
+ if (!is_sql_cmd)
+ nestlevel = applyRemoteGucs(conn);
- values = palloc_array(char *, nfields);
+ oldcontext = MemoryContextSwitchTo(rsinfo->econtext->ecxt_per_query_memory);
+ tupstore = tuplestore_begin_heap(true, false, work_mem);
+ rsinfo->setResult = tupstore;
+ rsinfo->setDesc = tupdesc;
+ MemoryContextSwitchTo(oldcontext);
- /* put all tuples into the tuplestore */
- for (row = 0; row < ntuples; row++)
+ values = palloc_array(char *, nfields);
+
+ /* put all tuples into the tuplestore */
+ for (row = 0; row < ntuples; row++)
+ {
+ HeapTuple tuple;
+
+ if (!is_sql_cmd)
{
- HeapTuple tuple;
+ int i;
- if (!is_sql_cmd)
- {
- int i;
-
- for (i = 0; i < nfields; i++)
- {
- if (PQgetisnull(res, row, i))
- values[i] = NULL;
- else
- values[i] = PQgetvalue(res, row, i);
- }
- }
- else
+ for (i = 0; i < nfields; i++)
{
- values[0] = PQcmdStatus(res);
+ if (PQgetisnull(res, row, i))
+ values[i] = NULL;
+ else
+ values[i] = PQgetvalue(res, row, i);
}
-
- /* build the tuple and put it into the tuplestore. */
- tuple = BuildTupleFromCStrings(attinmeta, values);
- tuplestore_puttuple(tupstore, tuple);
+ }
+ else
+ {
+ values[0] = PQcmdStatus(res);
}
- /* clean up GUC settings, if we changed any */
- restoreLocalGucs(nestlevel);
+ /* build the tuple and put it into the tuplestore. */
+ tuple = BuildTupleFromCStrings(attinmeta, values);
+ tuplestore_puttuple(tupstore, tuple);
}
- PQclear(res);
+ /* clean up GUC settings, if we changed any */
+ restoreLocalGucs(nestlevel);
+ }
+
+ PQclear(res);
}
/*
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index 2082c0b2ff8..d80860aed8e 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -979,40 +979,40 @@ void
pgfdw_report_error(int elevel, PGresult *res, PGconn *conn,
const char *sql)
{
- char *diag_sqlstate = PQresultErrorField(res, PG_DIAG_SQLSTATE);
- char *message_primary = PQresultErrorField(res, PG_DIAG_MESSAGE_PRIMARY);
- char *message_detail = PQresultErrorField(res, PG_DIAG_MESSAGE_DETAIL);
- char *message_hint = PQresultErrorField(res, PG_DIAG_MESSAGE_HINT);
- char *message_context = PQresultErrorField(res, PG_DIAG_CONTEXT);
- int sqlstate;
-
- if (diag_sqlstate)
- sqlstate = MAKE_SQLSTATE(diag_sqlstate[0],
- diag_sqlstate[1],
- diag_sqlstate[2],
- diag_sqlstate[3],
- diag_sqlstate[4]);
- else
- sqlstate = ERRCODE_CONNECTION_FAILURE;
+ char *diag_sqlstate = PQresultErrorField(res, PG_DIAG_SQLSTATE);
+ char *message_primary = PQresultErrorField(res, PG_DIAG_MESSAGE_PRIMARY);
+ char *message_detail = PQresultErrorField(res, PG_DIAG_MESSAGE_DETAIL);
+ char *message_hint = PQresultErrorField(res, PG_DIAG_MESSAGE_HINT);
+ char *message_context = PQresultErrorField(res, PG_DIAG_CONTEXT);
+ int sqlstate;
+
+ if (diag_sqlstate)
+ sqlstate = MAKE_SQLSTATE(diag_sqlstate[0],
+ diag_sqlstate[1],
+ diag_sqlstate[2],
+ diag_sqlstate[3],
+ diag_sqlstate[4]);
+ else
+ sqlstate = ERRCODE_CONNECTION_FAILURE;
- /*
- * If we don't get a message from the PGresult, try the PGconn. This
- * is needed because for connection-level failures, PQgetResult may
- * just return NULL, not a PGresult at all.
- */
- if (message_primary == NULL)
- message_primary = pchomp(PQerrorMessage(conn));
-
- ereport(elevel,
- (errcode(sqlstate),
- (message_primary != NULL && message_primary[0] != '\0') ?
- errmsg_internal("%s", message_primary) :
- errmsg("could not obtain message string for remote error"),
- message_detail ? errdetail_internal("%s", message_detail) : 0,
- message_hint ? errhint("%s", message_hint) : 0,
- message_context ? errcontext("%s", message_context) : 0,
- sql ? errcontext("remote SQL command: %s", sql) : 0));
- PQclear(res);
+ /*
+ * If we don't get a message from the PGresult, try the PGconn. This is
+ * needed because for connection-level failures, PQgetResult may just
+ * return NULL, not a PGresult at all.
+ */
+ if (message_primary == NULL)
+ message_primary = pchomp(PQerrorMessage(conn));
+
+ ereport(elevel,
+ (errcode(sqlstate),
+ (message_primary != NULL && message_primary[0] != '\0') ?
+ errmsg_internal("%s", message_primary) :
+ errmsg("could not obtain message string for remote error"),
+ message_detail ? errdetail_internal("%s", message_detail) : 0,
+ message_hint ? errhint("%s", message_hint) : 0,
+ message_context ? errcontext("%s", message_context) : 0,
+ sql ? errcontext("remote SQL command: %s", sql) : 0));
+ PQclear(res);
}
/*
@@ -1614,83 +1614,83 @@ pgfdw_get_cleanup_result(PGconn *conn, TimestampTz endtime,
*result = NULL;
*timed_out = false;
- for (;;)
+ for (;;)
+ {
+ PGresult *res;
+
+ while (PQisBusy(conn))
{
- PGresult *res;
+ int wc;
+ TimestampTz now = GetCurrentTimestamp();
+ long cur_timeout;
- while (PQisBusy(conn))
+ /* If timeout has expired, give up. */
+ if (now >= endtime)
{
- int wc;
- TimestampTz now = GetCurrentTimestamp();
- long cur_timeout;
-
- /* If timeout has expired, give up. */
- if (now >= endtime)
- {
- *timed_out = true;
- failed = true;
- goto exit;
- }
+ *timed_out = true;
+ failed = true;
+ goto exit;
+ }
- /* If we need to re-issue the cancel request, do that. */
- if (now >= retrycanceltime)
- {
- /* We ignore failure to issue the repeated request. */
- (void) libpqsrv_cancel(conn, endtime);
+ /* If we need to re-issue the cancel request, do that. */
+ if (now >= retrycanceltime)
+ {
+ /* We ignore failure to issue the repeated request. */
+ (void) libpqsrv_cancel(conn, endtime);
- /* Recompute "now" in case that took measurable time. */
- now = GetCurrentTimestamp();
+ /* Recompute "now" in case that took measurable time. */
+ now = GetCurrentTimestamp();
- /* Adjust re-cancel timeout in increasing steps. */
- retrycanceltime = TimestampTzPlusMilliseconds(now,
- canceldelta);
- canceldelta += canceldelta;
- }
+ /* Adjust re-cancel timeout in increasing steps. */
+ retrycanceltime = TimestampTzPlusMilliseconds(now,
+ canceldelta);
+ canceldelta += canceldelta;
+ }
- /* If timeout has expired, give up, else get sleep time. */
- cur_timeout = TimestampDifferenceMilliseconds(now,
- Min(endtime,
- retrycanceltime));
- if (cur_timeout <= 0)
- {
- *timed_out = true;
- failed = true;
- goto exit;
- }
+ /* If timeout has expired, give up, else get sleep time. */
+ cur_timeout = TimestampDifferenceMilliseconds(now,
+ Min(endtime,
+ retrycanceltime));
+ if (cur_timeout <= 0)
+ {
+ *timed_out = true;
+ failed = true;
+ goto exit;
+ }
- /* first time, allocate or get the custom wait event */
- if (pgfdw_we_cleanup_result == 0)
- pgfdw_we_cleanup_result = WaitEventExtensionNew("PostgresFdwCleanupResult");
+ /* first time, allocate or get the custom wait event */
+ if (pgfdw_we_cleanup_result == 0)
+ pgfdw_we_cleanup_result = WaitEventExtensionNew("PostgresFdwCleanupResult");
- /* Sleep until there's something to do */
- wc = WaitLatchOrSocket(MyLatch,
- WL_LATCH_SET | WL_SOCKET_READABLE |
- WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
- PQsocket(conn),
- cur_timeout, pgfdw_we_cleanup_result);
- ResetLatch(MyLatch);
+ /* Sleep until there's something to do */
+ wc = WaitLatchOrSocket(MyLatch,
+ WL_LATCH_SET | WL_SOCKET_READABLE |
+ WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
+ PQsocket(conn),
+ cur_timeout, pgfdw_we_cleanup_result);
+ ResetLatch(MyLatch);
- CHECK_FOR_INTERRUPTS();
+ CHECK_FOR_INTERRUPTS();
- /* Data available in socket? */
- if (wc & WL_SOCKET_READABLE)
+ /* Data available in socket? */
+ if (wc & WL_SOCKET_READABLE)
+ {
+ if (!PQconsumeInput(conn))
{
- if (!PQconsumeInput(conn))
- {
- /* connection trouble */
- failed = true;
- goto exit;
- }
+ /* connection trouble */
+ failed = true;
+ goto exit;
}
}
+ }
- res = PQgetResult(conn);
- if (res == NULL)
- break; /* query is complete */
+ res = PQgetResult(conn);
+ if (res == NULL)
+ break; /* query is complete */
- PQclear(last_res);
- last_res = res;
- }
+ PQclear(last_res);
+ last_res = res;
+ }
exit:
if (failed)
PQclear(last_res);
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 610ea779a87..837965a1a3c 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -3598,31 +3598,31 @@ get_remote_estimate(const char *sql, PGconn *conn,
Cost *startup_cost, Cost *total_cost)
{
PGresult *res;
- char *line;
- char *p;
- int n;
+ char *line;
+ char *p;
+ int n;
- /*
- * Execute EXPLAIN remotely.
- */
- res = pgfdw_exec_query(conn, sql, NULL);
- if (PQresultStatus(res) != PGRES_TUPLES_OK)
- pgfdw_report_error(ERROR, res, conn, sql);
+ /*
+ * Execute EXPLAIN remotely.
+ */
+ res = pgfdw_exec_query(conn, sql, NULL);
+ if (PQresultStatus(res) != PGRES_TUPLES_OK)
+ pgfdw_report_error(ERROR, res, conn, sql);
- /*
- * Extract cost numbers for topmost plan node. Note we search for a
- * left paren from the end of the line to avoid being confused by
- * other uses of parentheses.
- */
- line = PQgetvalue(res, 0, 0);
- p = strrchr(line, '(');
- if (p == NULL)
- elog(ERROR, "could not interpret EXPLAIN output: \"%s\"", line);
- n = sscanf(p, "(cost=%lf..%lf rows=%lf width=%d)",
- startup_cost, total_cost, rows, width);
- if (n != 4)
- elog(ERROR, "could not interpret EXPLAIN output: \"%s\"", line);
- PQclear(res);
+ /*
+ * Extract cost numbers for topmost plan node. Note we search for a left
+ * paren from the end of the line to avoid being confused by other uses of
+ * parentheses.
+ */
+ line = PQgetvalue(res, 0, 0);
+ p = strrchr(line, '(');
+ if (p == NULL)
+ elog(ERROR, "could not interpret EXPLAIN output: \"%s\"", line);
+ n = sscanf(p, "(cost=%lf..%lf rows=%lf width=%d)",
+ startup_cost, total_cost, rows, width);
+ if (n != 4)
+ elog(ERROR, "could not interpret EXPLAIN output: \"%s\"", line);
+ PQclear(res);
}
/*
@@ -3805,63 +3805,63 @@ fetch_more_data(ForeignScanState *node)
MemoryContextReset(fsstate->batch_cxt);
oldcontext = MemoryContextSwitchTo(fsstate->batch_cxt);
- if (fsstate->async_capable)
- {
- Assert(fsstate->conn_state->pendingAreq);
+ if (fsstate->async_capable)
+ {
+ Assert(fsstate->conn_state->pendingAreq);
- /*
- * The query was already sent by an earlier call to
- * fetch_more_data_begin. So now we just fetch the result.
- */
- res = pgfdw_get_result(conn);
- /* On error, report the original query, not the FETCH. */
- if (PQresultStatus(res) != PGRES_TUPLES_OK)
- pgfdw_report_error(ERROR, res, conn, fsstate->query);
+ /*
+ * The query was already sent by an earlier call to
+ * fetch_more_data_begin. So now we just fetch the result.
+ */
+ res = pgfdw_get_result(conn);
+ /* On error, report the original query, not the FETCH. */
+ if (PQresultStatus(res) != PGRES_TUPLES_OK)
+ pgfdw_report_error(ERROR, res, conn, fsstate->query);
- /* Reset per-connection state */
- fsstate->conn_state->pendingAreq = NULL;
- }
- else
- {
- char sql[64];
+ /* Reset per-connection state */
+ fsstate->conn_state->pendingAreq = NULL;
+ }
+ else
+ {
+ char sql[64];
- /* This is a regular synchronous fetch. */
- snprintf(sql, sizeof(sql), "FETCH %d FROM c%u",
- fsstate->fetch_size, fsstate->cursor_number);
+ /* This is a regular synchronous fetch. */
+ snprintf(sql, sizeof(sql), "FETCH %d FROM c%u",
+ fsstate->fetch_size, fsstate->cursor_number);
- res = pgfdw_exec_query(conn, sql, fsstate->conn_state);
- /* On error, report the original query, not the FETCH. */
- if (PQresultStatus(res) != PGRES_TUPLES_OK)
- pgfdw_report_error(ERROR, res, conn, fsstate->query);
- }
+ res = pgfdw_exec_query(conn, sql, fsstate->conn_state);
+ /* On error, report the original query, not the FETCH. */
+ if (PQresultStatus(res) != PGRES_TUPLES_OK)
+ pgfdw_report_error(ERROR, res, conn, fsstate->query);
+ }
- /* Convert the data into HeapTuples */
- numrows = PQntuples(res);
- fsstate->tuples = (HeapTuple *) palloc0(numrows * sizeof(HeapTuple));
- fsstate->num_tuples = numrows;
- fsstate->next_tuple = 0;
+ /* Convert the data into HeapTuples */
+ numrows = PQntuples(res);
+ fsstate->tuples = (HeapTuple *) palloc0(numrows * sizeof(HeapTuple));
+ fsstate->num_tuples = numrows;
+ fsstate->next_tuple = 0;
- for (i = 0; i < numrows; i++)
- {
- Assert(IsA(node->ss.ps.plan, ForeignScan));
-
- fsstate->tuples[i] =
- make_tuple_from_result_row(res, i,
- fsstate->rel,
- fsstate->attinmeta,
- fsstate->retrieved_attrs,
- node,
- fsstate->temp_cxt);
- }
+ for (i = 0; i < numrows; i++)
+ {
+ Assert(IsA(node->ss.ps.plan, ForeignScan));
+
+ fsstate->tuples[i] =
+ make_tuple_from_result_row(res, i,
+ fsstate->rel,
+ fsstate->attinmeta,
+ fsstate->retrieved_attrs,
+ node,
+ fsstate->temp_cxt);
+ }
- /* Update fetch_ct_2 */
- if (fsstate->fetch_ct_2 < 2)
- fsstate->fetch_ct_2++;
+ /* Update fetch_ct_2 */
+ if (fsstate->fetch_ct_2 < 2)
+ fsstate->fetch_ct_2++;
- /* Must be EOF if we didn't get as many tuples as we asked for. */
- fsstate->eof_reached = (numrows < fsstate->fetch_size);
+ /* Must be EOF if we didn't get as many tuples as we asked for. */
+ fsstate->eof_reached = (numrows < fsstate->fetch_size);
- PQclear(res);
+ PQclear(res);
MemoryContextSwitchTo(oldcontext);
}
@@ -4315,20 +4315,20 @@ static void
store_returning_result(PgFdwModifyState *fmstate,
TupleTableSlot *slot, PGresult *res)
{
- HeapTuple newtup;
+ HeapTuple newtup;
- newtup = make_tuple_from_result_row(res, 0,
- fmstate->rel,
- fmstate->attinmeta,
- fmstate->retrieved_attrs,
- NULL,
- fmstate->temp_cxt);
+ newtup = make_tuple_from_result_row(res, 0,
+ fmstate->rel,
+ fmstate->attinmeta,
+ fmstate->retrieved_attrs,
+ NULL,
+ fmstate->temp_cxt);
- /*
- * The returning slot will not necessarily be suitable to store
- * heaptuples directly, so allow for conversion.
- */
- ExecForceStoreHeapTuple(newtup, slot, true);
+ /*
+ * The returning slot will not necessarily be suitable to store heaptuples
+ * directly, so allow for conversion.
+ */
+ ExecForceStoreHeapTuple(newtup, slot, true);
}
/*
@@ -4593,16 +4593,16 @@ get_returning_data(ForeignScanState *node)
}
else
{
- HeapTuple newtup;
+ HeapTuple newtup;
- newtup = make_tuple_from_result_row(dmstate->result,
- dmstate->next_tuple,
- dmstate->rel,
- dmstate->attinmeta,
- dmstate->retrieved_attrs,
- node,
- dmstate->temp_cxt);
- ExecStoreHeapTuple(newtup, slot, false);
+ newtup = make_tuple_from_result_row(dmstate->result,
+ dmstate->next_tuple,
+ dmstate->rel,
+ dmstate->attinmeta,
+ dmstate->retrieved_attrs,
+ node,
+ dmstate->temp_cxt);
+ ExecStoreHeapTuple(newtup, slot, false);
/* Get the updated/deleted tuple. */
if (dmstate->rel)
@@ -4915,14 +4915,14 @@ postgresAnalyzeForeignTable(Relation relation,
initStringInfo(&sql);
deparseAnalyzeSizeSql(&sql, relation);
- res = pgfdw_exec_query(conn, sql.data, NULL);
- if (PQresultStatus(res) != PGRES_TUPLES_OK)
- pgfdw_report_error(ERROR, res, conn, sql.data);
+ res = pgfdw_exec_query(conn, sql.data, NULL);
+ if (PQresultStatus(res) != PGRES_TUPLES_OK)
+ pgfdw_report_error(ERROR, res, conn, sql.data);
- if (PQntuples(res) != 1 || PQnfields(res) != 1)
- elog(ERROR, "unexpected result from deparseAnalyzeSizeSql query");
- *totalpages = strtoul(PQgetvalue(res, 0, 0), NULL, 10);
- PQclear(res);
+ if (PQntuples(res) != 1 || PQnfields(res) != 1)
+ elog(ERROR, "unexpected result from deparseAnalyzeSizeSql query");
+ *totalpages = strtoul(PQgetvalue(res, 0, 0), NULL, 10);
+ PQclear(res);
ReleaseConnection(conn);
@@ -4964,15 +4964,15 @@ postgresGetAnalyzeInfoForForeignTable(Relation relation, bool *can_tablesample)
initStringInfo(&sql);
deparseAnalyzeInfoSql(&sql, relation);
- res = pgfdw_exec_query(conn, sql.data, NULL);
- if (PQresultStatus(res) != PGRES_TUPLES_OK)
- pgfdw_report_error(ERROR, res, conn, sql.data);
+ res = pgfdw_exec_query(conn, sql.data, NULL);
+ if (PQresultStatus(res) != PGRES_TUPLES_OK)
+ pgfdw_report_error(ERROR, res, conn, sql.data);
- if (PQntuples(res) != 1 || PQnfields(res) != 2)
- elog(ERROR, "unexpected result from deparseAnalyzeInfoSql query");
- reltuples = strtod(PQgetvalue(res, 0, 0), NULL);
- relkind = *(PQgetvalue(res, 0, 1));
- PQclear(res);
+ if (PQntuples(res) != 1 || PQnfields(res) != 2)
+ elog(ERROR, "unexpected result from deparseAnalyzeInfoSql query");
+ reltuples = strtod(PQgetvalue(res, 0, 0), NULL);
+ relkind = *(PQgetvalue(res, 0, 1));
+ PQclear(res);
ReleaseConnection(conn);
@@ -5194,76 +5194,76 @@ postgresAcquireSampleRowsFunc(Relation relation, int elevel,
deparseAnalyzeSql(&sql, relation, method, sample_frac, &astate.retrieved_attrs);
- res = pgfdw_exec_query(conn, sql.data, NULL);
- if (PQresultStatus(res) != PGRES_COMMAND_OK)
- pgfdw_report_error(ERROR, res, conn, sql.data);
- PQclear(res);
+ res = pgfdw_exec_query(conn, sql.data, NULL);
+ if (PQresultStatus(res) != PGRES_COMMAND_OK)
+ pgfdw_report_error(ERROR, res, conn, sql.data);
+ PQclear(res);
- /*
- * Determine the fetch size. The default is arbitrary, but shouldn't
- * be enormous.
- */
- fetch_size = 100;
- foreach(lc, server->options)
- {
- DefElem *def = (DefElem *) lfirst(lc);
+ /*
+ * Determine the fetch size. The default is arbitrary, but shouldn't be
+ * enormous.
+ */
+ fetch_size = 100;
+ foreach(lc, server->options)
+ {
+ DefElem *def = (DefElem *) lfirst(lc);
- if (strcmp(def->defname, "fetch_size") == 0)
- {
- (void) parse_int(defGetString(def), &fetch_size, 0, NULL);
- break;
- }
- }
- foreach(lc, table->options)
+ if (strcmp(def->defname, "fetch_size") == 0)
{
- DefElem *def = (DefElem *) lfirst(lc);
+ (void) parse_int(defGetString(def), &fetch_size, 0, NULL);
+ break;
+ }
+ }
+ foreach(lc, table->options)
+ {
+ DefElem *def = (DefElem *) lfirst(lc);
- if (strcmp(def->defname, "fetch_size") == 0)
- {
- (void) parse_int(defGetString(def), &fetch_size, 0, NULL);
- break;
- }
+ if (strcmp(def->defname, "fetch_size") == 0)
+ {
+ (void) parse_int(defGetString(def), &fetch_size, 0, NULL);
+ break;
}
+ }
- /* Construct command to fetch rows from remote. */
- snprintf(fetch_sql, sizeof(fetch_sql), "FETCH %d FROM c%u",
- fetch_size, cursor_number);
+ /* Construct command to fetch rows from remote. */
+ snprintf(fetch_sql, sizeof(fetch_sql), "FETCH %d FROM c%u",
+ fetch_size, cursor_number);
- /* Retrieve and process rows a batch at a time. */
- for (;;)
- {
- int numrows;
- int i;
+ /* Retrieve and process rows a batch at a time. */
+ for (;;)
+ {
+ int numrows;
+ int i;
- /* Allow users to cancel long query */
- CHECK_FOR_INTERRUPTS();
+ /* Allow users to cancel long query */
+ CHECK_FOR_INTERRUPTS();
- /*
- * XXX possible future improvement: if rowstoskip is large, we
- * could issue a MOVE rather than physically fetching the rows,
- * then just adjust rowstoskip and samplerows appropriately.
- */
+ /*
+ * XXX possible future improvement: if rowstoskip is large, we could
+ * issue a MOVE rather than physically fetching the rows, then just
+ * adjust rowstoskip and samplerows appropriately.
+ */
- /* Fetch some rows */
- res = pgfdw_exec_query(conn, fetch_sql, NULL);
- /* On error, report the original query, not the FETCH. */
- if (PQresultStatus(res) != PGRES_TUPLES_OK)
- pgfdw_report_error(ERROR, res, conn, sql.data);
+ /* Fetch some rows */
+ res = pgfdw_exec_query(conn, fetch_sql, NULL);
+ /* On error, report the original query, not the FETCH. */
+ if (PQresultStatus(res) != PGRES_TUPLES_OK)
+ pgfdw_report_error(ERROR, res, conn, sql.data);
- /* Process whatever we got. */
- numrows = PQntuples(res);
- for (i = 0; i < numrows; i++)
- analyze_row_processor(res, i, &astate);
+ /* Process whatever we got. */
+ numrows = PQntuples(res);
+ for (i = 0; i < numrows; i++)
+ analyze_row_processor(res, i, &astate);
- PQclear(res);
+ PQclear(res);
- /* Must be EOF if we didn't get all the rows requested. */
- if (numrows < fetch_size)
- break;
- }
+ /* Must be EOF if we didn't get all the rows requested. */
+ if (numrows < fetch_size)
+ break;
+ }
- /* Close the cursor, just to be tidy. */
- close_cursor(conn, cursor_number, NULL);
+ /* Close the cursor, just to be tidy. */
+ close_cursor(conn, cursor_number, NULL);
ReleaseConnection(conn);
@@ -5414,234 +5414,231 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
/* Create workspace for strings */
initStringInfo(&buf);
- /* Check that the schema really exists */
- appendStringInfoString(&buf, "SELECT 1 FROM pg_catalog.pg_namespace WHERE nspname = ");
- deparseStringLiteral(&buf, stmt->remote_schema);
+ /* Check that the schema really exists */
+ appendStringInfoString(&buf, "SELECT 1 FROM pg_catalog.pg_namespace WHERE nspname = ");
+ deparseStringLiteral(&buf, stmt->remote_schema);
- res = pgfdw_exec_query(conn, buf.data, NULL);
- if (PQresultStatus(res) != PGRES_TUPLES_OK)
- pgfdw_report_error(ERROR, res, conn, buf.data);
+ res = pgfdw_exec_query(conn, buf.data, NULL);
+ if (PQresultStatus(res) != PGRES_TUPLES_OK)
+ pgfdw_report_error(ERROR, res, conn, buf.data);
- if (PQntuples(res) != 1)
- ereport(ERROR,
- (errcode(ERRCODE_FDW_SCHEMA_NOT_FOUND),
- errmsg("schema \"%s\" is not present on foreign server \"%s\"",
- stmt->remote_schema, server->servername)));
+ if (PQntuples(res) != 1)
+ ereport(ERROR,
+ (errcode(ERRCODE_FDW_SCHEMA_NOT_FOUND),
+ errmsg("schema \"%s\" is not present on foreign server \"%s\"",
+ stmt->remote_schema, server->servername)));
- PQclear(res);
- resetStringInfo(&buf);
+ PQclear(res);
+ resetStringInfo(&buf);
- /*
- * Fetch all table data from this schema, possibly restricted by
- * EXCEPT or LIMIT TO. (We don't actually need to pay any attention
- * to EXCEPT/LIMIT TO here, because the core code will filter the
- * statements we return according to those lists anyway. But it
- * should save a few cycles to not process excluded tables in the
- * first place.)
- *
- * Import table data for partitions only when they are explicitly
- * specified in LIMIT TO clause. Otherwise ignore them and only
- * include the definitions of the root partitioned tables to allow
- * access to the complete remote data set locally in the schema
- * imported.
- *
- * Note: because we run the connection with search_path restricted to
- * pg_catalog, the format_type() and pg_get_expr() outputs will always
- * include a schema name for types/functions in other schemas, which
- * is what we want.
- */
+ /*
+ * Fetch all table data from this schema, possibly restricted by EXCEPT or
+ * LIMIT TO. (We don't actually need to pay any attention to EXCEPT/LIMIT
+ * TO here, because the core code will filter the statements we return
+ * according to those lists anyway. But it should save a few cycles to
+ * not process excluded tables in the first place.)
+ *
+ * Import table data for partitions only when they are explicitly
+ * specified in LIMIT TO clause. Otherwise ignore them and only include
+ * the definitions of the root partitioned tables to allow access to the
+ * complete remote data set locally in the schema imported.
+ *
+ * Note: because we run the connection with search_path restricted to
+ * pg_catalog, the format_type() and pg_get_expr() outputs will always
+ * include a schema name for types/functions in other schemas, which is
+ * what we want.
+ */
+ appendStringInfoString(&buf,
+ "SELECT relname, "
+ " attname, "
+ " format_type(atttypid, atttypmod), "
+ " attnotnull, "
+ " pg_get_expr(adbin, adrelid), ");
+
+ /* Generated columns are supported since Postgres 12 */
+ if (PQserverVersion(conn) >= 120000)
appendStringInfoString(&buf,
- "SELECT relname, "
- " attname, "
- " format_type(atttypid, atttypmod), "
- " attnotnull, "
- " pg_get_expr(adbin, adrelid), ");
-
- /* Generated columns are supported since Postgres 12 */
- if (PQserverVersion(conn) >= 120000)
- appendStringInfoString(&buf,
- " attgenerated, ");
- else
- appendStringInfoString(&buf,
- " NULL, ");
-
- if (import_collate)
- appendStringInfoString(&buf,
- " collname, "
- " collnsp.nspname ");
- else
- appendStringInfoString(&buf,
- " NULL, NULL ");
-
+ " attgenerated, ");
+ else
appendStringInfoString(&buf,
- "FROM pg_class c "
- " JOIN pg_namespace n ON "
- " relnamespace = n.oid "
- " LEFT JOIN pg_attribute a ON "
- " attrelid = c.oid AND attnum > 0 "
- " AND NOT attisdropped "
- " LEFT JOIN pg_attrdef ad ON "
- " adrelid = c.oid AND adnum = attnum ");
-
- if (import_collate)
- appendStringInfoString(&buf,
- " LEFT JOIN pg_collation coll ON "
- " coll.oid = attcollation "
- " LEFT JOIN pg_namespace collnsp ON "
- " collnsp.oid = collnamespace ");
+ " NULL, ");
+ if (import_collate)
appendStringInfoString(&buf,
- "WHERE c.relkind IN ("
- CppAsString2(RELKIND_RELATION) ","
- CppAsString2(RELKIND_VIEW) ","
- CppAsString2(RELKIND_FOREIGN_TABLE) ","
- CppAsString2(RELKIND_MATVIEW) ","
- CppAsString2(RELKIND_PARTITIONED_TABLE) ") "
- " AND n.nspname = ");
- deparseStringLiteral(&buf, stmt->remote_schema);
+ " collname, "
+ " collnsp.nspname ");
+ else
+ appendStringInfoString(&buf,
+ " NULL, NULL ");
+
+ appendStringInfoString(&buf,
+ "FROM pg_class c "
+ " JOIN pg_namespace n ON "
+ " relnamespace = n.oid "
+ " LEFT JOIN pg_attribute a ON "
+ " attrelid = c.oid AND attnum > 0 "
+ " AND NOT attisdropped "
+ " LEFT JOIN pg_attrdef ad ON "
+ " adrelid = c.oid AND adnum = attnum ");
+
+ if (import_collate)
+ appendStringInfoString(&buf,
+ " LEFT JOIN pg_collation coll ON "
+ " coll.oid = attcollation "
+ " LEFT JOIN pg_namespace collnsp ON "
+ " collnsp.oid = collnamespace ");
+
+ appendStringInfoString(&buf,
+ "WHERE c.relkind IN ("
+ CppAsString2(RELKIND_RELATION) ","
+ CppAsString2(RELKIND_VIEW) ","
+ CppAsString2(RELKIND_FOREIGN_TABLE) ","
+ CppAsString2(RELKIND_MATVIEW) ","
+ CppAsString2(RELKIND_PARTITIONED_TABLE) ") "
+ " AND n.nspname = ");
+ deparseStringLiteral(&buf, stmt->remote_schema);
+
+ /* Partitions are supported since Postgres 10 */
+ if (PQserverVersion(conn) >= 100000 &&
+ stmt->list_type != FDW_IMPORT_SCHEMA_LIMIT_TO)
+ appendStringInfoString(&buf, " AND NOT c.relispartition ");
+
+ /* Apply restrictions for LIMIT TO and EXCEPT */
+ if (stmt->list_type == FDW_IMPORT_SCHEMA_LIMIT_TO ||
+ stmt->list_type == FDW_IMPORT_SCHEMA_EXCEPT)
+ {
+ bool first_item = true;
- /* Partitions are supported since Postgres 10 */
- if (PQserverVersion(conn) >= 100000 &&
- stmt->list_type != FDW_IMPORT_SCHEMA_LIMIT_TO)
- appendStringInfoString(&buf, " AND NOT c.relispartition ");
+ appendStringInfoString(&buf, " AND c.relname ");
+ if (stmt->list_type == FDW_IMPORT_SCHEMA_EXCEPT)
+ appendStringInfoString(&buf, "NOT ");
+ appendStringInfoString(&buf, "IN (");
- /* Apply restrictions for LIMIT TO and EXCEPT */
- if (stmt->list_type == FDW_IMPORT_SCHEMA_LIMIT_TO ||
- stmt->list_type == FDW_IMPORT_SCHEMA_EXCEPT)
+ /* Append list of table names within IN clause */
+ foreach(lc, stmt->table_list)
{
- bool first_item = true;
+ RangeVar *rv = (RangeVar *) lfirst(lc);
- appendStringInfoString(&buf, " AND c.relname ");
- if (stmt->list_type == FDW_IMPORT_SCHEMA_EXCEPT)
- appendStringInfoString(&buf, "NOT ");
- appendStringInfoString(&buf, "IN (");
+ if (first_item)
+ first_item = false;
+ else
+ appendStringInfoString(&buf, ", ");
+ deparseStringLiteral(&buf, rv->relname);
+ }
+ appendStringInfoChar(&buf, ')');
+ }
- /* Append list of table names within IN clause */
- foreach(lc, stmt->table_list)
- {
- RangeVar *rv = (RangeVar *) lfirst(lc);
+ /* Append ORDER BY at the end of query to ensure output ordering */
+ appendStringInfoString(&buf, " ORDER BY c.relname, a.attnum");
- if (first_item)
- first_item = false;
- else
- appendStringInfoString(&buf, ", ");
- deparseStringLiteral(&buf, rv->relname);
- }
- appendStringInfoChar(&buf, ')');
- }
+ /* Fetch the data */
+ res = pgfdw_exec_query(conn, buf.data, NULL);
+ if (PQresultStatus(res) != PGRES_TUPLES_OK)
+ pgfdw_report_error(ERROR, res, conn, buf.data);
- /* Append ORDER BY at the end of query to ensure output ordering */
- appendStringInfoString(&buf, " ORDER BY c.relname, a.attnum");
+ /* Process results */
+ numrows = PQntuples(res);
+ /* note: incrementation of i happens in inner loop's while() test */
+ for (i = 0; i < numrows;)
+ {
+ char *tablename = PQgetvalue(res, i, 0);
+ bool first_item = true;
- /* Fetch the data */
- res = pgfdw_exec_query(conn, buf.data, NULL);
- if (PQresultStatus(res) != PGRES_TUPLES_OK)
- pgfdw_report_error(ERROR, res, conn, buf.data);
+ resetStringInfo(&buf);
+ appendStringInfo(&buf, "CREATE FOREIGN TABLE %s (\n",
+ quote_identifier(tablename));
- /* Process results */
- numrows = PQntuples(res);
- /* note: incrementation of i happens in inner loop's while() test */
- for (i = 0; i < numrows;)
+ /* Scan all rows for this table */
+ do
{
- char *tablename = PQgetvalue(res, i, 0);
- bool first_item = true;
+ char *attname;
+ char *typename;
+ char *attnotnull;
+ char *attgenerated;
+ char *attdefault;
+ char *collname;
+ char *collnamespace;
+
+ /* If table has no columns, we'll see nulls here */
+ if (PQgetisnull(res, i, 1))
+ continue;
- resetStringInfo(&buf);
- appendStringInfo(&buf, "CREATE FOREIGN TABLE %s (\n",
- quote_identifier(tablename));
+ attname = PQgetvalue(res, i, 1);
+ typename = PQgetvalue(res, i, 2);
+ attnotnull = PQgetvalue(res, i, 3);
+ attdefault = PQgetisnull(res, i, 4) ? NULL :
+ PQgetvalue(res, i, 4);
+ attgenerated = PQgetisnull(res, i, 5) ? NULL :
+ PQgetvalue(res, i, 5);
+ collname = PQgetisnull(res, i, 6) ? NULL :
+ PQgetvalue(res, i, 6);
+ collnamespace = PQgetisnull(res, i, 7) ? NULL :
+ PQgetvalue(res, i, 7);
+
+ if (first_item)
+ first_item = false;
+ else
+ appendStringInfoString(&buf, ",\n");
- /* Scan all rows for this table */
- do
- {
- char *attname;
- char *typename;
- char *attnotnull;
- char *attgenerated;
- char *attdefault;
- char *collname;
- char *collnamespace;
-
- /* If table has no columns, we'll see nulls here */
- if (PQgetisnull(res, i, 1))
- continue;
+ /* Print column name and type */
+ appendStringInfo(&buf, " %s %s",
+ quote_identifier(attname),
+ typename);
- attname = PQgetvalue(res, i, 1);
- typename = PQgetvalue(res, i, 2);
- attnotnull = PQgetvalue(res, i, 3);
- attdefault = PQgetisnull(res, i, 4) ? NULL :
- PQgetvalue(res, i, 4);
- attgenerated = PQgetisnull(res, i, 5) ? NULL :
- PQgetvalue(res, i, 5);
- collname = PQgetisnull(res, i, 6) ? NULL :
- PQgetvalue(res, i, 6);
- collnamespace = PQgetisnull(res, i, 7) ? NULL :
- PQgetvalue(res, i, 7);
-
- if (first_item)
- first_item = false;
- else
- appendStringInfoString(&buf, ",\n");
+ /*
+ * Add column_name option so that renaming the foreign table's
+ * column doesn't break the association to the underlying column.
+ */
+ appendStringInfoString(&buf, " OPTIONS (column_name ");
+ deparseStringLiteral(&buf, attname);
+ appendStringInfoChar(&buf, ')');
- /* Print column name and type */
- appendStringInfo(&buf, " %s %s",
- quote_identifier(attname),
- typename);
+ /* Add COLLATE if needed */
+ if (import_collate && collname != NULL && collnamespace != NULL)
+ appendStringInfo(&buf, " COLLATE %s.%s",
+ quote_identifier(collnamespace),
+ quote_identifier(collname));
- /*
- * Add column_name option so that renaming the foreign table's
- * column doesn't break the association to the underlying
- * column.
- */
- appendStringInfoString(&buf, " OPTIONS (column_name ");
- deparseStringLiteral(&buf, attname);
- appendStringInfoChar(&buf, ')');
-
- /* Add COLLATE if needed */
- if (import_collate && collname != NULL && collnamespace != NULL)
- appendStringInfo(&buf, " COLLATE %s.%s",
- quote_identifier(collnamespace),
- quote_identifier(collname));
-
- /* Add DEFAULT if needed */
- if (import_default && attdefault != NULL &&
- (!attgenerated || !attgenerated[0]))
- appendStringInfo(&buf, " DEFAULT %s", attdefault);
-
- /* Add GENERATED if needed */
- if (import_generated && attgenerated != NULL &&
- attgenerated[0] == ATTRIBUTE_GENERATED_STORED)
- {
- Assert(attdefault != NULL);
- appendStringInfo(&buf,
- " GENERATED ALWAYS AS (%s) STORED",
- attdefault);
- }
+ /* Add DEFAULT if needed */
+ if (import_default && attdefault != NULL &&
+ (!attgenerated || !attgenerated[0]))
+ appendStringInfo(&buf, " DEFAULT %s", attdefault);
- /* Add NOT NULL if needed */
- if (import_not_null && attnotnull[0] == 't')
- appendStringInfoString(&buf, " NOT NULL");
+ /* Add GENERATED if needed */
+ if (import_generated && attgenerated != NULL &&
+ attgenerated[0] == ATTRIBUTE_GENERATED_STORED)
+ {
+ Assert(attdefault != NULL);
+ appendStringInfo(&buf,
+ " GENERATED ALWAYS AS (%s) STORED",
+ attdefault);
}
- while (++i < numrows &&
- strcmp(PQgetvalue(res, i, 0), tablename) == 0);
- /*
- * Add server name and table-level options. We specify remote
- * schema and table name as options (the latter to ensure that
- * renaming the foreign table doesn't break the association).
- */
- appendStringInfo(&buf, "\n) SERVER %s\nOPTIONS (",
- quote_identifier(server->servername));
+ /* Add NOT NULL if needed */
+ if (import_not_null && attnotnull[0] == 't')
+ appendStringInfoString(&buf, " NOT NULL");
+ }
+ while (++i < numrows &&
+ strcmp(PQgetvalue(res, i, 0), tablename) == 0);
- appendStringInfoString(&buf, "schema_name ");
- deparseStringLiteral(&buf, stmt->remote_schema);
- appendStringInfoString(&buf, ", table_name ");
- deparseStringLiteral(&buf, tablename);
+ /*
+ * Add server name and table-level options. We specify remote schema
+ * and table name as options (the latter to ensure that renaming the
+ * foreign table doesn't break the association).
+ */
+ appendStringInfo(&buf, "\n) SERVER %s\nOPTIONS (",
+ quote_identifier(server->servername));
- appendStringInfoString(&buf, ");");
+ appendStringInfoString(&buf, "schema_name ");
+ deparseStringLiteral(&buf, stmt->remote_schema);
+ appendStringInfoString(&buf, ", table_name ");
+ deparseStringLiteral(&buf, tablename);
- commands = lappend(commands, pstrdup(buf.data));
- }
- PQclear(res);
+ appendStringInfoString(&buf, ");");
+
+ commands = lappend(commands, pstrdup(buf.data));
+ }
+ PQclear(res);
ReleaseConnection(conn);
diff --git a/src/include/libpq/libpq-be-fe-helpers.h b/src/include/libpq/libpq-be-fe-helpers.h
index e9fc59716ad..6f4e2bd7ef9 100644
--- a/src/include/libpq/libpq-be-fe-helpers.h
+++ b/src/include/libpq/libpq-be-fe-helpers.h
@@ -281,28 +281,28 @@ libpqsrv_get_result_last(PGconn *conn, uint32 wait_event_info)
{
PGresult *lastResult = NULL;
- for (;;)
- {
- /* Wait for, and collect, the next PGresult. */
- PGresult *result;
+ for (;;)
+ {
+ /* Wait for, and collect, the next PGresult. */
+ PGresult *result;
- result = libpqsrv_get_result(conn, wait_event_info);
- if (result == NULL)
- break; /* query is complete, or failure */
+ result = libpqsrv_get_result(conn, wait_event_info);
+ if (result == NULL)
+ break; /* query is complete, or failure */
- /*
- * Emulate PQexec()'s behavior of returning the last result when
- * there are many.
- */
- PQclear(lastResult);
- lastResult = result;
+ /*
+ * Emulate PQexec()'s behavior of returning the last result when there
+ * are many.
+ */
+ PQclear(lastResult);
+ lastResult = result;
- if (PQresultStatus(lastResult) == PGRES_COPY_IN ||
- PQresultStatus(lastResult) == PGRES_COPY_OUT ||
- PQresultStatus(lastResult) == PGRES_COPY_BOTH ||
- PQstatus(conn) == CONNECTION_BAD)
- break;
- }
+ if (PQresultStatus(lastResult) == PGRES_COPY_IN ||
+ PQresultStatus(lastResult) == PGRES_COPY_OUT ||
+ PQresultStatus(lastResult) == PGRES_COPY_BOTH ||
+ PQstatus(conn) == CONNECTION_BAD)
+ break;
+ }
return lastResult;
}
--
2.43.5
From e927202ab617f975e6e078a77a77db4c9d078a2f Mon Sep 17 00:00:00 2001
From: Tom Lane <t...@sss.pgh.pa.us>
Date: Mon, 26 May 2025 14:16:28 -0400
Subject: [PATCH v4 4/5] Silence leakage complaint about postgres_fdw's
InitPgFdwOptions.
Valgrind complains that the PQconninfoOption array returned by libpq
is leaked. We apparently believed that we could suppress that warning
by storing that array's address in a static variable. However, modern
C compilers are bright enough to optimize the static variable away.
We could escalate that arms race by making the variable global.
But on the whole it seems better to revise the code so that it
can free libpq's result properly. The only thing that costs
us is copying the parameter-name keywords; which seems like a
pretty negligible cost in a function that runs at most once per
process.
---
contrib/postgres_fdw/option.c | 33 ++++++++++++---------------------
1 file changed, 12 insertions(+), 21 deletions(-)
diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
index c2f936640bc..d6fa89bad93 100644
--- a/contrib/postgres_fdw/option.c
+++ b/contrib/postgres_fdw/option.c
@@ -21,6 +21,7 @@
#include "libpq/libpq-be.h"
#include "postgres_fdw.h"
#include "utils/guc.h"
+#include "utils/memutils.h"
#include "utils/varlena.h"
/*
@@ -39,12 +40,6 @@ typedef struct PgFdwOption
*/
static PgFdwOption *postgres_fdw_options;
-/*
- * Valid options for libpq.
- * Allocated and filled in InitPgFdwOptions.
- */
-static PQconninfoOption *libpq_options;
-
/*
* GUC parameters
*/
@@ -239,6 +234,7 @@ static void
InitPgFdwOptions(void)
{
int num_libpq_opts;
+ PQconninfoOption *libpq_options;
PQconninfoOption *lopt;
PgFdwOption *popt;
@@ -307,8 +303,8 @@ InitPgFdwOptions(void)
* Get list of valid libpq options.
*
* To avoid unnecessary work, we get the list once and use it throughout
- * the lifetime of this backend process. We don't need to care about
- * memory context issues, because PQconndefaults allocates with malloc.
+ * the lifetime of this backend process. Hence, we'll allocate it in
+ * TopMemoryContext.
*/
libpq_options = PQconndefaults();
if (!libpq_options) /* assume reason for failure is OOM */
@@ -325,19 +321,11 @@ InitPgFdwOptions(void)
/*
* Construct an array which consists of all valid options for
* postgres_fdw, by appending FDW-specific options to libpq options.
- *
- * We use plain malloc here to allocate postgres_fdw_options because it
- * lives as long as the backend process does. Besides, keeping
- * libpq_options in memory allows us to avoid copying every keyword
- * string.
*/
postgres_fdw_options = (PgFdwOption *)
- malloc(sizeof(PgFdwOption) * num_libpq_opts +
- sizeof(non_libpq_options));
- if (postgres_fdw_options == NULL)
- ereport(ERROR,
- (errcode(ERRCODE_FDW_OUT_OF_MEMORY),
- errmsg("out of memory")));
+ MemoryContextAlloc(TopMemoryContext,
+ sizeof(PgFdwOption) * num_libpq_opts +
+ sizeof(non_libpq_options));
popt = postgres_fdw_options;
for (lopt = libpq_options; lopt->keyword; lopt++)
@@ -355,8 +343,8 @@ InitPgFdwOptions(void)
if (strncmp(lopt->keyword, "oauth_", strlen("oauth_")) == 0)
continue;
- /* We don't have to copy keyword string, as described above. */
- popt->keyword = lopt->keyword;
+ popt->keyword = MemoryContextStrdup(TopMemoryContext,
+ lopt->keyword);
/*
* "user" and any secret options are allowed only on user mappings.
@@ -371,6 +359,9 @@ InitPgFdwOptions(void)
popt++;
}
+ /* Done with libpq's output structure. */
+ PQconninfoFree(libpq_options);
+
/* Append FDW-specific options and dummy terminator. */
memcpy(popt, non_libpq_options, sizeof(non_libpq_options));
}
--
2.43.5
From ff53ef5e3751fc0e9ba4b9a327e65df2c02de411 Mon Sep 17 00:00:00 2001
From: Tom Lane <t...@sss.pgh.pa.us>
Date: Mon, 26 May 2025 15:23:32 -0400
Subject: [PATCH v4 5/5] Avoid leak when dblink_connstr_check() fails.
dblink_connect() leaked the already-allocated remoteConn struct
in this case. We can improve matters by just doing that allocation
a bit later. There remain leakage opportunities, such as OOM
inside createNewConnection(), but at least they're not reached
while valgrind'ing the regression tests.
Author: Tom Lane <t...@sss.pgh.pa.us>
Discussion: https://postgr.es/m/2976982.1748049...@sss.pgh.pa.us
---
contrib/dblink/dblink.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index 1e1baf68635..5347bf74087 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -288,15 +288,6 @@ dblink_connect(PG_FUNCTION_ARGS)
else if (PG_NARGS() == 1)
conname_or_str = text_to_cstring(PG_GETARG_TEXT_PP(0));
- if (connname)
- {
- rconn = (remoteConn *) MemoryContextAlloc(TopMemoryContext,
- sizeof(remoteConn));
- rconn->conn = NULL;
- rconn->openCursorCount = 0;
- rconn->newXactForCursor = false;
- }
-
/* first check for valid foreign data server */
connstr = get_connect_string(conname_or_str);
if (connstr == NULL)
@@ -309,6 +300,16 @@ dblink_connect(PG_FUNCTION_ARGS)
if (dblink_we_connect == 0)
dblink_we_connect = WaitEventExtensionNew("DblinkConnect");
+ /* allocate rconn, if needed, as late as possible */
+ if (connname)
+ {
+ rconn = (remoteConn *) MemoryContextAlloc(TopMemoryContext,
+ sizeof(remoteConn));
+ rconn->conn = NULL;
+ rconn->openCursorCount = 0;
+ rconn->newXactForCursor = false;
+ }
+
/* OK to make connection */
conn = libpqsrv_connect(connstr, dblink_we_connect);
--
2.43.5
