From 15d6446bf65142a8297572dbbfea57bf0d82ead9 Mon Sep 17 00:00:00 2001 From: Nisha Moond Date: Fri, 16 Oct 2026 19:12:12 +0530 Subject: [PATCH v84 3/4] Address review comments on v81-0001 - Take replica_identity key names from the table instead of the index, whose column names can be stale (e.g. after a column rename) or wrong (e.g. UNIQUE (a, a)). - Render replica identity values using the apply worker's existing output settings, without temporarily changing DateStyle, IntervalStyle or extra_float_digits. The values are intended for human inspection; portable rendering can be revisited if field experience shows a need. - Assert that remote_xid is valid instead of storing NULL. - Explain in comments why HEAP_INSERT_NO_LOGICAL is needed (TOAST inserts), and add an XXX comment explaining why the oversize marker keeps no prefix. - Docs: explain how output settings affect replica_identity text. --- doc/src/sgml/logical-replication.sgml | 53 +++++++++++++++++++--- src/backend/replication/logical/conflict.c | 47 +++++++++++++------ 2 files changed, 80 insertions(+), 20 deletions(-) diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml index 0d88cbfd4fd..da8f09c88b2 100644 --- a/doc/src/sgml/logical-replication.sgml +++ b/doc/src/sgml/logical-replication.sgml @@ -2361,7 +2361,7 @@ DETAIL: detailed_explanation[: remote_commit_ts timestamptz - The remote commit timestamp of the remote transaction. This is NULL if the conflict is handled by a parallel apply worker, because in-progress transactions are streamed and applied before the remote commit timestamp is known. + The remote commit timestamp of the remote transaction. This is NULL if the conflict is handled by a parallel apply worker, because in-progress transactions are streamed and applied before the remote commit timestamp is known. It is also NULL if the conflict occurs in a transaction that is prepared on the subscriber (see two_phase), because the commit timestamp is known only when COMMIT PREPARED arrives. remote_origin @@ -2379,8 +2379,7 @@ DETAIL: detailed_explanation[: The replica identity key values identifying the conflicting row, as a JSON object keyed by column name; see below for how the values are represented. This is NULL when - replica_identity_full is true or - when replica identity is not applicable. + replica_identity_full is true. local_conflicts @@ -2389,9 +2388,13 @@ DETAIL: detailed_explanation[: xid), commit timestamp - (commit_ts), and origin (origin). - This is NULL if there are no local conflicting rows - (e.g., in update_missing and + (commit_ts), and the name of the replication + origin that last modified the row (origin). + The origin field is NULL if + the transaction had no replication origin set or if that origin has + since been dropped. The column itself is NULL + if there are no local conflicting rows (e.g., in + update_missing and delete_missing conflicts). @@ -2441,6 +2444,44 @@ SELECT (replica_identity->>'doc')::jsonb->>'x' FROM ...; -- 1 + + The apply worker's , + , + , + and + settings can affect how a + key value is written. If these settings differ between conflicts, the + same key can be recorded in different forms, and comparing the recorded + text may miss some of them. For settings that affect only the + representation, such as and + , cast the value back to the + column's type to compare equivalent forms. Here are examples for a + bytea key column b and a + regclass[] key column r: + +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 + + + + + and + affect how values are written and + interpreted. When casting such recorded values, use the same settings + that the apply worker used when recording the conflict; otherwise, the + original value may not be restored correctly. For example, + 02/01/2025 means January 2, 2025 with + DMY, but February 1, 2025 with MDY. + For intervals, the hours in -1 02:00:00 are positive + with postgres, but negative with + sql_standard. For floating-point keys, keep + at 1 or higher when recording; + zero or negative values round the output and can prevent exact + restoration. + + 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 diff --git a/src/backend/replication/logical/conflict.c b/src/backend/replication/logical/conflict.c index 6b4f0a4da24..9ab74a85538 100644 --- a/src/backend/replication/logical/conflict.c +++ b/src/backend/replication/logical/conflict.c @@ -1088,29 +1088,22 @@ build_index_key_json(EState *estate, Relation localrel, bool *omitted) { Relation indexDesc; - TupleDesc indexTupDesc; + TupleDesc reltupdesc = RelationGetDescr(localrel); Datum values[INDEX_MAX_KEYS]; bool isnull[INDEX_MAX_KEYS]; StringInfoData result; int indnkeyatts; Datum datum; - int save_nestlevel; Assert(slot != NULL); Assert(CheckRelationOidLockedByMe(indexid, RowExclusiveLock, true)); indexDesc = index_open(indexid, NoLock); - indexTupDesc = RelationGetDescr(indexDesc); build_index_datums_from_slot(estate, localrel, slot, indexDesc, values, isnull); - save_nestlevel = NewGUCNestLevel(); - (void) set_config_option("datestyle", "ISO", - PGC_USERSET, PGC_S_SESSION, - GUC_ACTION_SAVE, true, 0, false); - /* * Build the JSON object here rather than handing the values to * row_to_json(), and render each of them with its type's output function. @@ -1137,7 +1130,22 @@ build_index_key_json(EState *estate, Relation localrel, for (int i = 0; i < indnkeyatts; i++) { - Form_pg_attribute att = TupleDescAttr(indexTupDesc, i); + Form_pg_attribute att; + AttrNumber attnum; + + /* + * Take the column name from the table, not the index descriptor. The + * index keeps the name it had at creation, which goes stale after a + * column rename and can be wrong for repeated columns such as UNIQUE + * (a, a). This matches the names in the server log. + * + * Replica identity and primary key indexes have no expression + * columns, so every key maps to a table column. + */ + attnum = indexDesc->rd_index->indkey.values[i]; + Assert(attnum > 0); + + att = TupleDescAttr(reltupdesc, attnum - 1); if (i > 0) appendStringInfoChar(&result, ','); @@ -1165,6 +1173,10 @@ build_index_key_json(EState *estate, Relation localrel, /* * Don't record a value larger than CONFLICT_MAX_VALUE_SIZE; put a * marker recording its length in its place. + * + * XXX The marker deliberately keeps no prefix of the value. For + * arrays, composites and similar types the leading bytes are + * mostly punctuation and say little about the value. */ if (outputlen > CONFLICT_MAX_VALUE_SIZE) { @@ -1184,7 +1196,6 @@ build_index_key_json(EState *estate, Relation localrel, appendStringInfoChar(&result, '}'); - AtEOXact_GUC(true, save_nestlevel); index_close(indexDesc, NoLock); datum = PointerGetDatum(cstring_to_text_with_len(result.data, result.len)); @@ -1369,10 +1380,13 @@ insert_conflict_log_tuple(EState *estate, Relation rel, values[attno++] = CStringGetTextDatum(ConflictTypeNames[conflict_type]); - if (TransactionIdIsValid(remote_xid)) - values[attno++] = TransactionIdGetDatum(remote_xid); - else - nulls[attno++] = true; + /* + * The remote xid is always valid here: a conflict is only detected while + * applying a row change, which is preceded by a BEGIN, BEGIN PREPARE or + * STREAM START that recorded a valid top-level xid. + */ + Assert(TransactionIdIsValid(remote_xid)); + values[attno++] = TransactionIdGetDatum(remote_xid); if (XLogRecPtrIsValid(remote_final_lsn)) values[attno++] = LSNGetDatum(remote_final_lsn); @@ -1436,6 +1450,11 @@ insert_conflict_log_tuple(EState *estate, Relation rel, tuple = heap_form_tuple(RelationGetDescr(conflictlogrel), values, nulls); + /* + * The conflict log table itself is never logically decoded, but its TOAST + * table would be. heap_insert() passes HEAP_INSERT_NO_LOGICAL on to the + * TOAST inserts, so toasted values are not decoded either. + */ heap_insert(conflictlogrel, tuple, GetCurrentCommandId(true), HEAP_INSERT_NO_LOGICAL, NULL); -- 2.34.1