Thread

  1. 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