Subject: Set notice receiver before libpq connection startup

From: Chao Li <[email-masked]>
Date: Wed, 20 May 2026 15:21:08 +0800

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.

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

---

From: Fujii Masao <[email-masked]>
Date: Wed, 20 May 2026 18:19:51 +0900

> evantest=# SELECT dblink_connect('host=127.0.0.1 port=5432 dbname=remotedb user=chaol sslmode=disable gssencmode=disable');

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

---

From: Chao Li <[email-masked]>
Date: Thu, 21 May 2026 10:09:25 +0800

> On May 20, 2026, at 17:19, Fujii Masao <[email-masked]> wrote:
> 

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,

---

From: vignesh C <[email-masked]>
Date: Thu, 21 May 2026 10:33:09 +0530

> * 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…

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

---

From: Chao Li <[email-masked]>
Date: Thu, 21 May 2026 15:26:27 +0800

> On May 21, 2026, at 13:03, vignesh C <[email-masked]> wrote:
> 

Thanks for your comment. Addressed in v3.

Best regards,

---

From: Rafia Sabih <[email-masked]>
Date: Fri, 22 May 2026 13:33:21 +0200

On Thu, 21 May 2026 at 09:27, Chao Li <[email-masked]> wrote:

> >>>> 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:
> >>>> ```
> >>>> evantest=# SELECT dblink_connect('host=127.0.0.1 port=5432
> dbname=remotedb user=chaol sslmode=disable gssencmode=disable');
> >>>> dblink_connect
> >>>> ----------------
> >>>> OK
> >>>> (1 row)
> >>>> ```
> >>>>
> >>>> 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');
> >>>> 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().
> >>>>
> >>>> 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');
> >>>> ```
> >>>>
> >> * 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.
> >
> Here are my two cents,
we need not to check conn is null here
+ conn = libpqsrv_connect_start(connstr);
+ if (conn != NULL)
+ PQsetNoticeReceiver(conn, libpqsrv_notice_receiver,
+ "received message via remote connection");
because it is done so in  PQsetNoticeReceiver anyway. Also, since there is
no else here so it doesn't make sense more, because if it is null then also
we will just continue with the next function call.

Another point is, in pg_connect_server I don't get the value of adding
another PGConn variable start_conn, can't we use conn itself...?
I hope this helps.
-- 
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

---

From: Fujii Masao <[email-masked]>
Date: Fri, 22 May 2026 22:42:00 +0900

On Fri, May 22, 2026 at 8:33 PM Rafia Sabih <[email-masked]> wrote:
> because it is done so in  PQsetNoticeReceiver anyway. Also, since there is no else here so it doesn't make sense more, because if it is null then also we will just continue with the next function call.

Yes, but I'm fine with the current code in the patch. That code makes
the intent explicit, i.e., install the notice receiver only when a connection
object actually exists. That said, I'm also OK with simply calling
PQsetNoticeReceiver() without that check.

> Another point is, in pg_connect_server I don't get the value of adding another PGConn variable start_conn, can't we use conn itself...?
> I hope this helps.

Not only connect_pg_server() but libpqsrv_connect_complete() has
a PG_TRY/PG_CATCH block. So if start_conn were not used, an error thrown
in libpqsrv_connect_complete() could cause the current connection (conn) to
be cleaned up twice unexpectedly: once in libpqsrv_connect_complete() and
again in connect_pg_server(). I guess that's why Chao introduced start_conn.

Regards,

-- 
Fujii Masao

---

From: Chao Li <[email-masked]>
Date: Fri, 22 May 2026 22:55:18 +0800

> On May 22, 2026, at 21:42, Fujii Masao <[email-masked]> wrote:
> 

Every PG**() function checks if conn is NULL, so I am okay to remove the check.

> again in connect_pg_server(). I guess that's why Chao introduced start_conn.

Exactly. With introducing start_conn, when libpqsrv_connect_complete() raises an error, conn is still NULL, so that PG_CATCH clause won’t cleanup conn, which keeps the same behavior as the old code.

PFA v4, just removed conn NULL check.

Best regards,

---

From: Fujii Masao <[email-masked]>
Date: Sat, 23 May 2026 00:28:40 +0900

> PFA v4, just removed conn NULL check.

Thanks for updating the patch! I've pushed it.

Regards,

-- 
Fujii Masao

---

From: Chao Li <[email-masked]>
Date: Sat, 23 May 2026 07:56:32 +0800

> On May 22, 2026, at 23:28, Fujii Masao <[email-masked]> wrote:
> 

Thanks for pushing and still working hard during the PGConf.

Best regards,

---

From: Peter Eisentraut <[email-masked]>
Date: Wed, 27 May 2026 19:56:50 +0200

> Thanks for pushing and still working hard during the PGConf.

The committed patch violates the .gitattributes whitespace rules:

git show --check 06a5c3cdef02

contrib/postgres_fdw/connection.c:651: indent with spaces.
+                                                   /* expand_dbname = 
*/ false);

It is unfortunate that pgindent produces this layout that contradicts 
the git configuration.  (The current formatting also doesn't look like 
what I would produce in an editor, so I think git is right here.)

Maybe we could reformat this slightly to avoid that?  (unless someone 
wants to try to fix pgindent)

---

From: Chao Li <[email-masked]>
Date: Thu, 28 May 2026 08:39:48 +0800

> On May 28, 2026, at 01:56, Peter Eisentraut <[email-masked]> wrote:
> 
> +                                                   /* expand_dbname = */ false);
> 
> It is unfortunate that pgindent produces this layout that contradicts the git configuration.  (The current formatting also doesn't look like what I would produce in an editor, so I think git is right here.)
> 
> Maybe we could reformat this slightly to avoid that?  (unless someone wants to try to fix pgindent)
> 
> 

Ah, I was not aware of the whitespace rule. I think it was not pgindent; I made that change manually. I added “=” because I thought it might read more fluently.

Attached is a fix for that.

Best regards,

---

From: Fujii Masao <[email-masked]>
Date: Thu, 28 May 2026 14:22:21 +0900

> > Maybe we could reformat this slightly to avoid that?  (unless someone wants to try to fix pgindent)

Thanks for the report!

> Attached is a fix for that.

Thanks for the patch!

I suspect the whitespace issue was caused by pgindent that I ran
before committing the patch.

-     /* expand_dbname = */ false);
+    false /* expand_dbname */ );

I think "/* expand_dbname = */ false" looks better. libpqwalreceiver.c
also uses that comment style. So how about the attached v2 patch, which
reformats the comment accordingly?

After applying the v2 patch, I confirmed that neither "git show --check ..."
nor pgindent reports any issues.

Regards,

-- 
Fujii Masao

---

From: Chao Li <[email-masked]>
Date: Thu, 28 May 2026 13:46:45 +0800

> On May 28, 2026, at 13:22, Fujii Masao <[email-masked]> wrote:
> 
> <v2-0001-Fix-an-indentation-problem-in-postgres_fdw-connec.patch>

Oh, I misunderstood the problem. Nice to learn a new thing, I never knew git show check before. Yes, v2 passed “git show --check d359d02a238”.

Best regards,

---

From: Fujii Masao <[email-masked]>
Date: Thu, 28 May 2026 21:55:46 +0900

On Thu, May 28, 2026 at 2:47 PM Chao Li <[email-masked]> wrote:
> Yes, v2 passed “git show --check d359d02a238”.

I've pushed v2 patch. Thanks!

Regards,

-- 
Fujii Masao