Log prefix missing for subscriber log messages received from publisher

On 2025/04/15 13:37, vignesh C wrote:

Hi,

Currently, when a warning is emitted by the publisher, the
corresponding log message does not include the log prefix. This makes
it harder to correlate such messages with other log entries. For
example, in a simulated error scenario where directory removal fails,
the notice message lacks the standard log prefix, as shown below:
2025-03-18 16:44:36.071 IST [196901] LOG: logical replication table
synchronization worker for subscription "sub1", table "t1" has
finished
WARNING: could not remove directory
"pg_replslot/pg_16398_sync_16387_7483106341004194035.tmp"

In this case, the WARNING line does not include the usual timestamp
information, making it harder to trace.

To address this, we can have a custom notice processor for WAL
receiver connections—similar to what's done in the attached patch.

I like this idea.

If this issue also exists other features like dblink or postgres_fdw
connecting to remote PostgreSQL servers, it might be worth applying
a similar improvement there as well.

+notice_processor(void *arg, const char *message)
+{
+	elog(LOG, "%s", message);

Should ereport() be used here instead?

Also, would it be better to add some context before %s, for example,
something like "received message from the primary:"? Without that,
users might mistakenly think the message originated from the local server.

Regards,

--
Fujii Masao
NTT DATA Japan Corporation

On Mon, 14 Jul 2025 at 21:36, Fujii Masao <masao.fujii@oss.nttdata.com> wrote:

On 2025/04/15 13:37, vignesh C wrote:

Hi,

Currently, when a warning is emitted by the publisher, the
corresponding log message does not include the log prefix. This makes
it harder to correlate such messages with other log entries. For
example, in a simulated error scenario where directory removal fails,
the notice message lacks the standard log prefix, as shown below:
2025-03-18 16:44:36.071 IST [196901] LOG: logical replication table
synchronization worker for subscription "sub1", table "t1" has
finished
WARNING: could not remove directory
"pg_replslot/pg_16398_sync_16387_7483106341004194035.tmp"

In this case, the WARNING line does not include the usual timestamp
information, making it harder to trace.

To address this, we can have a custom notice processor for WAL
receiver connections—similar to what's done in the attached patch.

I like this idea.

If this issue also exists other features like dblink or postgres_fdw
connecting to remote PostgreSQL servers, it might be worth applying
a similar improvement there as well.

Included it for dblink and fdw

+notice_processor(void *arg, const char *message)
+{
+       elog(LOG, "%s", message);

Should ereport() be used here instead?

Yes, that is better. Modified it to ereport

Also, would it be better to add some context before %s, for example,
something like "received message from the primary:"? Without that,
users might mistakenly think the message originated from the local server.

Yes, that makes sense. Updated accordingly.

For dblink and postgres_fdw, create the dblink and postgres_fdw setup
and create the below object to raise a notice:
CREATE OR REPLACE FUNCTION emit_notice(msg text) RETURNS int AS $$
BEGIN
RAISE NOTICE '%', msg;
RETURN 1;
END;
$$ LANGUAGE plpgsql;

CREATE VIEW my_notice_view AS SELECT 1 AS value;

CREATE OR REPLACE RULE "_RETURN" AS
ON SELECT TO my_notice_view
DO INSTEAD
SELECT 1 AS value
FROM (SELECT emit_notice('NOTICE from _RETURN rule') AS dummy) s;

-- Scenario to see the notice raised by remote server using dblink
SELECT * FROM dblink('dbname=remotedb user=remoteuser', 'SELECT *
FROM my_notice_view') AS t(value int);

-- Scenario to see the notice raised by remote server using postgres_fdw
IMPORT FOREIGN SCHEMA public LIMIT TO (my_notice_view) FROM SERVER
foreign_server INTO public;

The attached v2 version patch has the changes for the same.

Regards,
Vignesh

On Wed, Jul 16, 2025 at 2:24 AM vignesh C <vignesh21@gmail.com> wrote:

On Mon, 14 Jul 2025 at 21:36, Fujii Masao <masao.fujii@oss.nttdata.com> wrote:

On 2025/04/15 13:37, vignesh C wrote:

Hi,

Currently, when a warning is emitted by the publisher, the
corresponding log message does not include the log prefix. This makes
it harder to correlate such messages with other log entries. For
example, in a simulated error scenario where directory removal fails,
the notice message lacks the standard log prefix, as shown below:
2025-03-18 16:44:36.071 IST [196901] LOG: logical replication table
synchronization worker for subscription "sub1", table "t1" has
finished
WARNING: could not remove directory
"pg_replslot/pg_16398_sync_16387_7483106341004194035.tmp"

In this case, the WARNING line does not include the usual timestamp
information, making it harder to trace.

To address this, we can have a custom notice processor for WAL
receiver connections—similar to what's done in the attached patch.

I like this idea.

If this issue also exists other features like dblink or postgres_fdw
connecting to remote PostgreSQL servers, it might be worth applying
a similar improvement there as well.

Included it for dblink and fdw

Thanks! It's better to submit each change for dblink and postgres_fdw
as separate patches, and first focus on the patch that sets the notice
processor for replication connections.

+notice_processor(void *arg, const char *message)
+{
+       elog(LOG, "%s", message);

Should ereport() be used here instead?

Yes, that is better. Modified it to ereport

Also, would it be better to add some context before %s, for example,
something like "received message from the primary:"? Without that,
users might mistakenly think the message originated from the local server.

Yes, that makes sense. Updated accordingly.

For dblink and postgres_fdw, create the dblink and postgres_fdw setup
and create the below object to raise a notice:
CREATE OR REPLACE FUNCTION emit_notice(msg text) RETURNS int AS $$
BEGIN
RAISE NOTICE '%', msg;
RETURN 1;
END;
$$ LANGUAGE plpgsql;

CREATE VIEW my_notice_view AS SELECT 1 AS value;

CREATE OR REPLACE RULE "_RETURN" AS
ON SELECT TO my_notice_view
DO INSTEAD
SELECT 1 AS value
FROM (SELECT emit_notice('NOTICE from _RETURN rule') AS dummy) s;

-- Scenario to see the notice raised by remote server using dblink
SELECT * FROM dblink('dbname=remotedb user=remoteuser', 'SELECT *
FROM my_notice_view') AS t(value int);

-- Scenario to see the notice raised by remote server using postgres_fdw
IMPORT FOREIGN SCHEMA public LIMIT TO (my_notice_view) FROM SERVER
foreign_server INTO public;

The attached v2 version patch has the changes for the same.

Thanks for updating the patch!

+ /* Trim trailing newline for cleaner logs */
+ size_t len = strlen(message);
+
+ if (len > 0 && message[len - 1] == '\n')
+ ereport(LOG,
+ errmsg("Received message from publisher: %.*s",
+ (int) (len - 1), message));

Do we really need to trim the trailing newline? Are there actual
WARNING/NOTICE/INFO messages that include an unnecessary newline?

"Received" should be lowercase, i.e., "received" since the primary log message
should start with a lowercase letter, according to the Error Message
Style Guide [1]https://www.postgresql.org/docs/devel/error-style-guide.html.

The phrase "from publisher" could be misleading, since the sender might be
a primary in physical replication, not necessarily a logical publisher.
Maybe something like "received message via replication" would be clearer?
I'd like to hear others' thoughts on a better wording.

Regards,

[1]: https://www.postgresql.org/docs/devel/error-style-guide.html

--
Fujii Masao

On Wed, 16 Jul 2025 at 09:09, Fujii Masao <masao.fujii@gmail.com> wrote:

Included it for dblink and fdw

Thanks! It's better to submit each change for dblink and postgres_fdw
as separate patches, and first focus on the patch that sets the notice
processor for replication connections.

Modified

+notice_processor(void *arg, const char *message)
+{
+       elog(LOG, "%s", message);

Should ereport() be used here instead?

Yes, that is better. Modified it to ereport

Also, would it be better to add some context before %s, for example,
something like "received message from the primary:"? Without that,
users might mistakenly think the message originated from the local server.

Yes, that makes sense. Updated accordingly.

For dblink and postgres_fdw, create the dblink and postgres_fdw setup
and create the below object to raise a notice:
CREATE OR REPLACE FUNCTION emit_notice(msg text) RETURNS int AS $$
BEGIN
RAISE NOTICE '%', msg;
RETURN 1;
END;
$$ LANGUAGE plpgsql;

CREATE VIEW my_notice_view AS SELECT 1 AS value;

CREATE OR REPLACE RULE "_RETURN" AS
ON SELECT TO my_notice_view
DO INSTEAD
SELECT 1 AS value
FROM (SELECT emit_notice('NOTICE from _RETURN rule') AS dummy) s;

-- Scenario to see the notice raised by remote server using dblink
SELECT * FROM dblink('dbname=remotedb user=remoteuser', 'SELECT *
FROM my_notice_view') AS t(value int);

-- Scenario to see the notice raised by remote server using postgres_fdw
IMPORT FOREIGN SCHEMA public LIMIT TO (my_notice_view) FROM SERVER
foreign_server INTO public;

The attached v2 version patch has the changes for the same.

Thanks for updating the patch!

+ /* Trim trailing newline for cleaner logs */
+ size_t len = strlen(message);
+
+ if (len > 0 && message[len - 1] == '\n')
+ ereport(LOG,
+ errmsg("Received message from publisher: %.*s",
+ (int) (len - 1), message));

Do we really need to trim the trailing newline? Are there actual
WARNING/NOTICE/INFO messages that include an unnecessary newline?

If we don't trim the trailing newline, an extra blank line will appear
after the message is printed, like this:
2025-07-16 12:44:20.076 IST [534376] LOG: logical replication table
synchronization worker for subscription "sub1", table "t2" has started
2025-07-16 12:44:20.294 IST [534374] LOG: received message via
replication: WARNING: could not remove directory
"pg_replslot/pg_16396_sync_16384_7527574647116269356.tmp"

2025-07-16 12:44:20.294 IST [534374] LOG: logical replication table
synchronization worker for subscription "sub1", table "t1" has
finished

This occurs because a newline is appended to the message in
pqBuildErrorMessage3, which is called when the message is received in
pqGetErrorNotice3:
...
}
appendPQExpBufferChar(msg, '\n');
if (verbosity != PQERRORS_TERSE)
...

"Received" should be lowercase, i.e., "received" since the primary log message
should start with a lowercase letter, according to the Error Message
Style Guide [1].

Modified

The phrase "from publisher" could be misleading, since the sender might be
a primary in physical replication, not necessarily a logical publisher.
Maybe something like "received message via replication" would be clearer?
I'd like to hear others' thoughts on a better wording.

Modified the message to "received message via replication." Will
update it further if we get any better wording.

The attached v3 version patch has the changes for the same.

Regards,
Vignesh

On 2025/07/16 19:45, vignesh C wrote:

If we don't trim the trailing newline, an extra blank line will appear
after the message is printed, like this:
2025-07-16 12:44:20.076 IST [534376] LOG: logical replication table
synchronization worker for subscription "sub1", table "t2" has started
2025-07-16 12:44:20.294 IST [534374] LOG: received message via
replication: WARNING: could not remove directory
"pg_replslot/pg_16396_sync_16384_7527574647116269356.tmp"

2025-07-16 12:44:20.294 IST [534374] LOG: logical replication table
synchronization worker for subscription "sub1", table "t1" has
finished

This occurs because a newline is appended to the message in
pqBuildErrorMessage3, which is called when the message is received in
pqGetErrorNotice3:
...
}
appendPQExpBufferChar(msg, '\n');
if (verbosity != PQERRORS_TERSE)
...

You're right, the message text passed to the notice processor includes
a trailing newline. I confirmed this is documented in the PQsetNoticeProcessor
docs [1]https://www.postgresql.org/docs/devel/libpq-notice-processing.html, so I agree it's reasonable to trim the newline for cleaner log output.

Regarding the current implementation:
+       /* Trim trailing newline for cleaner logs */
+       size_t          len = strlen(message);
+
+       if (len > 0 && message[len - 1] == '\n')
+               ereport(LOG,
+                               errmsg("received message via replication: %.*s",
+                                          (int) (len - 1), message));
+       else
+               ereport(LOG,
+                               errmsg("received message via replication: %s", message));

To avoid repeating ereport(), how about refactoring it like this?
I think it's simpler and easier to read:

---------------------------------------
/*
* Custom notice processor for replication connection.
*/
static void
notice_processor(void *arg, const char *message)
{
int len;

/*
* Trim the trailing newline from the message text passed to the notice
* processor, as it always includes one, to produce cleaner log output.
*/
len = strlen(message);
if (len > 0 && message[len - 1] == '\n')
len--;

ereport(LOG,
errmsg("received message via replication: %.*s",
len, message));
}
---------------------------------------

Also, I suggest adding the prototype for notice_processor() in libpqwalreceiver.c:

---------------------------------------
/* Prototypes for private functions */
static void notice_processor(void *arg, const char *message);
static char *stringlist_to_identifierstr(PGconn *conn, List *strings);
---------------------------------------

Currently, notice_processor() is placed just before libpqrcv_connect(),
but I think it would be better to move it before stringlist_to_identifierstr(),
since the libpqwalreceiver.c seems to follow a structure where private
functions come after the libpqrcv_* functions.

+ PQsetNoticeProcessor(conn->streamConn, notice_processor, NULL);

It might be helpful to add a comment explaining the purpose, such as:

Set a custom notice processor so that notice, warning, and similar messages
received via the replication connection are logged in our preferred style,
instead of just being printed to stderr.

Thought?

The phrase "from publisher" could be misleading, since the sender might be
a primary in physical replication, not necessarily a logical publisher.
Maybe something like "received message via replication" would be clearer?
I'd like to hear others' thoughts on a better wording.

Modified the message to "received message via replication." Will
update it further if we get any better wording.

+1

Regards,

[1]: https://www.postgresql.org/docs/devel/libpq-notice-processing.html

--
Fujii Masao
NTT DATA Japan Corporation

On Wed, 16 Jul 2025 at 21:30, Fujii Masao <masao.fujii@oss.nttdata.com> wrote:

On 2025/07/16 19:45, vignesh C wrote:

If we don't trim the trailing newline, an extra blank line will appear
after the message is printed, like this:
2025-07-16 12:44:20.076 IST [534376] LOG: logical replication table
synchronization worker for subscription "sub1", table "t2" has started
2025-07-16 12:44:20.294 IST [534374] LOG: received message via
replication: WARNING: could not remove directory
"pg_replslot/pg_16396_sync_16384_7527574647116269356.tmp"

2025-07-16 12:44:20.294 IST [534374] LOG: logical replication table
synchronization worker for subscription "sub1", table "t1" has
finished

This occurs because a newline is appended to the message in
pqBuildErrorMessage3, which is called when the message is received in
pqGetErrorNotice3:
...
}
appendPQExpBufferChar(msg, '\n');
if (verbosity != PQERRORS_TERSE)
...

You're right, the message text passed to the notice processor includes
a trailing newline. I confirmed this is documented in the PQsetNoticeProcessor
docs [1], so I agree it's reasonable to trim the newline for cleaner log output.

Regarding the current implementation:
+       /* Trim trailing newline for cleaner logs */
+       size_t          len = strlen(message);
+
+       if (len > 0 && message[len - 1] == '\n')
+               ereport(LOG,
+                               errmsg("received message via replication: %.*s",
+                                          (int) (len - 1), message));
+       else
+               ereport(LOG,
+                               errmsg("received message via replication: %s", message));

To avoid repeating ereport(), how about refactoring it like this?
I think it's simpler and easier to read:

---------------------------------------
/*
* Custom notice processor for replication connection.
*/
static void
notice_processor(void *arg, const char *message)
{
int len;

/*
* Trim the trailing newline from the message text passed to the notice
* processor, as it always includes one, to produce cleaner log output.
*/
len = strlen(message);
if (len > 0 && message[len - 1] == '\n')
len--;

ereport(LOG,
errmsg("received message via replication: %.*s",
len, message));
}
---------------------------------------

Looks good, modified

Also, I suggest adding the prototype for notice_processor() in libpqwalreceiver.c:

---------------------------------------
/* Prototypes for private functions */
static void notice_processor(void *arg, const char *message);
static char *stringlist_to_identifierstr(PGconn *conn, List *strings);
---------------------------------------

Currently, notice_processor() is placed just before libpqrcv_connect(),
but I think it would be better to move it before stringlist_to_identifierstr(),
since the libpqwalreceiver.c seems to follow a structure where private
functions come after the libpqrcv_* functions.

Modified

+ PQsetNoticeProcessor(conn->streamConn, notice_processor, NULL);

It might be helpful to add a comment explaining the purpose, such as:

Set a custom notice processor so that notice, warning, and similar messages
received via the replication connection are logged in our preferred style,
instead of just being printed to stderr.

Modified

The attached v4 version patch has the changes for the same.

Regards,
Vignesh

Hi,

Shouldn't we be using a notice receiver rather than a notice processor?

--
Álvaro Herrera

On Thu, 17 Jul 2025 at 11:18, Álvaro Herrera <alvherre@kurilemu.de> wrote:

Hi,

Shouldn't we be using a notice receiver rather than a notice processor?

I saw the following comment in code regarding PQsetNoticeProcessor
should be deprecated:
/*
* The default notice message receiver just gets the standard notice text
* and sends it to the notice processor. This two-level setup exists
* mostly for backwards compatibility; perhaps we should deprecate use of
* PQsetNoticeProcessor?
*/

So I changed it to PQsetNoticeReceiver. The attached v5 version patch
has the changes for the same.

Regards,
Vignesh

On 2025/07/17 17:05, vignesh C wrote:

On Thu, 17 Jul 2025 at 11:18, Álvaro Herrera <alvherre@kurilemu.de> wrote:

Hi,

Shouldn't we be using a notice receiver rather than a notice processor?

I saw the following comment in code regarding PQsetNoticeProcessor
should be deprecated:
/*
* The default notice message receiver just gets the standard notice text
* and sends it to the notice processor. This two-level setup exists
* mostly for backwards compatibility; perhaps we should deprecate use of
* PQsetNoticeProcessor?
*/

So I changed it to PQsetNoticeReceiver.

+1

As a side note, I'd like to clarify in the source comments or documentation
that PQsetNoticeProcessor() exists mainly for backward compatibility,
and PQsetNoticeReceiver() should be preferred. But that's a separate topic
from this patch.

The attached v5 version patch
has the changes for the same.

Thanks for updating the patches!

+static void notice_receiver(void *arg, const PGresult *result);

For consistency with the typedef for PQnoticeReceiver, it would be better
to name the argument "res" instead of "result".

+ * Set a custom notice receiver so that NOTICEs, WARNINGs, and similar

The "s" in "NOTICEs" and "WARNINGs" isn't needed.

+	 * Trim the trailing newline from the message text passed to the notice
+	 * receiver, as it always includes one, to produce cleaner log output.

"message text passed to the notice receiver" should be changed to
"message text returned by PQresultErrorMessage()"?

Regards,

--
Fujii Masao
NTT DATA Japan Corporation

On 2025/07/17 23:34, Fujii Masao wrote:

The attached v5 version patch
has the changes for the same.

Thanks for updating the patches!

The current patches add nearly identical notice_receiver functions
in multiple places such as libpqwalreceiver.c and elsewhere. To avoid
duplicating the same logic, could we define a shared notice receiver
function in a common file, like libpq-be-fe-helpers.h, and use it in
all three locations?

--------------------
static inline void
libpqsrv_notice_receiver(void *arg, const PGresult *res)
{
char *message;
int len;
char *prefix = (char *) arg;

/*
* Trim the trailing newline from the message text returned from
* PQresultErrorMessage(), as it always includes one, to produce
* cleaner log output.
*/
message = PQresultErrorMessage(res);
len = strlen(message);
if (len > 0 && message[len - 1] == '\n')
len--;

ereport(LOG,
errmsg("received message %s: %.*s", prefix, len, message));
}
--------------------

Regards,

--
Fujii Masao
NTT DATA Japan Corporation

On 2025-Jul-18, Fujii Masao wrote:

The current patches add nearly identical notice_receiver functions
in multiple places such as libpqwalreceiver.c and elsewhere. To avoid
duplicating the same logic, could we define a shared notice receiver
function in a common file, like libpq-be-fe-helpers.h, and use it in
all three locations?

I like the idea of reducing duplication. I don't like the "prefix"
as proposed though, because it's not going to work well for translation
(string building) -- I'd rather pass the entire phrase from caller, so
that the translator has one piece to translate which lives in the module
that emits it. I think I'd do something like

ereport(LOG,
errmsg_internal("%s: %.*s",
_(prefix), len, message));

and see to it that each caller uses gettext_noop() around the string
they pass as "arg", for gettext to collect.

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/

On 2025/07/18 1:15, Álvaro Herrera wrote:

On 2025-Jul-18, Fujii Masao wrote:

The current patches add nearly identical notice_receiver functions
in multiple places such as libpqwalreceiver.c and elsewhere. To avoid
duplicating the same logic, could we define a shared notice receiver
function in a common file, like libpq-be-fe-helpers.h, and use it in
all three locations?

I like the idea of reducing duplication. I don't like the "prefix"
as proposed though, because it's not going to work well for translation
(string building) -- I'd rather pass the entire phrase from caller, so
that the translator has one piece to translate which lives in the module
that emits it. I think I'd do something like

ereport(LOG,
errmsg_internal("%s: %.*s",
_(prefix), len, message));

and see to it that each caller uses gettext_noop() around the string
they pass as "arg", for gettext to collect.

Agreed. Thanks!

Regards,

--
Fujii Masao
NTT DATA Japan Corporation

On Fri, 18 Jul 2025 at 04:46, Fujii Masao <masao.fujii@oss.nttdata.com> wrote:

On 2025/07/18 1:15, Álvaro Herrera wrote:

On 2025-Jul-18, Fujii Masao wrote:

The current patches add nearly identical notice_receiver functions
in multiple places such as libpqwalreceiver.c and elsewhere. To avoid
duplicating the same logic, could we define a shared notice receiver
function in a common file, like libpq-be-fe-helpers.h, and use it in
all three locations?

I like the idea of reducing duplication. I don't like the "prefix"
as proposed though, because it's not going to work well for translation
(string building) -- I'd rather pass the entire phrase from caller, so
that the translator has one piece to translate which lives in the module
that emits it. I think I'd do something like

ereport(LOG,
errmsg_internal("%s: %.*s",
_(prefix), len, message));

and see to it that each caller uses gettext_noop() around the string
they pass as "arg", for gettext to collect.

Agreed. Thanks!

The attached v6 version patch has the changes for these comments.

Regards,
Vignesh

On 2025-Jul-18, vignesh C wrote:

diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index 304f3c20f83..c1ce6f33436 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -625,6 +625,9 @@ connect_pg_server(ForeignServer *server, UserMapping *user)
server->servername),
errdetail_internal("%s", pchomp(PQerrorMessage(conn)))));

+		PQsetNoticeReceiver(conn, libpqsrv_notice_receiver,
+							"received message via remote connection");
+

These need to be:

PQsetNoticeReceiver(conn, libpqsrv_notice_receiver,
gettext_noop("received message via remote connection"));

so that the strings are picked up by gettext.

Also, I don't see why there are three patches here instead of just one.

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/

On Sat, 19 Jul 2025 at 01:39, Álvaro Herrera <alvherre@kurilemu.de> wrote:

On 2025-Jul-18, vignesh C wrote:

diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index 304f3c20f83..c1ce6f33436 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -625,6 +625,9 @@ connect_pg_server(ForeignServer *server, UserMapping *user)
server->servername),
errdetail_internal("%s", pchomp(PQerrorMessage(conn)))));

