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

On Thu, Oct 14, 2021 at 3:48 PM Sadhuprasad Patro <b.sadhu@gmail.com> wrote:

Hi All,

Publisher 'DateStyle' is set as "SQL, MDY", whereas in Subscriber as
"SQL, DMY", the logical replication is not working...

From Publisher:
postgres=# INSERT INTO calendar VALUES ('07-18-1036', '1'), ('05-15-1135', '1');
INSERT 0 2

Getting below error in the subscriber log file,
2021-10-14 00:59:23.067 PDT [38262] ERROR: date/time field value out
of range: "07/18/1036"
2021-10-14 00:59:23.067 PDT [38262] HINT: Perhaps you need a
different "datestyle" setting.

Is this an expected behavior?

Looks like a problem to me, I think for fixing this, on logical
replication connection always set subscriber's DateStlyle, with that
the walsender will always send the data in the same DateStyle that
worker understands and then we are good.

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

On Thu, 14 Oct 2021 at 19:49, Dilip Kumar <dilipbalaut@gmail.com> wrote:

On Thu, Oct 14, 2021 at 3:48 PM Sadhuprasad Patro <b.sadhu@gmail.com> wrote:

Hi All,

Publisher 'DateStyle' is set as "SQL, MDY", whereas in Subscriber as
"SQL, DMY", the logical replication is not working...

From Publisher:
postgres=# INSERT INTO calendar VALUES ('07-18-1036', '1'), ('05-15-1135', '1');
INSERT 0 2

Getting below error in the subscriber log file,
2021-10-14 00:59:23.067 PDT [38262] ERROR: date/time field value out
of range: "07/18/1036"
2021-10-14 00:59:23.067 PDT [38262] HINT: Perhaps you need a
different "datestyle" setting.

Is this an expected behavior?

Looks like a problem to me, I think for fixing this, on logical
replication connection always set subscriber's DateStlyle, with that
the walsender will always send the data in the same DateStyle that
worker understands and then we are good.

Right! Attached fix it.

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

On Sat, 16 Oct 2021 at 22:42, Japin Li <japinli@hotmail.com> wrote:

On Thu, 14 Oct 2021 at 19:49, Dilip Kumar <dilipbalaut@gmail.com> wrote:

On Thu, Oct 14, 2021 at 3:48 PM Sadhuprasad Patro <b.sadhu@gmail.com> wrote:

Hi All,

Publisher 'DateStyle' is set as "SQL, MDY", whereas in Subscriber as
"SQL, DMY", the logical replication is not working...

From Publisher:
postgres=# INSERT INTO calendar VALUES ('07-18-1036', '1'), ('05-15-1135', '1');
INSERT 0 2

Getting below error in the subscriber log file,
2021-10-14 00:59:23.067 PDT [38262] ERROR: date/time field value out
of range: "07/18/1036"
2021-10-14 00:59:23.067 PDT [38262] HINT: Perhaps you need a
different "datestyle" setting.

Is this an expected behavior?

Looks like a problem to me, I think for fixing this, on logical
replication connection always set subscriber's DateStlyle, with that
the walsender will always send the data in the same DateStyle that
worker understands and then we are good.

Right! Attached fix it.

Add a test case in subscription/t/100_bugs.pl. Please consider the v2 patch
for review.

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

Add a test case in subscription/t/100_bugs.pl. Please consider the v2 patch
for review.

Reviewed and tested the patch, it works fine... There are some
trailing spaces present in the newly added code lines, which needs to
be corrected...
Doing some further testing with different datestyles, will update further...

Thanks & Regards
SadhuPrasad
EnterpriseDB: http://www.enterprisedb.com

On Thu, Oct 14, 2021 at 8:50 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:

On Thu, Oct 14, 2021 at 3:48 PM Sadhuprasad Patro <b.sadhu@gmail.com> wrote:

Hi All,

Publisher 'DateStyle' is set as "SQL, MDY", whereas in Subscriber as
"SQL, DMY", the logical replication is not working...

