From 16c66de323e75c4240de8d240dc81a91fcf9f114 Mon Sep 17 00:00:00 2001 From: Zhijie Hou Date: Mon, 9 Nov 2026 16:17:46 +0800 Subject: [PATCH v2] Fix logical replication conflict detection on frozen tuples. GetTupleTransactionInfo() obtained the local tuple's xmin through the xmin system attribute. For heap tuples, heap_getsysattr() returns the raw XID from the tuple header even after the tuple has been frozen. Such an XID cannot be used reliably for commit timestamp lookup: its entry may have been trimmed, or the XID may have wrapped around and been reassigned. This could cause spurious update_origin_differs or delete_origin_differs conflicts, and unique-conflict DETAIL messages could report bogus transaction ID, origin, and commit timestamp information. Fix this by using HeapTupleHeaderGetXmin() when the slot is a heap or buffer-heap tuple slot (which returns FrozenTransactionId for frozen tuples) and falling back to slot_getsysattr() for other slot types. GetTupleTransactionInfo() ignores non-normal XIDs and sets *xmin to InvalidTransactionId, and errdetail_apply_conflict() omits transaction details from unique-conflict messages when the local XID is invalid. Author: Zhijie Hou Discussion: https://postgr.es/m/CAFvd2n_S++xXFtDe208kbZUDbdeH8OabcGvOi6UdbdeJmUfgOQ@mail.gmail.com --- src/backend/replication/logical/conflict.c | 68 +++++++++++++++++----- src/test/subscription/t/035_conflicts.pl | 45 ++++++++++++++ 2 files changed, 100 insertions(+), 13 deletions(-) diff --git a/src/backend/replication/logical/conflict.c b/src/backend/replication/logical/conflict.c index 90431ffce98..7e1dea158de 100644 --- a/src/backend/replication/logical/conflict.c +++ b/src/backend/replication/logical/conflict.c @@ -16,6 +16,7 @@ #include "access/commit_ts.h" #include "access/genam.h" +#include "access/htup_details.h" #include "access/tableam.h" #include "catalog/heap.h" #include "catalog/pg_am.h" @@ -227,8 +228,11 @@ GetConflictLogDest(const char *dest) } /* - * Get the xmin and commit timestamp data (origin and timestamp) associated - * with the provided local row. + * Get the effective xmin and commit timestamp data (origin and timestamp) + * associated with the provided local row. + * + * If the tuple is frozen or does not have a normal XID, *xmin is set to + * InvalidTransactionId. * * Return true if the commit timestamp data was found, false otherwise. */ @@ -236,24 +240,53 @@ bool GetTupleTransactionInfo(TupleTableSlot *localslot, TransactionId *xmin, ReplOriginId *localorigin, TimestampTz *localts) { - Datum xminDatum; - bool isnull; + TransactionId tup_xmin; + + *xmin = InvalidTransactionId; + *localorigin = InvalidReplOriginId; + *localts = 0; + + /* + * For heap slots, heap_getsysattr() returns the raw xmin even when the + * tuple is frozen, so use HeapTupleHeaderGetXmin() to obtain the + * effective xmin. For other slot types, fall back to fetching the xmin + * system attribute. + */ + if (TTS_IS_BUFFERTUPLE(localslot) || TTS_IS_HEAPTUPLE(localslot)) + { + bool shouldFree; + HeapTuple tuple; - xminDatum = slot_getsysattr(localslot, MinTransactionIdAttributeNumber, - &isnull); - *xmin = DatumGetTransactionId(xminDatum); - Assert(!isnull); + tuple = ExecFetchSlotHeapTuple(localslot, false, &shouldFree); + Assert(!shouldFree); + tup_xmin = HeapTupleHeaderGetXmin(tuple->t_data); + } + else + { + Datum xminDatum; + bool isnull; + + xminDatum = slot_getsysattr(localslot, MinTransactionIdAttributeNumber, + &isnull); + tup_xmin = DatumGetTransactionId(xminDatum); + Assert(!isnull); + } + + /* + * Non-normal XIDs cannot be used to obtain meaningful transaction + * information. + */ + if (!TransactionIdIsNormal(tup_xmin)) + return false; + + *xmin = tup_xmin; /* * The commit timestamp data is not available if track_commit_timestamp is * disabled. */ if (!track_commit_timestamp) - { - *localorigin = InvalidReplOriginId; - *localts = 0; return false; - } return TransactionIdGetCommitTsData(*xmin, localts, localorigin); } @@ -499,7 +532,7 @@ errdetail_apply_conflict(EState *estate, ResultRelInfo *relinfo, localxmin, timestamptz_to_str(localts)); } } - else + else if (TransactionIdIsValid(localxmin)) { if (tuple_buf.len) appendStringInfo(&err_detail, _("Key already exists in unique index \"%s\", modified in transaction %u: %s."), @@ -509,6 +542,15 @@ errdetail_apply_conflict(EState *estate, ResultRelInfo *relinfo, appendStringInfo(&err_detail, _("Key already exists in unique index \"%s\", modified in transaction %u."), get_rel_name(indexoid), localxmin); } + else + { + if (tuple_buf.len) + appendStringInfo(&err_detail, _("Key already exists in unique index \"%s\": %s."), + get_rel_name(indexoid), tuple_buf.data); + else + appendStringInfo(&err_detail, _("Key already exists in unique index \"%s\"."), + get_rel_name(indexoid)); + } break; diff --git a/src/test/subscription/t/035_conflicts.pl b/src/test/subscription/t/035_conflicts.pl index 48ce95d9d13..34157935e2b 100644 --- a/src/test/subscription/t/035_conflicts.pl +++ b/src/test/subscription/t/035_conflicts.pl @@ -209,6 +209,51 @@ ok( $node_B->poll_query_until( ), "the xmin value of slot 'pg_conflict_detection' is valid on Node B"); +############################################################################### +# Check that frozen tuples do not use their raw xmin for conflict detection or +# reporting. Run this before enabling retain_dead_tuples on Node A so that +# VACUUM FREEZE is not held back by the conflict detection slot. +############################################################################### + +$node_A->safe_psql( + 'postgres', " + CREATE UNIQUE INDEX tab_b_key ON tab (b); + INSERT INTO tab VALUES (3, 1), (4, 2), (5, 3);"); +$node_A->wait_for_catchup($subname_BA); +$node_A->safe_psql('postgres', "VACUUM FREEZE tab;"); + +$log_offset = -s $node_A->logfile; + +$node_B->safe_psql( + 'postgres', " + UPDATE tab SET b = 10 WHERE a = 3; + DELETE FROM tab WHERE a = 4;"); +$node_B->wait_for_catchup($subname_AB); + +unlike( + slurp_file($node_A->logfile(), $log_offset), + qr/conflict detected on relation "public.tab": conflict=(update|delete)_origin_differs/, + 'frozen tuples do not trigger update_origin_differs or delete_origin_differs' +); + +$log_offset = -s $node_A->logfile; + +$node_B->safe_psql('postgres', "INSERT INTO tab VALUES (6, 3);"); + +$node_A->wait_for_log( + qr/conflict detected on relation "public.tab": conflict=insert_exists.* +.*Could not apply remote change: remote row \(6, 3\).* +.*Key already exists in unique index "tab_b_key": key \(b\)=\(3\), local row \(5, 3\)./, + $log_offset); + +pass('insert_exists on frozen tuple does not report transaction details'); + +# Clean up +$node_A->safe_psql('postgres', "DROP INDEX tab_b_key;"); +$node_B->wait_for_catchup($subname_AB); +$node_A->safe_psql('postgres', "TRUNCATE tab;"); +$node_A->wait_for_catchup($subname_BA); + ################################################## # Check that the retain_dead_tuples option can be enabled only for disabled # subscriptions. Validate the NOTICE message during the subscription DDL, and -- 2.54.0 (Apple Git-157)