On Thu, Oct 15, 2026 at 9:47 AM shveta malik <[email protected]> wrote:
>
> On Wed, Oct 14, 2026 at 8:16 PM Nisha Moond <[email protected]> wrote:
> >
> >
> > 1) Does the remote_origin column in CLT add any value?
> > + if (replorigin_xact_state.origin != InvalidReplOriginId)
> > + replorigin_by_oid(replorigin_xact_state.origin, true, &remote_origin);
> >
> > IIUC, replorigin_xact_state.origin is the subscription's own origin,
> > set at worker start, so every row has pg_<subid>.
>
> It could also be pg_<subid>_<relid>, for a tablesync worker. But
> again, it is not very useful as both subid and relid can be figured
> out from CLT table itself. So I too currently do not see any use of
> this column, unless I am missing some case?
>
Not changed in v82. Removing the column touches several places: the
CLT schema, the docs, the TAP tests and the regression output. Let's
wait to hear what others think before making the change.
> > A CLT only holds
> > conflicts of its own subscription, and its name already contains the
> > subid. Once the subscription is dropped, the CLT goes with it, and
> > even a saved copy can't be traced back to a publisher node. Should we
> > drop this column? Thoughts?
> >
> > 2) conflict.c: insert_conflict_log_tuple():
> > + heap_insert(conflictlogrel, tuple,
> > + GetCurrentCommandId(true), HEAP_INSERT_NO_LOGICAL, NULL);
> > Should we add a comment explaining why HEAP_INSERT_NO_LOGICAL is still
> > needed even though a CLT can't be added to a publication?
> >
Added a comment explaining the reason.
> > 3) v78-0004 removed "or when replica identity is not applicable" from
> > the replica_identity description, since searchslot can never be NULL.
> > I think v80 added it back:
> > + when replica identity is not applicable.</entry>
> > The code also has Assert(!TupIsNull(searchslot)), so replica_identity
> > is NULL only when replica_identity_full is true. Should remove this
> > part again.
> >
Removed it from the replica_identity description.
> > 4) The conflict below is from a 2PC txn hitting delete_missing while
> > the worker processes PREPARE TRANSACTION:
> >
> > relname | conflict_type | remote_xid | remote_commit_lsn |
> > remote_commit_ts
> > ---------+----------------+------------+-------------------+------------------
> > t2pc | delete_missing | 669 | 0/017D0958 |
> >
> > Using the prepare LSN as remote_commit_lsn for 2PC seems intentional.
> > But remote_commit_ts stays NULL, because the commit time isn't known
> > during PREPARE TRANSACTION. Could we use the prepare time here, as we
> > do with the prepare lsn? It may not help with resolution, though. If
> > not, we should document the NULL for 2PC too; currently the docs
> > mention it only for the parallel apply worker.
> >
Updated the docs to say it's NULL when the conflict occurs in a
transaction prepared on the subscriber. Let me know if others think we
should store the prepare time instead.
> > 5) The text recorded for a key depends on the apply worker's output
> > settings. With bytea_output = escape or quote_all_identifiers = on set
> > on the subscriber, the same key is stored differently from the
> > default. For example, the two settings were toggled between two
> > conflicts on the same keys (tb has a bytea key, tr a regclass[] key):
> >
> > postgres=# SELECT relname, conflict_type, replica_identity::text FROM
> > pg_conflict.pg_conflict_log_16398;
> > relname | conflict_type | replica_identity
> > ---------+----------------+--------------------------------
> > -- bytea_output
> > tb | delete_missing | {"k":"\\x80ff"} -- hex case
> > tb | delete_missing | {"k":"\\200\\377"} -- escape case
> >
> > --quote_all_identifiers
> > tr | delete_missing | {"k":"{pg_class}"} -- off case
> > tr | delete_missing | {"k":"{\"\\\"pg_class\\\"\"}"} -- on case
> >
> > The values themselves are correct: casting back to the column type
> > gives the same key, e.g. (replica_identity->>'k')::bytea or
> > ::regclass[]. But anyone searching or grouping conflicts by
> > replica_identity->>'k' as text gets split or missing results. The
> > current docs say the value is "the same text a SELECT of that column
> > would show", which only holds when the reader's settings match the
> > apply worker's. It would help to document this.
>
> Doc looks okay to me.
>
Thanks for the review; I added it in v82-0003.
--
Thanks,
Nisha
