On Thu, Nov 27, 2025 at 11:46 AM Fujii Masao <[email protected]> wrote:
>
> On Thu, Nov 27, 2025 at 2:37 AM Kirill Reshke <[email protected]> wrote:
> > Looking at v3 raises two questions for me.
> >
> > First is if we should have a doc notion of which variables ought to be
> > set to what.
>
> Are you suggesting that we document which GUC parameters should be set,
> and to what values, for logical replication? We already have a section on this
> in logical-replication.sgml. Is that sufficient?
>
>
> > Second, how do we actually test that subscription connection options
> > are applied on the subscriber side? Can we have TAP for this  (is is
> > worth the troubles)?
>
> +1 on adding a test. One idea is to enable log_replication_commands via
> the CONNECTION option and then check that the publisher’s log contains
> the message "received replication command: IDENTIFY_SYSTEM".
> There may be a cleaner way to test this, though.

I've added the test and attached it as patch v4-0002.

The test enables log_disconnections via the CONNECTION string and then checks
that the publisher's log contains the expected disconnection message after
the logical replication connection is reestablished.

Regards,

-- 
Fujii Masao
From 159e0c886546f3b12ebd48b57d12c68493d5309e Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Wed, 26 Nov 2025 23:35:44 +0900
Subject: [PATCH v4] Honor GUC settings specified in CREATE SUBSCRIPTION
 CONNECTION.

Prior to v15, GUC settings supplied in the CONNECTION clause of
CREATE SUBSCRIPTION were correctly passed through to
the publisher's walsender. For example:

        CREATE SUBSCRIPTION mysub
            CONNECTION 'options=''-c wal_sender_timeout=1000'''
            PUBLICATION ...

would cause wal_sender_timeout to take effect on the publisher's walsender.

However, commit f3d4019da5d changed the way logical replication
connections are established, forcing the publisher's relevant
GUC settings (datestyle, intervalstyle, extra_float_digits) to
override those provided in the CONNECTION string. As a result,
from v15 through v18, GUC settings in the CONNECTION string were
always ignored.

This regression prevented per-connection tuning of logical replication.
For example, using a shorter timeout for walsender connecting
to a nearby subscriber and a longer one for walsender connecting
to a remote subscriber.

This commit restores the intended behavior by ensuring that
GUC settings in the CONNECTION string are again passed through
and applied by the walsender, allowing per-connection configuration.

Backpatch to v15, where the regression was introduced.

Author: Fujii Masao <[email protected]>
Reviewed-by: Chao Li <[email protected]>
Discussion: 
https://postgr.es/m/CAHGQGwGYV+-abbKwdrM2UHUe-JYOFWmsrs6=qicyjo-j+-w...@mail.gmail.com
Backpatch-through: 15
---
 .../libpqwalreceiver/libpqwalreceiver.c       | 43 ++++++++++++++-----
 1 file changed, 32 insertions(+), 11 deletions(-)

diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c 
b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index 568024ec974..1270d162c88 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -170,17 +170,6 @@ libpqrcv_connect(const char *conninfo, bool logical, bool 
must_use_password,
                /* Tell the publisher to translate to our encoding */
                keys[++i] = "client_encoding";
                vals[i] = GetDatabaseEncodingName();
-
-               /*
-                * Force assorted GUC parameters to settings that ensure that 
the
-                * publisher will output data values in a form that is 
unambiguous to
-                * the subscriber.  (We don't want to modify the subscriber's 
GUC
-                * settings, since that might surprise user-defined code 
running in
-                * the subscriber, such as triggers.)  This should match what 
pg_dump
-                * does.
-                */
-               keys[++i] = "options";
-               vals[i] = "-c datestyle=ISO -c intervalstyle=postgres -c 
extra_float_digits=3";
        }
        keys[++i] = NULL;
        vals[i] = NULL;
@@ -263,6 +252,37 @@ libpqrcv_connect(const char *conninfo, bool logical, bool 
must_use_password,
                PQclear(res);
        }
 
+       /*
+        * Force assorted GUC parameters to settings that ensure that the
+        * publisher will output data values in a form that is unambiguous to 
the
+        * subscriber.  (We don't want to modify the subscriber's GUC settings,
+        * since that might surprise user-defined code running in the 
subscriber,
+        * such as triggers.)  This should match what pg_dump does.
+        */
+       if (logical)
+       {
+               const char *params[] =
+               {"datestyle", "intervalstyle", "extra_float_digits"};
+               const char *values[] = {"ISO", "postgres", "3"};
+
+               for (int j = 0; j < lengthof(params); j++)
+               {
+                       char            sql[100];
+                       PGresult   *res;
+
+                       sprintf(sql, "SET %s = %s", params[j], values[j]);
+                       res = libpqrcv_PQexec(conn->streamConn, sql);
+                       if (PQresultStatus(res) != PGRES_COMMAND_OK)
+                       {
+                               PQclear(res);
+                               *err = psprintf(_("could not set %s: %s"),
+                                                               params[j], 
pchomp(PQerrorMessage(conn->streamConn)));
+                               goto bad_connection;
+                       }
+                       PQclear(res);
+               }
+       }
+
        conn->logical = logical;
 
        return conn;
