> On May 21, 2026, at 13:03, vignesh C <[email protected]> wrote: > > On Thu, 21 May 2026 at 07:40, Chao Li <[email protected]> wrote: >> >> >> >>> On May 20, 2026, at 17:19, Fujii Masao <[email protected]> wrote: >>> >>> On Wed, May 20, 2026 at 4:21 PM Chao Li <[email protected]> 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
Thanks for your comment. Addressed in v3. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
v3-0001-Set-notice-receiver-before-libpq-connection-start.patch
Description: Binary data
