Set notice receiver before libpq connection startup

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

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.

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

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

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/

On Thu, 21 May 2026 at 09:27, Chao Li <li.evan.chao@gmail.com> wrote:

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.

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

On Fri, May 22, 2026 at 8:33 PM Rafia Sabih <rafia.pghackers@gmail.com> wrote:

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.

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

On May 22, 2026, at 21:42, Fujii Masao <masao.fujii@gmail.com> wrote:

On Fri, May 22, 2026 at 8:33 PM Rafia Sabih <rafia.pghackers@gmail.com> wrote:

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.

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.

Every PG**() function checks if conn is NULL, so I am okay to remove the 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.

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,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

On Fri, May 22, 2026 at 11:56 PM Chao Li <li.evan.chao@gmail.com> wrote:

PFA v4, just removed conn NULL check.

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

Regards,

--
Fujii Masao

On May 22, 2026, at 23:28, Fujii Masao <masao.fujii@gmail.com> wrote:

On Fri, May 22, 2026 at 11:56 PM Chao Li <li.evan.chao@gmail.com> wrote:

PFA v4, just removed conn NULL check.

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

Regards,

--
Fujii Masao

Thanks for pushing and still working hard during the PGConf.

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

On 23.05.26 01:56, Chao Li wrote:

On May 22, 2026, at 23:28, Fujii Masao <masao.fujii@gmail.com> wrote:

On Fri, May 22, 2026 at 11:56 PM Chao Li <li.evan.chao@gmail.com> wrote:

PFA v4, just removed conn NULL check.

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

Regards,

--
Fujii Masao

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)

On May 28, 2026, at 01:56, Peter Eisentraut <peter@eisentraut.org> wrote:

On 23.05.26 01:56, Chao Li wrote:

On May 22, 2026, at 23:28, Fujii Masao <masao.fujii@gmail.com> wrote:

On Fri, May 22, 2026 at 11:56 PM Chao Li <li.evan.chao@gmail.com> wrote:

PFA v4, just removed conn NULL check.

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

Regards,

--
Fujii Masao

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)

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,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

On Thu, May 28, 2026 at 9:40 AM Chao Li <li.evan.chao@gmail.com> wrote:

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)

Thanks for the report!

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.

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

On May 28, 2026, at 13:22, Fujii Masao <masao.fujii@gmail.com> wrote:

On Thu, May 28, 2026 at 9:40 AM Chao Li <li.evan.chao@gmail.com> wrote:

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)

Thanks for the report!

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.

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
<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,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

On Thu, May 28, 2026 at 2:47 PM Chao Li <li.evan.chao@gmail.com> wrote:

Yes, v2 passed “git show --check d359d02a238”.

I've pushed v2 patch. Thanks!

Regards,

--
Fujii Masao