| From: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
|---|---|
| To: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
| Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Zhijie Hou <houzhijie22(at)gmail(dot)com>, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, saurabh singh <saurabh(dot)singh214(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, shveta malik <shveta(dot)malik(at)gmail(dot)com> |
| Subject: | Re: Proposal: Conflict log history table for Logical Replication |
| Date: | 2026-10-13 09:41:49 |
| Message-ID: | CAJpy0uDSoSr3NtYbL4j=yhq5oYCnKEmwZADPj2JCB44A1oU6_g@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Tue, Oct 13, 2026 at 2:39 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> On Tue, Oct 13, 2026 at 9:39 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> >
> > On Mon, Oct 12, 2026 at 11:31 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> > >
> > > On Mon, Oct 12, 2026 at 6:04 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > > >
> > > > On Fri, Oct 9, 2026 at 1:43 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> > > > >
> > > > > Another concern on v79 is:
> > > > >
> > > > > I noticed that one CLT row contains three timestamp representations
> > > > > with three different behaviors:
> > > > >
> > > > > 1) replica_identity is frozen in the apply worker’s DateStyle.
> > > > > Session's Datestyle has no impact on it.
> > > > > 2) remote_commit_ts is represented in the reader’s (session's) DateStyle, and
> > > > > 3) local_conflicts.commit_ts is always ISO.
> > > > >
> > > > > This may cause some conufsion. Please see the results below. I can
> > > > > share the testcase if needed:
> > > > >
> > > > > Conflicts:
> > > > >
> > > > > postgres=# select conflict_type, remote_commit_ts, replica_identity,
> > > > > local_conflicts from pg_conflict.pg_conflict_log_16390;
> > > > > conflict_type | remote_commit_ts |
> > > > > replica_identity |
> > > > > local_conflicts
> > > > > -----------------------+--------------------------------+-----------------------------------+------------------------------------------------------------------------------------------
> > > > > delete_origin_differs | 09/10/2026 14:15:29.055423 IST |
> > > > > {"k":"2025-12-25 10:00:00+05:30"} |
> > > > > {"{\"xid\":\"672\",\"commit_ts\":\"2026-10-09T14:15:22.033391+05:30\",\"origin\":null}"}
> > > > > delete_origin_differs | 09/10/2026 14:17:12.929024 IST |
> > > > > {"k":"02/01/2025 08:00:00 IST"} |
> > > > > {"{\"xid\":\"678\",\"commit_ts\":\"2026-10-09T14:17:09.326423+05:30\",\"origin\":null}"}
> > > > > (2 rows)
> > > > >
> > > > >
> > > > > 1) First conflict row was inserted with default Datastyle of DB and
> > > > > the apply worker which is:
> > > > > postgres=# show DateStyle;
> > > > > DateStyle
> > > > > -----------
> > > > > ISO, DMY
> > > > >
> > > > > 2) Before inserting second conflict, below was done.
> > > > >
> > > > > ALTER DATABASE postgres SET DateStyle = 'SQL, DMY';
> > > > > ALTER SUBSCRIPTION sub1 DISABLE; ALTER SUBSCRIPTION sub1 ENABLE;
> > > > >
> > > > > (So Above conflict data is read when session's format was also 'SQL,
> > > > > DMY', thus we see remote_commit_ts in SQL format)
> > > > > ----
> > > > >
> > > > > We can see the differnce in 'replica_identity' values in 2 rows, while
> > > > > remote_commit_ts and local_conflicts.commit_ts remain same. Second RI
> > > > > is 02/01/2025 in DMY format, meaning 2nd january.
> > > > >
> > > > > 3)
> > > > > Now if I do this:
> > > > > SET DateStyle = 'ISO, MDY'
> > > > >
> > > > > postgres=# select conflict_type, remote_commit_ts, replica_identity,
> > > > > local_conflicts from pg_conflict.pg_conflict_log_16390;
> > > > > conflict_type | remote_commit_ts |
> > > > > replica_identity |
> > > > > local_conflicts
> > > > > -----------------------+----------------------------------+-----------------------------------+------------------------------------------------------------------------------------------
> > > > > delete_origin_differs | 2026-10-09 14:15:29.055423+05:30 |
> > > > > {"k":"2025-12-25 10:00:00+05:30"} |
> > > > > {"{\"xid\":\"672\",\"commit_ts\":\"2026-10-09T14:15:22.033391+05:30\",\"origin\":null}"}
> > > > > delete_origin_differs | 2026-10-09 14:17:12.929024+05:30 |
> > > > > {"k":"02/01/2025 08:00:00 IST"} |
> > > > > {"{\"xid\":\"678\",\"commit_ts\":\"2026-10-09T14:17:09.326423+05:30\",\"origin\":null}"}
> > > > > (2 rows)
> > > > >
> > > > > See remote_commit_ts changed from previous SQL to ISO, while
> > > > > replica_identity and local_conflicts.commit_ts are the same.
> > > > >
> > > > > If I try to read it as documented (casting it back to original
> > > > > column's type), I get misleading results:
> > > > >
> > > > > postgres=# SET DateStyle = 'ISO, MDY';
> > > > >
> > > > > postgres=#
> > > > > SELECT (local_conflicts[1]->>'commit_ts')::timestamptz as local_ts,
> > > > > (replica_identity->>'k')::timestamptz as ri_ts
> > > > > FROM pg_conflict.pg_conflict_log_16390;
> > > > >
> > > > > local_ts | ri_ts
> > > > > ----------------------------------+---------------------------
> > > > > 2026-10-09 14:15:22.033391+05:30 | 2025-12-25 10:00:00+05:30
> > > > > 2026-10-09 14:17:09.326423+05:30 | 2025-02-01 08:00:00+05:30
> > > > > (2 rows)
> > > > >
> > > > > See second row for ri_ts, it will now represent '1st Feb' rather than
> > > > > '2nd January'.
> > > > >
> > > > > Shouldn't all three timestamps be in the same format?
> > > > >
> > > > > IMO, remote_commit_ts being session-dependent is expected.
> > > > > The concern is the other two JSON text columns. replica_identity
> > > > > stores the worker’s DateStyle, so the ->>'k'::timestamptz can
> > > > > silently return a different value (here 2nd Jan read back as 1st Feb);
> > > > > while local_conflicts.commit_ts is always ISO. Shouldn’t the recorded
> > > > > JSON timestamps use a single, stable format (ISO) so they’re
> > > > > unambiguous and safe to cast back?
> > > > >
> > > >
> > > > What are the values if you don't change the Datestyle manually and use
> > > > the default or what was used by worker while reading it?
> > > >
> > >
> > >
> > > With default, values are:
> > >
> > > postgres=# SELECT conflict_type,
> > > remote_commit_ts,
> > > replica_identity->>'k' AS ri_key_timestamp,
> > > local_conflicts[1]->>'commit_ts' AS lc_commit_ts
> > > FROM pg_conflict.pg_conflict_log_16390;
> > >
> > > conflict_type | remote_commit_ts |
> > > ri_key_timestamp | lc_commit_ts
> > > -----------------------+----------------------------------+---------------------------+----------------------------------
> > > delete_origin_differs | 2026-10-12 10:47:07.349306+05:30 | 2025-12-25
> > > 10:00:00+05:30 | 2026-10-12T10:47:04.618018+05:30
> > > delete_origin_differs | 2026-10-12 10:48:22.419669+05:30 | 2025-01-02
> > > 08:00:00+05:30 | 2026-10-12T10:48:20.277719+05:30
> > > (2 rows)
> > >
> > > Default DateStyle of Db:
> > >
> > > postgres=# show DateStyle;
> > > DateStyle
> > > -----------
> > > ISO, DMY
> > > (1 row)
> > >
> > > Same is used by worker while insert and in same we have read the above data.
> > >
> > > ~~
> > >
> > > So with the default (ISO), or whenever the reader’s DateStyle matches
> > > the one the worker used at write time, the values read back correctly;
> > > the misleading result only appears when the reader’s DateStyle differs
> > > from the worker’s write-time DateStyle (specifically when the worker
> > > wrote under a non-ISO style).
> > >
> > > ~~
> > >
> > > Further observations and analysis:
> > >
> > > 1) Apply worker uses current datestyle of DB for replica_identity, and
> > > then materializes it into text (json col).
> > >
> > > 2) local_conflicts always uses ISO because it’s built via row_to_json.
> > > See JsonEncodeDateTime() → EncodeDateTime(), which is called with
> > > USE_XSD_DATES; in EncodeDateTime() that falls through the same branch
> > > as USE_ISO_DATES.
> > >
> > > 3) remote_commit_ts is stored as timestamp column rather than text in json.
> > >
> > > The one stored as timestamptz (3) will have no issues. Because it’s a
> > > genuine timestamptz, we always get back the correct instant; only its
> > > display changes with the reader’s DateStyle.
> > >
> > > Both (1) and (2) are frozen as text, but IIUC, only replica_identity
> > > is at risk.
> > >
> > > local_conflicts.commit_ts avoids it by alwyas using ISO (written
> > > always as YYYY-MM-DD, where month and day are in fixed positions. Thus
> > > no reader’s DMY/MDY setting can swap them.
> > >
> > > replica_identity is the only one that stores the worker’s DateStyle
> > > into stored text (unknown and un-recoverable later). Different rows
> > > can be frozen under different styles (config change), so a reader has
> > > no reliable style to pick while reading.
> > >
> > > The fix could be to have the apply worker also use ISO style while
> > > inserting; similar to row_to_json, and how logical replication’s
> > > libpqwalreceiver standardizes the wire timestamp values using ISO. See
> > > libpqrcv_connect using datestyle=ISO.
> > >
> >
> > If we want to address this, I have attached the patch for a potential
> > solution. I have also attached the testcase.
> >
> > In the worker, we can change the config to datestyle=ISO before
> > building the JSON values.
> >
> > --NewGUCNestLevel() opens a new GUC nesting level and returns its
> > number. It does not change any setting.
> > --SetConfigOption() then changes the GUC (datestyle=ISO).
> > --AtEOXact_GUC() restores the GUC stack back to that marked point. In
> > case of error, AtEOXact_GUC() will be called by Abort-Txn.
>
> I have reviewed this change, IMHO there are slight error, basically,
> SetConfigOption() does not restore the old value at AtEOXact_GUC(true,
> save_nestlevel). SetConfigOption calls set_config_option(...,
> GUC_ACTION_SET, ...), which records the stack entry with state GUC_SET
> rather than GUC_SAVE. When AtEOXact_GUC(true, save_nestlevel) is
> called with isCommit = true: It only restores the prior value when
> stack->state == GUC_SAVE not GUC_SET. So for fixing this correctly we
> need to directly call set_config_option() with GUC_ACTION_SAVE
> instead of calling SetConfigOption() as shown below.
You are right. I missed this point.
> save_nestlevel = NewGUCNestLevel();
> (void) set_config_option("datestyle", "ISO",
> PGC_USERSET, PGC_S_SESSION,
> GUC_ACTION_SAVE, true, 0, false);
> .....
> AtEOXact_GUC(true, save_nestlevel);
I agree with the changes.
thanks
Shveta
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Paul Kim | 2026-10-13 09:51:05 | Re: Be strict when request to flush past end of WAL in WaitXLogInsertionsToFinish |
| Previous Message | Paul Kim | 2026-10-13 09:26:40 | Re: Be strict when request to flush past end of WAL in WaitXLogInsertionsToFinish |