Subject: Allow GUC settings in CREATE SUBSCRIPTION CONNECTION to take effect

From: Fujii Masao <[email-masked]>
Date: Wed, 19 Nov 2025 00:59:00 +0900

Hi,

In logical replication, any GUC settings specified in the CONNECTION clause of
CREATE SUBSCRIPTION are currently ignored. For example:

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

The wal_sender_timeout value here has no effect.

This is inconvenient when different logical replication walsenders need
different settings - e.g., a small wal_sender_timeout for walsender
connecting to a nearby subscriber and a larger one for walsender
connecting to a distant subscriber. Right now, it's not easy for users
to control such per-connection behavior.

The reason of thid limitation is that libpqrcv_connect() always overwrites
the options connection parameter as follows:

        keys[++i] = "options";
        vals[i] = "-c datestyle=ISO -c intervalstyle=postgres -c
extra_float_digits=3";

This wipes out any user-specified GUCs in the CONNECTION string.
Physical replication does not have this problem because it does not overwrite
options, so GUCs in primary_conninfo are honored.

To remove this restriction, how about switching to issuing SET commands for
datestyle, intervalstyle, and extra_float_digits after the connection
is established,
similar to what postgres_fdw does, instead of forcing them into options?
That would allow user-specified GUC settings in CREATE SUBSCRIPTION to
take effect.

This overwrite behavior was introduced in commit f3d4019da5d and chosen mainly
to avoid extra network round trips according to the discussion [1].
While SET commands would add a round trip, it only happens at
connection startup,
which is infrequent - so the overhead seems negligible.

Thoughts?

Regards,

[1] https://postgr.es/m/CAFF0-CF=[email-masked]

-- 
Fujii Masao

---

From: Fujii Masao <[email-masked]>
Date: Wed, 19 Nov 2025 15:22:28 +0900

> which is infrequent - so the overhead seems negligible.

Attached is a patch implementing this idea.
I've also added it to the next CommitFest.

Regards,

-- 
Fujii Masao

---

From: Fujii Masao <[email-masked]>
Date: Wed, 19 Nov 2025 19:38:08 +0900

> I've also added it to the next CommitFest.

I've fixed the compiler warning issue.
Attached is an updated version of the patch.

Regards,

-- 
Fujii Masao

---

From: Chao Li <[email-masked]>
Date: Thu, 20 Nov 2025 14:53:49 +0800

> On Nov 18, 2025, at 23:59, Fujii Masao <[email-masked]> wrote:
> 
> Hi,
> 

Before this patch, all user specified options are silently discarded, now all user specified options expect the 3 will be kept, will that expose a hold where user badly specifies some option that may break logical replication? If that’s true, then we need to parse user specified options and do some verifications.

I just reviewed v2, and got some comments:

1.
```
+			char		sql[100];
```

Hardcode 100 here doesn’t look good. If you decide to keep, I won’t have a strong objection.

2
```
+		const char *params[] =
+		{"datestyle", "intervalstyle", "extra_float_digits", NULL};
+		const char *values[] = {"ISO", "postgres", "3", NULL};
```

Nit: we don’t need to have a NULL terminator element. We can use lengthof() macro to get array length. lengthof() is defined in c.h.

3. To minimize the network round-trip, maybe we can combine the 3 set into a single statement?

4. The commit message:
```
This commit removes the restriction by changing how logical replication
connections are established so that GUC settings in the CONNECTION string
are properly passed through to and uesd by the walsender. This enables
```

This is a little bit inaccurate, all user specified settings expected the 3 ones being overwritten will be honored.

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

---

From: Fujii Masao <[email-masked]>
Date: Fri, 21 Nov 2025 16:47:05 +0900

On Thu, Nov 20, 2025 at 3:54 PM Chao Li <[email-masked]> wrote:
> Before this patch, all user specified options are silently discarded,

The GUC settings in CREATE SUBSCRIPTION were honored up through v14;
the behavior changed in commit f3d4019da5d, so some might view this
as a regression.

> now all user specified options expect the 3 will be kept, will that expose a hold where user badly specifies some option that may break logical replication? If that’s true, then we need to parse user specified options and do some verifications.
>

