Обсуждение: Allow GUC settings in CREATE SUBSCRIPTION CONNECTION to take effect
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=D7pc6st-3A9f1JnOt0qmc+BcBPVzD6fLYisKyAjkGA@mail.gmail.com
--
Fujii Masao
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
bekept, 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
definedin 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
optionthat 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
definedin 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. ButI 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 optionthat 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]. 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.
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 rebalanceruse a higher CPU priority[1]. I had no clue, until now, that that logic got completely broken in PG15 (which wecoincidentally 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 theonly 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