Re: [Bug] Logical Replication failing if the DateStyle is different in Publisher & Subscriber

On Thu, 21 Oct 2021 at 14:04, Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> On Thu, Oct 21, 2021 at 11:16 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>>
>> On Wed, Oct 20, 2021 at 8:12 PM Japin Li <japinli(at)hotmail(dot)com> wrote:
>> >
>> >
>> > On Mon, 18 Oct 2021 at 17:27, Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>> > > On Mon, Oct 18, 2021 at 1:41 PM Japin Li <japinli(at)hotmail(dot)com> wrote:
>> > >
>> > >> I attached v3 patch that set IntervalStyle to 'postgres' when the
>> > >> server backend is walsender, and this problem has gone.
>> > >
>> > >> I test that set IntervalStyle to 'sql_standard' on publisher and
>> > >> 'iso_8601' on subscriber, it works fine.
>> > >
>> > >> Please try v3 patch and let me know if they work as unexpected.
>> > >> Thanks in advance.
>> > >
>> > > I think the idea of setting the standard DateStyle and the
>> > > IntervalStyle on the walsender process looks fine to me. As this will
>> > > avoid extra network round trips as Tom mentioned.
>> >
>> > After some test, I find we also should set the extra_float_digits to avoid
>> > precision lossing.
>>
>> Thank you for the patch!
>>
>> --- a/src/backend/postmaster/postmaster.c
>> +++ b/src/backend/postmaster/postmaster.c
>> @@ -2223,6 +2223,24 @@ retry1:
>> {
>> am_walsender = true;
>> am_db_walsender = true;
>> +
>> + /*
>> + * Force assorted GUC
>> parameters to settings that ensure
>> + * that we'll output data
>> values in a form that is
>> + * unambiguous to the walreceiver.
>> + */
>> + port->guc_options =
>> lappend(port->guc_options,
>> +
>> pstrdup("datestyle"));
>> + port->guc_options =
>> lappend(port->guc_options,
>> +
>> pstrdup("ISO"));
>> + port->guc_options =
>> lappend(port->guc_options,
>> +
>> pstrdup("intervalstyle"));
>> + port->guc_options =
>> lappend(port->guc_options,
>> +
>> pstrdup("postgres"));
>> + port->guc_options =
>> lappend(port->guc_options,
>> +
>> pstrdup("extra_float_digits"));
>> + port->guc_options =
>> lappend(port->guc_options,
>> +
>> pstrdup("3"));
>> }
>>
>> I'm concerned that it sets parameters too early since wal senders end
>> up setting the parameters regardless of logical decoding plugins. It
>> might be better to force the parameters within the plugin for logical
>> replication, pgoutput, in order to avoid affecting other plugins? On
>> the other hand, if we do so, we will need to handle table sync worker
>> cases separately since they copy data via COPY executed by the wal
>> sender process. For example, we can have table sync workers set the
>> parameters.
>
> You mean table sync worker to set over the replication connection
> right? I think that was the first solution where normal workers, as
> well as table sync workers, were setting over the replication
> connection, but Tom suggested that setting on the walsender is a
> better option as we can avoid the network round trip.
>
> If we want to set it over the replication connection then do it for
> both as Japin's first patch is doing, otherwise, I am not seeing any
> big issue in setting it early in the walsender also. I think it is
> good to let walsender always send in the standard format which can be
> understood by other node, no?

+1

I inclined to let walsender set the parameters.

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