Allow GUC settings in CREATE SUBSCRIPTION CONNECTION to take effect

On Wed, Nov 19, 2025 at 12:59 AM Fujii Masao <masao.fujii@gmail.com> wrote:

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?

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

Regards,

--
Fujii Masao

On Wed, Nov 19, 2025 at 3:22 PM Fujii Masao <masao.fujii@gmail.com> wrote:

Attached is a patch implementing this idea.
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

On Nov 18, 2025, at 23:59, Fujii Masao <masao.fujii@gmail.com> wrote:

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?

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/

On Thu, Nov 20, 2025 at 3:54 PM Chao Li <li.evan.chao@gmail.com> 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!

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

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.

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.

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.

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.

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

On Nov 21, 2025, at 15:47, Fujii Masao <masao.fujii@gmail.com> wrote:

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.

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?

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

On Fri, Nov 21, 2025 at 6:24 PM Chao Li <li.evan.chao@gmail.com> wrote:

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.

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

On Nov 22, 2025, at 00:14, Fujii Masao <masao.fujii@gmail.com> wrote:

On Fri, Nov 21, 2025 at 6:24 PM Chao Li <li.evan.chao@gmail.com> wrote:

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.

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.

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

On Sat, Nov 22, 2025 at 10:31 AM Chao Li <li.evan.chao@gmail.com> wrote:

On Nov 22, 2025, at 00:14, Fujii Masao <masao.fujii@gmail.com> wrote:

On Fri, Nov 21, 2025 at 6:24 PM Chao Li <li.evan.chao@gmail.com> wrote:

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.

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.

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

On Nov 22, 2025, at 22:14, Fujii Masao <masao.fujii@gmail.com> wrote:

On Sat, Nov 22, 2025 at 10:31 AM Chao Li <li.evan.chao@gmail.com> wrote:

On Nov 22, 2025, at 00:14, Fujii Masao <masao.fujii@gmail.com> wrote:

On Fri, Nov 21, 2025 at 6:24 PM Chao Li <li.evan.chao@gmail.com> wrote:

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.

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.

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
<v3-0001-Honor-GUC-settings-specified-in-CREATE-SUBSCRIPTI.patch>

V3 looks good to me.

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.

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

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

On Fri, Nov 21, 2025, 00:47 Fujii Masao <masao.fujii@gmail.com> wrote:

On Thu, Nov 20, 2025 at 3:54 PM Chao Li <li.evan.chao@gmail.com> 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.

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]https://github.com/citusdata/citus/blame/662b7248db2146d33a1a21227271b839355a970a/src/backend/distributed/replication/multi_logical_replication.c#L1510. 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
https://github.com/citusdata/citus/blame/662b7248db2146d33a1a21227271b839355a970a/src/backend/distributed/replication/multi_logical_replication.c#L1510

On Mon, Nov 24, 2025 at 12:52 PM Chao Li <li.evan.chao@gmail.com> wrote:

V3 looks good to me.

Thanks for reviewing the patch!

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.

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

On Mon, Nov 24, 2025 at 2:55 PM Jelte Fennema-Nio <postgres@jeltef.nl> wrote:

On Fri, Nov 21, 2025, 00:47 Fujii Masao <masao.fujii@gmail.com> wrote:

On Thu, Nov 20, 2025 at 3:54 PM Chao Li <li.evan.chao@gmail.com> 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.

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

On Tue, Nov 25, 2025 at 8:32 PM Fujii Masao <masao.fujii@gmail.com> wrote:

I found the following description in logical replication docs, which makes me
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

On Sat, 22 Nov 2025 at 17:14, Fujii Masao <masao.fujii@gmail.com> wrote:

Okay, thanks!

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

Regards,

--
Fujii Masao

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

On Thu, Nov 27, 2025 at 2:37 AM Kirill Reshke <reshkekirill@gmail.com> 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.

Regards,

--
Fujii Masao

On Thu, Nov 27, 2025 at 11:46 AM Fujii Masao <masao.fujii@gmail.com> wrote:

On Thu, Nov 27, 2025 at 2:37 AM Kirill Reshke <reshkekirill@gmail.com> 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

On Thu, Nov 27, 2025 at 2:17 PM Fujii Masao <masao.fujii@gmail.com> wrote:

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.

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

Regards,

--
Fujii Masao

On Tue, 2 Dec 2025 at 11:02, Fujii Masao <masao.fujii@gmail.com> wrote:

On Thu, Nov 27, 2025 at 2:17 PM Fujii Masao <masao.fujii@gmail.com> wrote:

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.

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

Regards,

--
Fujii Masao

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

--
Best regards,
Kirill Reshke

On Tue, Nov 18, 2025 at 9:29 PM Fujii Masao <masao.fujii@gmail.com> wrote:

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.

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.