| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
| Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, vignesh C <vignesh21(at)gmail(dot)com> |
| Subject: | Re: Set notice receiver before libpq connection startup |
| Date: | 2026-05-21 02:09:25 |
| Message-ID: | 978D8971-08C3-4AAD-AE8B-976D753C882A@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On May 20, 2026, at 17:19, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>
> On Wed, May 20, 2026 at 4:21 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)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.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-Set-notice-receiver-before-libpq-connection-start.patch | application/octet-stream | 10.2 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Peter Smith | 2026-05-21 03:59:52 | Re: Proposal: Conflict log history table for Logical Replication |
| Previous Message | Peter Smith | 2026-05-21 01:19:18 | Re: Proposal: Conflict log history table for Logical Replication |