On Tue, Oct 13, 2026 at 3:34 PM Dilip Kumar <[email protected]> wrote:
>
> I merged v80-0001 and v80-0002 and adjusted the commit message and
> test accordingly; I also included Amit's and Hou-san's code
> refactoring. Rebased Nisha's V80_2-0003 on top of that as 0002, also
> merged Shveta's datestyle fix with some modification into the 0002
>
Thanks for the updated patches. A few comments on v81-0001:
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>. 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?
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.
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.
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.
The attached diff adds a paragraph for this. Please consider if it
looks okay to you, and feel free to modify it as you see fit.
--
Thanks,
Nisha
diff --git a/doc/src/sgml/logical-replication.sgml
b/doc/src/sgml/logical-replication.sgml
index 0d88cbfd4fd..5f0afb2a796 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -2441,6 +2441,23 @@ SELECT (replica_identity->>'doc')::jsonb->>'x' FROM ...;
-- 1
</programlisting>
</para>
+ <para>
+ The apply worker's <xref linkend="guc-bytea-output"/> and
+ <xref linkend="guc-quote-all-identifiers"/> settings change how a value
+ is written without changing the value itself. If these settings differ
+ between conflicts, the same key can be recorded in different but
+ equivalent forms, and comparing the recorded text may miss some of them.
+ To match conflicts on the same key, cast the value back to the column's
+ type instead, here for a <type>bytea</type> key column
<literal>b</literal>
+ and a <type>regclass[]</type> key column <literal>r</literal>:
+<programlisting>
+SELECT * FROM ... WHERE replica_identity->>'b' = '\x0102';
-- may miss rows
+SELECT * FROM ... WHERE (replica_identity->>'b')::bytea = '\x0102';
-- matches all
+SELECT * FROM ... WHERE replica_identity->>'r' = '{t}';
-- may miss rows
+SELECT * FROM ... WHERE (replica_identity->>'r')::regclass[] = '{t}';
-- matches all
+</programlisting>
+ </para>
+
<para>
If a key value's text representation is larger than 64kB, it is not
recorded. In its place the conflict log stores a small object marking it
