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

On Fri, 19 Dec 2025 at 16:55, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> On Wed, Dec 3, 2025 at 2:45 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>>
>> On Tue, Dec 2, 2025 at 8:30 PM Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> >
>> > On Tue, Dec 2, 2025 at 9:08 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>> > > Is it possible that we append the predefined options to the options
>> > > 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.
>> >
>>
>> 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
>> > to avoid the extra round trip?
>> >
>>
>> 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.
>
> 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.

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.