+             PQsetNoticeReceiver(conn, libpqsrv_notice_receiver,
+                                                     "received message via remote connection");
+

These need to be:

PQsetNoticeReceiver(conn, libpqsrv_notice_receiver,
gettext_noop("received message via remote connection"));

so that the strings are picked up by gettext.

Modified

Also, I don't see why there are three patches here instead of just one.

Earlier we thought to commit replication changes function firstlly and
then commit dblink and fdw changes, but now that we are using a common
notice receiver function. I feel it can be a single patch. Merged the
patches. The attached v7 version patch has the changes for the same.

Regards,
Vignesh

On 2025/07/19 15:45, vignesh C wrote:

Also, I don't see why there are three patches here instead of just one.

Earlier we thought to commit replication changes function firstlly and
then commit dblink and fdw changes, but now that we are using a common
notice receiver function. I feel it can be a single patch.

Yes, I initially suggested splitting the patches for that reason,
but I agree it's better to merge them into a single patch now that
we're using a shared custom notice receiver.

Merged the
patches. The attached v7 version patch has the changes for the same.

Thanks for updating the patch! It looks good to me, except for one minor point:

static inline PGresult *libpqsrv_get_result(PGconn *conn, uint32 wait_event_info);
+static inline void libpqsrv_notice_receiver(void *arg, const PGresult *res);