From Publisher:
postgres=# INSERT INTO calendar VALUES ('07-18-1036', '1'), ('05-15-1135', '1');
INSERT 0 2

Getting below error in the subscriber log file,
2021-10-14 00:59:23.067 PDT [38262] ERROR: date/time field value out
of range: "07/18/1036"
2021-10-14 00:59:23.067 PDT [38262] HINT: Perhaps you need a
different "datestyle" setting.

Is this an expected behavior?

Looks like a problem to me, I think for fixing this, on logical
replication connection always set subscriber's DateStlyle, with that
the walsender will always send the data in the same DateStyle that
worker understands and then we are good.

+1

Probably the same is true for IntervalStyle? If the publisher sets
'sql_standard', the subscriber sets 'postgres', and an interval value
'-1 11:22:33' is inserted, these two nodes have different data:

* Publisher
=# set intervalstyle to 'postgres'; select * from test;
i
-------------------
-1 days -11:22:33
(1 row)

* Subscriber
=# set intervalstyle to 'postgres'; select * from test;
i
-------------------
-1 days +11:22:33
(1 row)

Regards,

--
Masahiko Sawada
EDB: https://www.enterprisedb.com/

Masahiko Sawada <sawada.mshk@gmail.com> writes:

On Thu, Oct 14, 2021 at 8:50 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:

Looks like a problem to me, I think for fixing this, on logical
replication connection always set subscriber's DateStlyle, with that
the walsender will always send the data in the same DateStyle that
worker understands and then we are good.

+1

An alternative that wouldn't require a network round trip is for the
publisher to set its own datestyle to ISO/YMD. I'm pretty sure that
will be interpreted correctly regardless of the receiver's datestyle.

Probably the same is true for IntervalStyle?

Not sure if an equivalent solution applies to intervals ...

regards, tom lane

I wrote:

An alternative that wouldn't require a network round trip is for the
publisher to set its own datestyle to ISO/YMD. I'm pretty sure that
will be interpreted correctly regardless of the receiver's datestyle.

Ah ... see postgres_fdw's set_transmission_modes(). I think we want
to copy that logic not invent some other way to do it.

regards, tom lane

On Sun, Oct 17, 2021 at 11:41:35PM -0400, Tom Lane wrote:

Ah ... see postgres_fdw's set_transmission_modes(). I think we want
to copy that logic not invent some other way to do it.

dblink.c has something similar as of applyRemoteGucs(), except that it
does not do extra_float_digits. It would be nice to avoid more
duplication for those things, at least on HEAD. On the top of my
head, don't we have something similar for parallel workers when
passing down GUCs from the leader?
--
Michael

On Mon, 18 Oct 2021 at 11:14, Sadhuprasad Patro <b.sadhu@gmail.com> wrote:

Add a test case in subscription/t/100_bugs.pl. Please consider the v2 patch
for review.

Reviewed and tested the patch, it works fine... There are some
trailing spaces present in the newly added code lines, which needs to
be corrected...
Doing some further testing with different datestyles, will update further...

Thanks for your review and test! As Tom Lane said, the postgres_fdw has the similar
things, I will update the patch later.

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

On Mon, 18 Oct 2021 at 11:59, Michael Paquier <michael@paquier.xyz> wrote:

On Sun, Oct 17, 2021 at 11:41:35PM -0400, Tom Lane wrote:

Ah ... see postgres_fdw's set_transmission_modes(). I think we want
to copy that logic not invent some other way to do it.

dblink.c has something similar as of applyRemoteGucs(), except that it
does not do extra_float_digits. It would be nice to avoid more
duplication for those things, at least on HEAD. On the top of my
head, don't we have something similar for parallel workers when
passing down GUCs from the leader?

Since it will be used in more than one places. IMO, we can implement it in core.
Any thoughts?

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

On Mon, 18 Oct 2021 at 11:41, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I wrote:

An alternative that wouldn't require a network round trip is for the
publisher to set its own datestyle to ISO/YMD. I'm pretty sure that
will be interpreted correctly regardless of the receiver's datestyle.

