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

On Thu, Oct 21, 2021 at 11:16 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Wed, Oct 20, 2021 at 8:12 PM Japin Li <japinli@hotmail.com> wrote:
> >
> >
> > On Mon, 18 Oct 2021 at 17:27, Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > > On Mon, Oct 18, 2021 at 1:41 PM Japin Li <japinli@hotmail.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?

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com