Commit
112faf1378e added custom notice receivers for replication,
postgres_fdw, and dblink so that remote NOTICE, WARNING, and similar
messages are reported via ereport(). However, those notice receivers were
installed only after libpqsrv_connect() and libpqsrv_connect_params()
returned, by which point libpq connection startup had already completed.
As a result, messages emitted during connection establishment could be
missed.
This commit fixes the issue by splitting libpqsrv_connect() and
libpqsrv_connect_params() into separate start and complete phases:
libpqsrv_connect_start(), libpqsrv_connect_params_start(), and
libpqsrv_connect_complete(). This allows callers to perform
per-connection setup, such as installing a notice receiver, after the
connection has been started but before startup completes.
Note that callers of libpqsrv_connect_start() and
libpqsrv_connect_params_start() must still call
libpqsrv_connect_complete(), even if the start function returns NULL, so
that any external FDs reserved during startup are released properly.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Vignesh C <vignesh21@gmail.com>
Reviewed-by: Rafia Sabih <rafia.pghackers@gmail.com>
Discussion: https://postgr.es/m/
A2B8B7DE-C119-492F-A9FA-
14CF86849777@gmail.com
dblink_we_get_conn = WaitEventExtensionNew("DblinkGetConnect");
/* OK to make connection */
- conn = libpqsrv_connect(connstr, dblink_we_get_conn);
+ conn = libpqsrv_connect_start(connstr);
+ PQsetNoticeReceiver(conn, libpqsrv_notice_receiver,
+ "received message via remote connection");
+ libpqsrv_connect_complete(conn, dblink_we_get_conn);
if (PQstatus(conn) == CONNECTION_BAD)
{
errdetail_internal("%s", msg)));
}
- PQsetNoticeReceiver(conn, libpqsrv_notice_receiver,
- "received message via remote connection");
-
dblink_security_check(conn, NULL, connstr);
if (PQclientEncoding(conn) != GetDatabaseEncoding())
PQsetClientEncoding(conn, GetDatabaseEncodingName());
}
/* OK to make connection */
- conn = libpqsrv_connect(connstr, dblink_we_connect);
+ conn = libpqsrv_connect_start(connstr);
+ if (conn != NULL)
+ PQsetNoticeReceiver(conn, libpqsrv_notice_receiver,
+ "received message via remote connection");
+ libpqsrv_connect_complete(conn, dblink_we_connect);
if (PQstatus(conn) == CONNECTION_BAD)
{
errdetail_internal("%s", msg)));
}
- PQsetNoticeReceiver(conn, libpqsrv_notice_receiver,
- "received message via remote connection");
-
/* check password actually used if not superuser */
dblink_security_check(conn, connname, connstr);
const char **keywords;
const char **values;
char *appname;
+ PGconn *start_conn;
construct_connection_params(server, user, &keywords, &values, &appname);
pgfdw_we_connect = WaitEventExtensionNew("PostgresFdwConnect");
/* OK to make connection */
- conn = libpqsrv_connect_params(keywords, values,
- false, /* expand_dbname */
- pgfdw_we_connect);
+ start_conn = libpqsrv_connect_params_start(keywords, values,
+ /* expand_dbname = */ false);
+ PQsetNoticeReceiver(start_conn, libpqsrv_notice_receiver,
+ "received message via remote connection");
+ libpqsrv_connect_complete(start_conn, pgfdw_we_connect);
+ conn = start_conn;
if (!conn || PQstatus(conn) != CONNECTION_OK)
ereport(ERROR,
server->servername),
errdetail_internal("%s", pchomp(PQerrorMessage(conn)))));
- PQsetNoticeReceiver(conn, libpqsrv_notice_receiver,
- "received message via remote connection");
-
/* Perform post-connection security checks. */
pgfdw_security_check(keywords, values, user, conn);
conn = palloc0_object(WalReceiverConn);
conn->streamConn =
- libpqsrv_connect_params(keys, vals,
- /* expand_dbname = */ true,
- WAIT_EVENT_LIBPQWALRECEIVER_CONNECT);
+ libpqsrv_connect_params_start(keys, vals,
+ /* expand_dbname = */ true);
+ PQsetNoticeReceiver(conn->streamConn, libpqsrv_notice_receiver,
+ "received message via replication");
+ libpqsrv_connect_complete(conn->streamConn,
+ WAIT_EVENT_LIBPQWALRECEIVER_CONNECT);
if (options_val != NULL)
pfree(options_val);
errhint("Target server's authentication method must be changed, or set password_required=false in the subscription parameters.")));
}
- PQsetNoticeReceiver(conn->streamConn, libpqsrv_notice_receiver,
- "received message via replication");
-
/*
* Set always-secure search path for the cases where the connection is
* used to run SQL queries, so malicious users can't get control.
static inline void libpqsrv_connect_prepare(void);
-static inline void libpqsrv_connect_internal(PGconn *conn, uint32 wait_event_info);
+static inline void libpqsrv_connect_complete(PGconn *conn, uint32 wait_event_info);
static inline PGresult *libpqsrv_get_result_last(PGconn *conn, uint32 wait_event_info);
static inline PGresult *libpqsrv_get_result(PGconn *conn, uint32 wait_event_info);
+/*
+ * Start a connection using PQconnectStart().
+ *
+ * The returned connection has not yet completed its startup sequence. Callers
+ * may perform per-connection setup, such as installing a notice receiver,
+ * before calling libpqsrv_connect_complete().
+ *
+ * Callers must call libpqsrv_connect_complete(), even if this function returns
+ * NULL, because libpqsrv_connect_prepare() may already have reserved an
+ * external FD that must be released.
+ */
+static inline PGconn *
+libpqsrv_connect_start(const char *conninfo)
+{
+ libpqsrv_connect_prepare();
+
+ return PQconnectStart(conninfo);
+}
/*
* PQconnectdb() wrapper that reserves a file descriptor and processes
static inline PGconn *
libpqsrv_connect(const char *conninfo, uint32 wait_event_info)
{
- PGconn *conn = NULL;
+ PGconn *conn;
- libpqsrv_connect_prepare();
-
- conn = PQconnectStart(conninfo);
+ conn = libpqsrv_connect_start(conninfo);
- libpqsrv_connect_internal(conn, wait_event_info);
+ libpqsrv_connect_complete(conn, wait_event_info);
return conn;
}
+/*
+ * Start a connection using PQconnectStartParams().
+ *
+ * See libpqsrv_connect_start() for the resource-lifetime rules.
+ */
+static inline PGconn *
+libpqsrv_connect_params_start(const char *const *keywords,
+ const char *const *values,
+ int expand_dbname)
+{
+ libpqsrv_connect_prepare();
+
+ return PQconnectStartParams(keywords, values, expand_dbname);
+}
+
/*
* Like libpqsrv_connect(), except that this is a wrapper for
* PQconnectdbParams().
int expand_dbname,
uint32 wait_event_info)
{
- PGconn *conn = NULL;
+ PGconn *conn;
- libpqsrv_connect_prepare();
+ conn = libpqsrv_connect_params_start(keywords, values, expand_dbname);
- conn = PQconnectStartParams(keywords, values, expand_dbname);
-
- libpqsrv_connect_internal(conn, wait_event_info);
+ libpqsrv_connect_complete(conn, wait_event_info);
return conn;
}
/*
* PQfinish() wrapper that additionally releases the reserved file descriptor.
*
- * It is allowed to call this with a NULL pgconn iff NULL was returned by
- * libpqsrv_connect*.
+ * It is allowed to call this with NULL only when the external FD reservation
+ * has already been released, for example after calling
+ * libpqsrv_connect_complete() with a NULL connection.
*/
static inline void
libpqsrv_disconnect(PGconn *conn)
* already released it). This rule makes it easier to write PG_CATCH()
* handlers for this facility's users.
*
- * See also libpqsrv_connect_internal().
+ * See also libpqsrv_connect_complete().
*/
if (conn == NULL)
return;
}
-/* internal helper functions follow */
+/* lower-level connection helper functions follow */
/*
}
/*
- * Helper function for all connection establishment functions.
+ * Complete a connection started by libpqsrv_connect_start() or
+ * libpqsrv_connect_params_start().
*/
static inline void
-libpqsrv_connect_internal(PGconn *conn, uint32 wait_event_info)
+libpqsrv_connect_complete(PGconn *conn, uint32 wait_event_info)
{
/*
* With conn == NULL libpqsrv_disconnect() wouldn't release the FD. So do
projects / thirdparty / postgresql.git / commitdiff