Ah ... see postgres_fdw's set_transmission_modes(). I think we want
to copy that logic not invent some other way to do it.

Thanks for your remention. As Michael Paquier side, dblink also uses the
similar logical. I will read them then update the patch.

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

Japin Li <japinli@hotmail.com> writes:

On Mon, 18 Oct 2021 at 11:59, Michael Paquier <michael@paquier.xyz> wrote:

dblink.c has something similar as of applyRemoteGucs(), except that it
does not do extra_float_digits. It would be nice to avoid more
duplication for those things, at least on HEAD. On the top of my
head, don't we have something similar for parallel workers when
passing down GUCs from the leader?

Since it will be used in more than one places. IMO, we can implement it in core.
Any thoughts?

It's not going to be the same code everywhere. A logrep sender won't
have a need to save-and-restore the settings like postgres_fdw does,
AFAICS. Also, now that I look at it, dblink is doing the opposite
thing of absorbing the sender's values.

It would be good I guess to have some central notion of which
variables ought to be set to what, but I'm not sure how to
mechanize that given the need for different behaviors.

regards, tom lane

On Mon, 18 Oct 2021 at 12:17, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Japin Li <japinli@hotmail.com> writes:

On Mon, 18 Oct 2021 at 11:59, Michael Paquier <michael@paquier.xyz> wrote:

dblink.c has something similar as of applyRemoteGucs(), except that it
does not do extra_float_digits. It would be nice to avoid more
duplication for those things, at least on HEAD. On the top of my
head, don't we have something similar for parallel workers when
passing down GUCs from the leader?

Since it will be used in more than one places. IMO, we can implement it in core.
Any thoughts?

It's not going to be the same code everywhere. A logrep sender won't
have a need to save-and-restore the settings like postgres_fdw does,

Thanks for your explanation. Yeah, we do not need reset the settings in
logical replication.

AFAICS. Also, now that I look at it, dblink is doing the opposite
thing of absorbing the sender's values.

Sorry I misunderstand. You are right, the dblink applies the remote
server's settings to local server.

Attached v3 patch modify the settings on sender as you suggest.

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

On Mon, 18 Oct 2021 at 11:26, Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Thu, Oct 14, 2021 at 8:50 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:

On Thu, Oct 14, 2021 at 3:48 PM Sadhuprasad Patro <b.sadhu@gmail.com> wrote:

Hi All,

Publisher 'DateStyle' is set as "SQL, MDY", whereas in Subscriber as
"SQL, DMY", the logical replication is not working...

From Publisher:
postgres=# INSERT INTO calendar VALUES ('07-18-1036', '1'), ('05-15-1135', '1');
INSERT 0 2

Getting below error in the subscriber log file,
2021-10-14 00:59:23.067 PDT [38262] ERROR: date/time field value out
of range: "07/18/1036"
2021-10-14 00:59:23.067 PDT [38262] HINT: Perhaps you need a
different "datestyle" setting.

Is this an expected behavior?

Looks like a problem to me, I think for fixing this, on logical
replication connection always set subscriber's DateStlyle, with that
the walsender will always send the data in the same DateStyle that
worker understands and then we are good.

+1

Probably the same is true for IntervalStyle? If the publisher sets
'sql_standard', the subscriber sets 'postgres', and an interval value
'-1 11:22:33' is inserted, these two nodes have different data:

* Publisher
=# set intervalstyle to 'postgres'; select * from test;
i
-------------------
-1 days -11:22:33
(1 row)

* Subscriber
=# set intervalstyle to 'postgres'; select * from test;
i
-------------------
-1 days +11:22:33
(1 row)

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.

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

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.

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

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.

Please consider the v4 patch to review.

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

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.

Regards,

--
Masahiko Sawada
EDB: https://www.enterprisedb.com/

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

On Thu, 21 Oct 2021 at 14:04, Dilip Kumar <dilipbalaut@gmail.com> wrote:

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?

+1

I inclined to let walsender set the parameters.

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