This prototype is only needed if the function is used earlier in libpq-be-fe-helpers.h,
but that's not the case here, so I don't think it's necessary.

Unless there are objections, I'll remove that prototype and commit the patch.

Regards,

--
Fujii Masao
NTT DATA Japan Corporation

On 2025-Jul-21, Fujii Masao wrote:

Thanks for updating the patch! It looks good to me, except for one minor point:

static inline PGresult *libpqsrv_get_result(PGconn *conn, uint32 wait_event_info);
+static inline void libpqsrv_notice_receiver(void *arg, const PGresult *res);

This prototype is only needed if the function is used earlier in libpq-be-fe-helpers.h,
but that's not the case here, so I don't think it's necessary.

Unless there are objections, I'll remove that prototype and commit the patch.

LGTM.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Debido a que la velocidad de la luz es mucho mayor que la del sonido,
algunas personas nos parecen brillantes un minuto antes
de escuchar las pelotudeces que dicen." (Roberto Fontanarrosa)

On Mon, Jul 21, 2025 at 5:51 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:

On 2025-Jul-21, Fujii Masao wrote:

Thanks for updating the patch! It looks good to me, except for one minor point:

static inline PGresult *libpqsrv_get_result(PGconn *conn, uint32 wait_event_info);
+static inline void libpqsrv_notice_receiver(void *arg, const PGresult *res);