Yeah, I agree that if certain GUCs can break logical replication,
we should enforce "safe" values, just as we currently do for datestyle.
And if any other GUCs can cause the issue, they could affect
postgres_fdw etc, so the fix would need to be broader.

> I just reviewed v2, and got some comments:

Thanks for the review!

> Hardcode 100 here doesn’t look good. If you decide to keep, I won’t have a strong objection.

I think hardcoding 100 here is sufficient, since the queries built on
that buffer are fixed and clearly fit within that limit.

> Nit: we don’t need to have a NULL terminator element. We can use lengthof() macro to get array length. lengthof() is defined in c.h.

Okay, I'll adjust the patch accordingly.

> 3. To minimize the network round-trip, maybe we can combine the 3 set into a single statement?

As for the extra network round trip, I still doubt it will matter
in practice given that it happens only at replication connection startup.

> This is a little bit inaccurate, all user specified settings expected the 3 ones being overwritten will be honored.

Are you suggesting that, because datestyle and the other two parameters
specified in CONNECTION aren(t actually applied by the walsender,
the commit message should explicitly mention that not all parameters
from CONNECTION are used?

Regards,

-- 
Fujii Masao

---

From: Chao Li <[email-masked]>
Date: Fri, 21 Nov 2025 17:23:10 +0800

> On Nov 21, 2025, at 15:47, Fujii Masao <[email-masked]> wrote:
> 
>> 
>> 4. The commit message:
>> ```

No, what I was thinking is that, we could combine the three set statement into one, like:

```
Set a = 1; set b = 2; set c = 3;
```
So that sends a single statement to publisher server, that reduces round-trip from 3 times to one time.

Best regards,

---

From: Fujii Masao <[email-masked]>
Date: Sat, 22 Nov 2025 01:14:04 +0900

> So that sends a single statement to publisher server, that reduces round-trip from 3 times to one time.

I see the point about combining the three SET commands to reduce round trips,
but I think the current approach in the patch (i.e., issuing a separate
SET command for each parameter) is sufficient. I still don't think
the additional round trip during replication connection startup is
a real concern. This approach is also consistent with what postgres_fdw
and pg_dump already do.

Regards,

-- 
Fujii Masao

---

From: Chao Li <[email-masked]>
Date: Sat, 22 Nov 2025 09:31:06 +0800

> On Nov 22, 2025, at 00:14, Fujii Masao <[email-masked]> wrote:
> 

No problem. I don’t have a strong option here. I just saw you mentioned overhead of round trip and thought to improve. But I agree that overhead is tiny, not a real concern.

Best regards,

---

From: Fujii Masao <[email-masked]>
Date: Sat, 22 Nov 2025 23:14:27 +0900

> No problem. I don’t have a strong option here. I just saw you mentioned overhead of round trip and thought to improve. But I agree that overhead is tiny, not a real concern.

Okay, thanks!

I've updated the patch to use lengthof() as you suggested.
The revised version is attached.

Regards,

-- 
Fujii Masao

---

From: Chao Li <[email-masked]>
Date: Mon, 24 Nov 2025 11:51:58 +0800

> On Nov 22, 2025, at 22:14, Fujii Masao <[email-masked]> wrote:
> 
> <v3-0001-Honor-GUC-settings-specified-in-CREATE-SUBSCRIPTI.patch>

V3 looks good to me.

> postgres_fdw etc, so the fix would need to be broader.

Just want to clarify if you mean you will handle this in a future patch?

Best regards,

---

From: Jelte Fennema-Nio <[email-masked]>
Date: Sun, 23 Nov 2025 22:54:54 -0700

> the behavior changed in commit f3d4019da5d, so some might view this

FWIW, I definitely view it as a regression. I used this in citus to make
the logical replication sender of the shard rebalancer use a higher CPU
priority[1]. I had no clue, until now, that that logic got completely
broken in PG15 (which we coincidentally added support for in the same
release).

I'm not entirely sure if it's worth a backpatch. This citus feature
probably isn't the most critical. So if that's the only usecase in the wild
that got broken, then that might be fine. But I at least wanted to share
that others (i.e. me) have used this feature.

[1]:
https://github.com/citusdata/citus/blame/662b7248db2146d33a1a21227271b839355a970a/src/backend/distributed/replication/multi_logical_replication.c#L1510

---

From: Fujii Masao <[email-masked]>
Date: Tue, 25 Nov 2025 20:30:35 +0900

> V3 looks good to me.

Thanks for reviewing the patch!

> Just want to clarify if you mean you will handle this in a future patch?

I don't currently know of any other parameters that must be forced for
logical replication or postgres_fdw. But if we identify any, I'm happy to
review a patch that adds the necessary handling.

Regards,

-- 
Fujii Masao

---

From: Fujii Masao <[email-masked]>
Date: Tue, 25 Nov 2025 20:32:51 +0900

On Mon, Nov 24, 2025 at 2:55 PM Jelte Fennema-Nio <[email-masked]> wrote:
> FWIW, I definitely view it as a regression. I used this in citus to make the logical replication sender of the shard rebalancer use a higher CPU priority[1]. I had no clue, until now, that that logic got completely broken in PG15 (which we coincidentally added support for in the same release).

Thanks for sharing this!

> I'm not entirely sure if it's worth a backpatch. This citus feature probably isn't the most critical. So if that's the only usecase in the wild that got broken, then that might be fine. But I at least wanted to share that others (i.e. me) have used this feature.

I found the following description in logical replication docs, which makes me
start thinking that the patch would need to be backpatched.

-----------------
If the role does not trust all table owners, include options=-crow_security=off
in the connection string
https://www.postgresql.org/docs/devel/logical-replication-security.html
-----------------

Regards,

-- 
Fujii Masao

---

From: Fujii Masao <[email-masked]>
Date: Thu, 27 Nov 2025 01:33:25 +0900

> start thinking that the patch would need to be backpatched.

I've prepared patches for the older branches as well and attached them.

Regards,

-- 
Fujii Masao
From 159e0c886546f3b12ebd48b57d12c68493d5309e Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email-masked]>
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:

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-masked]>
Reviewed-by: Chao Li <[email-masked]>
Discussion: https://postgr.es/m/CAHGQGwGYV+-abbKwdrM2UHUe-JYOFWmsrs6=[email-masked]
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

From 1a1c412e2d320d82d7a6927e5e6550c554a77b23 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email-masked]>
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:

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-masked]>
Reviewed-by: Chao Li <[email-masked]>
Discussion: https://postgr.es/m/CAHGQGwGYV+-abbKwdrM2UHUe-JYOFWmsrs6=[email-masked]
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

---

From: Kirill Reshke <[email-masked]>
Date: Wed, 26 Nov 2025 20:37:39 +0300

> The revised version is attached.

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.
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)?

-- 
Best regards,
Kirill Reshke

---

From: Fujii Masao <[email-masked]>
Date: Thu, 27 Nov 2025 11:46:10 +0900

> First is if we should have a doc notion of which variables ought to be

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?

> are applied on the subscriber side? Can we have TAP for this  (is is

+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.

Regards,

-- 
Fujii Masao

---

From: Fujii Masao <[email-masked]>
Date: Thu, 27 Nov 2025 14:17:15 +0900

> 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,

 
+	/*
 
 	return conn;

 
+	/*
 
 	return conn;
-- 
2.51.2

---

From: Fujii Masao <[email-masked]>
Date: Tue, 2 Dec 2025 15:02:28 +0900

> the logical replication connection is reestablished.

Barring any objections, I will commit the v4-001 patch and backpatch it to v15.

Regards,

-- 
Fujii Masao

---

From: Kirill Reshke <[email-masked]>
Date: Tue, 2 Dec 2025 11:37:08 +0500

> Barring any objections, I will commit the v4-001 patch and backpatch it to v15.

Sure lgtm. I actually checked v4-0001, but did not got any objections

-- 
Best regards,
Kirill Reshke

---

From: Amit Kapila <[email-masked]>
Date: Tue, 2 Dec 2025 17:38:03 +0530

> That would allow user-specified GUC settings in CREATE SUBSCRIPTION to

Is it possible that we append the predefined options to the options
given by the user to avoid extra round-trip?

-- 
With Regards,
Amit Kapila.

---

From: Fujii Masao <[email-masked]>
Date: Tue, 2 Dec 2025 23:59:55 +0900

> given by the user to avoid extra round-trip?

One idea is to add a function, similar to libpqrcv_get_dbname_from_conninfo()
in libpqwalreceiver.c, that extracts the options string from the conninfo,
to append the required fixed settings, and then to use the combined string as
the value of the options parameter. Do you think implementing this is worthwhile
to avoid the extra round trip?

Regards,

-- 
Fujii Masao

---

From: Amit Kapila <[email-masked]>
Date: Wed, 3 Dec 2025 11:15:06 +0530

On Tue, Dec 2, 2025 at 8:30 PM Fujii Masao <[email-masked]> wrote:
> the value of the options parameter.
>

Yes, but libpqrcv_get_dbname_from_conninfo() is an exposed function,
we can implement something internal for libpqwalreceiver.c like
conninfo_add_defaults() present in fe-connect.c.

> Do you think implementing this is worthwhile

I think so. Today, we have three variables, tomorrow there could be
more such variables as well and apart from avoiding extra round trips,
the idea to append options to provided options (if any) sounds more
logical to me.

-- 
With Regards,
Amit Kapila.

---

From: Fujii Masao <[email-masked]>
Date: Fri, 19 Dec 2025 16:55:19 +0900

> the idea to append options to provided options (if any) sounds more

OK, I've implemented the idea discussed upthread. The patch updates
libpqrcv_connect() so that, for logical replication, it extracts the options
string from the conninfo, appends the required fixed settings, and passes
the combined string as the options parameter to libpq.

For example, if the conninfo specifies -c wal_sender_timeout=10s,
the resulting options value becomes:

        -c wal_sender_timeout=10s -c datestyle=ISO -c
intervalstyle=postgres -c extra_float_digits=3.

Patch attached.

Regards,

-- 
Fujii Masao
From f8e0dcdea82d08d65e30224da2339f58de8c5fc3 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email-masked]>
Date: Wed, 3 Dec 2025 00:59:09 +0900
Subject: [PATCH v5] Honor GUC settings specified in CREATE SUBSCRIPTION
 CONNECTION.

Author: Fujii Masao <[email-masked]>
Reviewed-by: Chao Li <[email-masked]>
Reviewed-by: Kirill Reshke <[email-masked]>
Reviewed-by: Amit Kapila <[email-masked]>
Discussion: https://postgr.es/m/CAHGQGwGYV+-abbKwdrM2UHUe-JYOFWmsrs6=[email-masked]
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

---

From: Japin Li <[email-masked]>
Date: Fri, 19 Dec 2025 18:07:24 +0800

> intervalstyle=postgres -c extra_float_digits=3.

Thanks for the patch — that was my oversight.

LGTM with one small suggestion:

The comment says: "If the option is not found in connInfo, return NULL value."

Since the parameter is named `keyword`, I'd suggest: "If the keyword is not found in connInfo, return NULL."

This keeps terminology consistent with the function signature.

-- 
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

---

From: Fujii Masao <[email-masked]>
Date: Fri, 19 Dec 2025 20:05:31 +0900

Thanks for the review!

> This keeps terminology consistent with the function signature.

I think "the option with the given keyword" is more precise than just
"the keyword".
That said, simply using "the option" also seems sufficient in this context...

Regarding 0002 patch, I found that it caused a CI failure, so I’ve updated
the patch to fix that. The revised patch is attached.

Regards,

-- 
Fujii Masao
From a29e80e158b043a76f00383705ee9c27f3f864a5 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email-masked]>
Date: Wed, 3 Dec 2025 00:59:09 +0900
Subject: [PATCH v6 1/2] Honor GUC settings specified in CREATE SUBSCRIPTION
 CONNECTION.

Reviewed-by: Japin Li <[email-masked]>

 	/*

 
+/*

---

From: Kirill Reshke <[email-masked]>
Date: Fri, 19 Dec 2025 17:30:36 +0500

> the patch to fix that. The revised patch is attached.

Hi!
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?

-- 
Best regards,
Kirill Reshke

---

From: Fujii Masao <[email-masked]>
Date: Fri, 19 Dec 2025 23:42:14 +0900

> 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

---

From: Kirill Reshke <[email-masked]>
Date: Fri, 19 Dec 2025 20:48:39 +0500

> Yeah, that might be better and would also make the test easier to read.

Yes, exactly

-- 
Best regards,
Kirill Reshke

---

From: Amit Kapila <[email-masked]>
Date: Tue, 23 Dec 2025 10:35:32 +0530

> intervalstyle=postgres -c extra_float_digits=3.

Thanks, the idea looks good now. One minor comment:
  }
+
  /*

This appears to be an unnecessary addition in the patch.

-- 
With Regards,
Amit Kapila.

---

From: Chao Li <[email-masked]>
Date: Tue, 23 Dec 2025 14:17:34 +0800

> On Dec 19, 2025, at 19:05, Fujii Masao <[email-masked]> wrote:
> 
> <v6-0001-PG15-PG16-Honor-GUC-settings-specified-in-CREATE-SUBSCRIPTI.txt><v6-0002-Add-TAP-test-for-GUC-settings-passed-via-CONNECTI.patch><v6-0001-Honor-GUC-settings-specified-in-CREATE-SUBSCRIPTI.patch>

A few more comments on v6:

1 - 0001
```
+			if (opt != NULL)
+				pfree(opt);
```

From what I learned from previous reviews, pfree() is safe to handle NULL, we can omit the NULL check, which makes the code simpler. This comment applies to multiple places.

2. I still think we should verify options extracted from conninfo. In the 0002’s tap script, if I make the following change:
```
 $node_subscriber->safe_psql('postgres',
-       "ALTER SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr options=''-c log_statement_stats=on'''"
+       "ALTER SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr options=''-c log_statement_stats=on-c wal_sender_timeout=1000'’'"
```

Notice, there is no space between the second “-c” and “on”, meaning that a user passes an invalid options. Then the test will get stuck forever, which would never happen before this patch.

Best regards,

---

From: Japin Li <[email-masked]>
Date: Tue, 23 Dec 2025 16:13:06 +0800

> From what I learned from previous reviews, pfree() is safe to handle NULL, we can omit the NULL check, which makes the code simpler. This comment applies to multiple places.

I think we cannot omit it here. We have two pfree()s, one for frontend and
one for backend. IIUC, the frontend pfree() is safe to handle NULL, but the
backend pfree() is not.

> Notice, there is no space between the second “-c” and “on”, meaning that a user passes an invalid options. Then the test will get stuck forever, which would never happen before this patch.

How to verify the options? Checking for -c isn't enough because valid flags
can still carry invalid values. For example, -c log_statement_stats=aa is
syntactically correct but will fail at runtime.

---

From: Chao Li <[email-masked]>
Date: Tue, 23 Dec 2025 16:45:56 +0800

> On Dec 23, 2025, at 16:13, Japin Li <[email-masked]> wrote:
> 

Ahh… You are right. I just traced a backend pfree(), and it ends up calling GetMemoryChunkMethodID(const void *point), and the function doesn’t handle NULL pointer.

Best regards,

---

From: Fujii Masao <[email-masked]>
Date: Thu, 25 Dec 2025 16:00:04 +0900

> 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-masked]>
Date: Wed, 3 Dec 2025 00:59:09 +0900
Subject: [PATCH v7 1/2] Honor GUC settings specified in CREATE SUBSCRIPTION
 CONNECTION.

 
 	/*

 
+/*

---

From: Fujii Masao <[email-masked]>
Date: Thu, 25 Dec 2025 16:01:28 +0900

Thanks for the review!

> This appears to be an unnecessary addition in the patch.

The added blank line is due to pgindent, so I didn't exclude it from the patch.
I'm not sure what the preferred practice is for handling pgindent changes
in older branches, but in general it seems better to keep the code clean
and pgindent-compliant even there. Thoughts?

Regards,

-- 
Fujii Masao