On Sat, Dec 20, 2025 at 12:48 AM Kirill Reshke <[email protected]> wrote:
>
> On Fri, 19 Dec 2025 at 19:42, Fujii Masao <[email protected]> wrote:
> >
> > On Fri, Dec 19, 2025 at 9:30 PM Kirill Reshke <[email protected]> 
> > wrote:
> > > I checked the new TAP test 0002 changes. I am wondering, why are
> > > connection options validated so late in this test? I mean,  we do
> > > ALTER PUBLICATION, then we restart publisher, wait for catchup, check
> > > alter publication, and etc, and only then we look if connection
> > > options are indeed applied?
> >
> > Are you suggesting testing whether the conninfo setting is applied earlier,
> > for example, right after both running ALTER SUBSCRIPTION CONNECTION and
> > confirming that the logical replication connection is re-established?
> > Yeah, that might be better and would also make the test easier to read.
> >
> > Regards,
> >
> > --
> > Fujii Masao
>
> Yes, exactly

OK, I've updated the 0002 patch accordingly.

Regards,

-- 
Fujii Masao
From 38798c9bc99f0499bea754f3d6a51d5244163360 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Wed, 3 Dec 2025 00:59:09 +0900
Subject: [PATCH v7 1/2] 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]>
Reviewed-by: Kirill Reshke <[email protected]>
Reviewed-by: Amit Kapila <[email protected]>
Reviewed-by: Japin Li <[email protected]>
Discussion: 
https://postgr.es/m/CAHGQGwGYV+-abbKwdrM2UHUe-JYOFWmsrs6=qicyjo-j+-w...@mail.gmail.com
Backpatch-through: 15
---
 .../libpqwalreceiver/libpqwalreceiver.c       | 61 ++++++++++++++++++-
 1 file changed, 60 insertions(+), 1 deletion(-)

diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c 
b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index 568024ec974..743f951f330 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -57,6 +57,8 @@ static void libpqrcv_get_senderinfo(WalReceiverConn *conn,
                                                                        char 
**sender_host, int *sender_port);
 static char *libpqrcv_identify_system(WalReceiverConn *conn,
                                                                          
TimeLineID *primary_tli);
+static char *libpqrcv_get_option_from_conninfo(const char *connInfo,
+                                                                               
           const char *keyword);
 static int     libpqrcv_server_version(WalReceiverConn *conn);
 static void libpqrcv_readtimelinehistoryfile(WalReceiverConn *conn,
                                                                                
         TimeLineID tli, char **filename,
@@ -136,6 +138,7 @@ libpqrcv_connect(const char *conninfo, bool logical, bool 
must_use_password,
        const char *keys[6];
        const char *vals[6];
        int                     i = 0;
+       char       *options_val = NULL;
 
        /*
         * Re-validate connection string. The validation already happened at DDL
@@ -167,6 +170,8 @@ libpqrcv_connect(const char *conninfo, bool logical, bool 
must_use_password,
        vals[i] = appname;
        if (logical)
        {
+               char       *opt = NULL;
+
                /* Tell the publisher to translate to our encoding */
                keys[++i] = "client_encoding";
                vals[i] = GetDatabaseEncodingName();
@@ -179,8 +184,13 @@ libpqrcv_connect(const char *conninfo, bool logical, bool 
must_use_password,
                 * the subscriber, such as triggers.)  This should match what 
pg_dump
                 * does.
                 */
+               opt = libpqrcv_get_option_from_conninfo(conninfo, "options");
+               options_val = psprintf("%s -c datestyle=ISO -c 
intervalstyle=postgres -c extra_float_digits=3",
+                                                          (opt == NULL) ? "" : 
opt);
                keys[++i] = "options";
-               vals[i] = "-c datestyle=ISO -c intervalstyle=postgres -c 
extra_float_digits=3";
+               vals[i] = options_val;
+               if (opt != NULL)
+                       pfree(opt);
        }
        keys[++i] = NULL;
        vals[i] = NULL;
@@ -232,6 +242,9 @@ libpqrcv_connect(const char *conninfo, bool logical, bool 
must_use_password,
                        status = PQconnectPoll(conn->streamConn);
        } while (status != PGRES_POLLING_OK && status != PGRES_POLLING_FAILED);
 
+       if (options_val != NULL)
+               pfree(options_val);
+
        if (PQstatus(conn->streamConn) != CONNECTION_OK)
                goto bad_connection_errmsg;
 
@@ -434,6 +447,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.
@@ -466,6 +480,51 @@ libpqrcv_server_version(WalReceiverConn *conn)
        return PQserverVersion(conn->streamConn);
 }
 
+/*
+ * Get the value of the option with the given keyword from the primary
+ * server's conninfo.
+ *
+ * If the option is not found in connInfo, return NULL value.
+ */
+static char *
+libpqrcv_get_option_from_conninfo(const char *connInfo, const char *keyword)
+{
+       PQconninfoOption *opts;
+       char       *option = NULL;
+       char       *err = NULL;
+
+       opts = PQconninfoParse(connInfo, &err);
+       if (opts == NULL)
+       {
+               /* The error string is malloc'd, so we must free it explicitly 
*/
+               char       *errcopy = err ? pstrdup(err) : "out of memory";
+
+               PQfreemem(err);
+               ereport(ERROR,
+                               (errcode(ERRCODE_SYNTAX_ERROR),
+                                errmsg("invalid connection string syntax: %s", 
errcopy)));
+       }
+
+       for (PQconninfoOption *opt = opts; opt->keyword != NULL; ++opt)
+       {
+               /*
+                * If the same option appears multiple times, then the last one 
will
+                * be returned
+                */
+               if (strcmp(opt->keyword, keyword) == 0 && opt->val &&
+                       *opt->val)
+               {
+                       if (option)
+                               pfree(option);
+
+                       option = pstrdup(opt->val);
+               }
+       }
+
+       PQconninfoFree(opts);
+       return option;
+}
+
 /*
  * Start streaming WAL data from given streaming options.
  *
-- 
2.51.2

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

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

Reply via email to