This prototype is only needed if the function is used earlier in libpq-be-fe-helpers.h,
but that's not the case here, so I don't think it's necessary.

Unless there are objections, I'll remove that prototype and commit the patch.

LGTM.

I've pushed the patch. Thanks!

Regards,

--
Fujii Masao

On 2025/07/22 14:29, Fujii Masao wrote:

On Mon, Jul 21, 2025 at 5:51 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:

On 2025-Jul-21, Fujii Masao wrote:

Thanks for updating the patch! It looks good to me, except for one minor point:

static inline PGresult *libpqsrv_get_result(PGconn *conn, uint32 wait_event_info);
+static inline void libpqsrv_notice_receiver(void *arg, const PGresult *res);

This prototype is only needed if the function is used earlier in libpq-be-fe-helpers.h,
but that's not the case here, so I don't think it's necessary.

Unless there are objections, I'll remove that prototype and commit the patch.

LGTM.

I've pushed the patch. Thanks!

The buildfarm member indri reported the following error, which seems related to
the recent changes in dblink. I'll investigate this later.

https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=indri&amp;dt=2025-07-22%2006%3A11%3A16&amp;stg=make-contrib

/Library/Developer/CommandLineTools/usr/bin/make -C intagg all
make[1]: Nothing to be done for `all'.
/opt/local/bin/clang -Wno-ignored-attributes -fno-strict-aliasing -fwrapv -fexcess-precision=standard -mno-outline-atomics -Wno-unused-command-line-argument -Wno-compound-token-split-by-macro -Wno-format-truncation -O2 -I. -I. -I../../src/include -I/opt/local/include -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX15.5.sdk -I/opt/local/include/libxml2 -I/opt/local/include -I/opt/local/include -I/opt/local/include -I/opt/local/include -flto=thin -emit-llvm -c -o fuzzystrmatch.bc fuzzystrmatch.c
Undefined symbols for architecture arm64:
"_libintl_gettext", referenced from:
_libpqsrv_notice_receiver in dblink.o
ld: symbol(s) not found for architecture arm64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[1]: *** [dblink.dylib] Error 1
make: *** [all-dblink-recurse] Error 2
make: *** Waiting for unfinished jobs....

Regards,

--
Fujii Masao
NTT DATA Japan Corporation