Thread

Re: Set notice receiver before libpq connection startup

Chao Li <li.evan.chao@gmail.com> — 2026-05-21T07:26:27Z

> On May 21, 2026, at 13:03, vignesh C <vignesh21@gmail.com> wrote:
> 
> 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

Thanks for your comment. Addressed in v3.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/