Thread
-
Re: Set notice receiver before libpq connection startup
vignesh C <vignesh21@gmail.com> — 2026-05-21T05:03:09Z
On Thu, 21 May 2026 at 07:40, Chao Li <li.evan.chao@gmail.com> wrote: > > > > > On May 20, 2026, at 17:19, Fujii Masao <masao.fujii@gmail.com> wrote: > > > > On Wed, May 20, 2026 at 4:21 PM Chao Li <li.evan.chao@gmail.com> wrote: > >> > >> Hi, > >> > >> While testing “Log remote NOTICE, WARNING, and similar messages using ereport()”, I noticed that libpqsrv_notice_receiver is only installed after libpqsrv_connect() finishes. As a result, NOTICE messages generated during connection establishment are missed by ereport() and are still printed to stderr. > >> > >> To reproduce the issue, I created a separate database called remotedb and defined a login trigger that emits a NOTICE message: > >> ``` > >> CREATE DATABASE remotedb; > >> > >> \c remotedb > >> > >> CREATE OR REPLACE FUNCTION repro_login_notice() > >> RETURNS event_trigger > >> LANGUAGE plpgsql AS $$ > >> BEGIN > >> RAISE NOTICE 'startup notice from remotedb login trigger'; > >> END; > >> $$; > >> > >> CREATE EVENT TRIGGER repro_login_notice_trg > >> ON login > >> EXECUTE FUNCTION repro_login_notice(); > >> > >> ALTER EVENT TRIGGER repro_login_notice_trg ENABLE ALWAYS; > >> ``` > >> > >> Then, from another database: > >> ``` > >> evantest=# create extension dblink; > >> CREATE EXTENSION > >> evantest=# SELECT dblink_connect('host=127.0.0.1 port=5432 dbname=remotedb user=chaol sslmode=disable gssencmode=disable'); > >> dblink_connect > >> ---------------- > >> OK > >> (1 row) > >> ``` > >> > >> In the system log, the NOTICE message is printed directly: > >> ``` > >> 2026-05-20 13:02:19.350 CST [24909] STATEMENT: SELECT dblink_connect('host=127.0.0.1 port=5432 dbname=remotedb user=chaol sslmode=disable gssencmode=disable'); > >> NOTICE: startup notice from remotedb login trigger > >> ``` > >> > >> To fix that, I think we should install libpqsrv_notice_receiver before libpqsrv_connect_internal(). In the attached patch, I added two helpers: libpqsrv_connect_with_notice_receiver() and libpqsrv_connect_params_with_notice_receiver(). > >> > >> With the fix, the NOTICE message now looks like this: > >> ``` > >> 2026-05-20 14:44:49.296 CST [45567] LOG: received message via remote connection: NOTICE: startup notice from remotedb login trigger > >> 2026-05-20 14:44:49.296 CST [45567] STATEMENT: SELECT dblink_connect('host=127.0.0.1 port=5432 dbname=remotedb user=chaol sslmode=disable gssencmode=disable'); > >> ``` > >> > >> Please see the attached patch for details. > > > > Thanks for the report and patch! > > > > I'd prefer to avoid adding notice-receiver-specific wrappers such as > > libpqsrv_connect_with_notice_receiver(). Instead, how about splitting > > libpqsrv_connect() into two steps: libpqsrv_connect_start(), which performs > > libpqsrv_connect_prepare() and PQconnectStart(), and > > libpqsrv_connect_complete(), which runs libpqsrv_connect_internal()? > > > > With this approach, callers could invoke PQsetNoticeReceiver() after > > libpqsrv_connect_start() returns but before libpqsrv_connect_complete() is > > called. This would allow startup-time notices to be handled correctly without > > introducing a dedicated wrapper function. > > > > Compared to the *_with_notice_receiver() approach, this design is more > > general because it exposes the phase between PQconnectStart() and connection > > completion. It could also support other kinds of per-connection setup in the > > future, not just notice receiver installation. Thought? > > > > Regards, > > > > -- > > Fujii Masao > > The idea sounds good to me, so v2 is implemented following that idea. > > A few things I want to point out abut v2: > > * Since libpqsrv_connect_complete() would only wrap libpqsrv_connect_internal(), I just renamed libpqsrv_connect_internal() to libpqsrv_connect_complete(). > * Since libpqsrv_connect() is now split into two phases, libpqsrv_connect_complete() must be called even if conn is NULL, because it may need to call ReleaseExternalFD(). I mentioned this in the header comment of libpqsrv_connect_start(). > * In the postgres_fdw/connection.c change, I introduced a new local variable, start_conn, to keep the original logic unchanged. Because there is a PG_TRY/PG_CATCH block, conn is assigned only after libpqsrv_connect_complete() finishes successfully. Thanks for reporting this issue. I was able to reproduce the issue with the steps provided and your patch fixes the issue. Few comments: 1) No need of conn variable here, we can directly return PQconnectStart(conninfo) in this function: +static inline PGconn * +libpqsrv_connect_start(const char *conninfo) +{ + PGconn *conn = NULL; + + libpqsrv_connect_prepare(); + + conn = PQconnectStart(conninfo); + + return conn; +} 2) Similarly here too: +static inline PGconn * +libpqsrv_connect_params_start(const char *const *keywords, + const char *const *values, + int expand_dbname) { PGconn *conn = NULL; libpqsrv_connect_prepare(); - conn = PQconnectStart(conninfo); - - libpqsrv_connect_internal(conn, wait_event_info); + conn = PQconnectStartParams(keywords, values, expand_dbname); return conn; } Regards, Vignesh