@@ -434,6 +454,7 @@ libpqrcv_identify_system(WalReceiverConn *conn, TimeLineID 
*primary_tli)
                                                "the primary server: %s",
                                                
pchomp(PQerrorMessage(conn->streamConn)))));
        }
+
        /*
         * IDENTIFY_SYSTEM returns 3 columns in 9.3 and earlier, and 4 columns 
in
         * 9.4 and onwards.
-- 
2.51.2

Attachment: v4-0001-Honor-GUC-settings-specified-in-CREATE-SUBSCRIPTI.patch
Description: Binary data

Attachment: v4-0002-Add-TAP-test-for-GUC-settings-passed-via-CONNECTI.patch
Description: Binary data

From 1a1c412e2d320d82d7a6927e5e6550c554a77b23 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Wed, 26 Nov 2025 23:35:44 +0900
Subject: [PATCH v4] Honor GUC settings specified in CREATE SUBSCRIPTION
 CONNECTION.

Prior to v15, GUC settings supplied in the CONNECTION clause of
CREATE SUBSCRIPTION were correctly passed through to
the publisher's walsender. For example:

        CREATE SUBSCRIPTION mysub
            CONNECTION 'options=''-c wal_sender_timeout=1000'''
            PUBLICATION ...

would cause wal_sender_timeout to take effect on the publisher's walsender.

However, commit f3d4019da5d changed the way logical replication
connections are established, forcing the publisher's relevant
GUC settings (datestyle, intervalstyle, extra_float_digits) to
override those provided in the CONNECTION string. As a result,
from v15 through v18, GUC settings in the CONNECTION string were
always ignored.

This regression prevented per-connection tuning of logical replication.
For example, using a shorter timeout for walsender connecting
to a nearby subscriber and a longer one for walsender connecting
to a remote subscriber.

This commit restores the intended behavior by ensuring that
GUC settings in the CONNECTION string are again passed through
and applied by the walsender, allowing per-connection configuration.

Backpatch to v15, where the regression was introduced.

Author: Fujii Masao <[email protected]>
Reviewed-by: Chao Li <[email protected]>
Discussion: 
https://postgr.es/m/CAHGQGwGYV+-abbKwdrM2UHUe-JYOFWmsrs6=qicyjo-j+-w...@mail.gmail.com
Backpatch-through: 15
---
 .../libpqwalreceiver/libpqwalreceiver.c       | 42 ++++++++++++++-----
 1 file changed, 31 insertions(+), 11 deletions(-)

diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c 
b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index 02f12f29219..02f3ee15c87 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -178,17 +178,6 @@ libpqrcv_connect(const char *conninfo, bool replication, 
bool logical,
                        /* Tell the publisher to translate to our encoding */
                        keys[++i] = "client_encoding";
                        vals[i] = GetDatabaseEncodingName();
-
-                       /*
-                        * Force assorted GUC parameters to settings that 
ensure that the
-                        * publisher will output data values in a form that is 
unambiguous
-                        * to the subscriber.  (We don't want to modify the 
subscriber's
-                        * GUC settings, since that might surprise user-defined 
code
-                        * running in the subscriber, such as triggers.)  This 
should
-                        * match what pg_dump does.
-                        */
-                       keys[++i] = "options";
-                       vals[i] = "-c datestyle=ISO -c intervalstyle=postgres 
-c extra_float_digits=3";
                }
                else
                {
@@ -289,6 +278,37 @@ libpqrcv_connect(const char *conninfo, bool replication, 
bool logical,
                PQclear(res);
        }
 
+       /*
+        * Force assorted GUC parameters to settings that ensure that the
+        * publisher will output data values in a form that is unambiguous to 
the
+        * subscriber.  (We don't want to modify the subscriber's GUC settings,
+        * since that might surprise user-defined code running in the 
subscriber,
+        * such as triggers.)  This should match what pg_dump does.
+        */
+       if (replication && logical)
+       {
+               const char *params[] =
+               {"datestyle", "intervalstyle", "extra_float_digits"};
+               const char *values[] = {"ISO", "postgres", "3"};
+
+               for (int j = 0; j < lengthof(params); j++)
+               {
+                       char            sql[100];
+                       PGresult   *res;
+
+                       sprintf(sql, "SET %s = %s", params[j], values[j]);
+                       res = libpqrcv_PQexec(conn->streamConn, sql);
+                       if (PQresultStatus(res) != PGRES_COMMAND_OK)
+                       {
+                               PQclear(res);
+                               *err = psprintf(_("could not set %s: %s"),
+                                                               params[j], 
pchomp(PQerrorMessage(conn->streamConn)));
+                               goto bad_connection;
+                       }
+                       PQclear(res);
+               }
+       }
+
        conn->logical = logical;
 
        return conn;
-- 
2.51.2

Reply via email to