From fb0ec48c2d6fdb8719c4e06b531c1930c21744ff Mon Sep 17 00:00:00 2001 From: Zhijie Hou Date: Wed, 4 Nov 2026 13:29:20 +0800 Subject: [PATCH v1] Avoid using raw xmin of frozen tuples in conflict detection. GetTupleTransactionInfo() used slot_getsysattr() with MinTransactionIdAttributeNumber to fetch the local tuple's xmin. However, heap_getsysattr() calls HeapTupleHeaderGetRawXmin(), which returns the raw 32-bit t_xmin from the tuple header even when the tuple is marked frozen (HEAP_XMIN_FROZEN). Once a tuple is frozen, its raw t_xmin is retained only for debugging purposes; its commit timestamp entry may no longer be valid, or the 32-bit XID may have wrapped around and been reassigned in a later epoch. Using the raw xmin of a frozen tuple could therefore trigger spurious update_origin_differs or delete_origin_differs conflicts, or report bogus transaction ID, origin, and commit timestamp details for unique constraint conflicts. Fix this by fetching the tuple via ExecFetchSlotHeapTuple() and calling HeapTupleHeaderGetXmin(), returning InvalidTransactionId and skipping the commit timestamp lookup when the tuple's xmin is not a normal transaction ID. Also update errdetail_apply_conflict() to omit the transaction ID from unique-conflict detail messages when localxmin is invalid. Author: Zhijie Hou Backpatch-through: 18 --- src/backend/replication/logical/conflict.c | 40 ++++++++++++----- src/test/subscription/t/035_conflicts.pl | 51 +++++++++++++++++++++- 2 files changed, 78 insertions(+), 13 deletions(-) diff --git a/src/backend/replication/logical/conflict.c b/src/backend/replication/logical/conflict.c index 90431ffce98..f8613940268 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" @@ -236,24 +237,32 @@ bool GetTupleTransactionInfo(TupleTableSlot *localslot, TransactionId *xmin, ReplOriginId *localorigin, TimestampTz *localts) { - Datum xminDatum; - bool isnull; + HeapTuple tuple; + TransactionId tup_xmin; - xminDatum = slot_getsysattr(localslot, MinTransactionIdAttributeNumber, - &isnull); - *xmin = DatumGetTransactionId(xminDatum); - Assert(!isnull); + *xmin = InvalidTransactionId; + *localorigin = InvalidReplOriginId; + *localts = 0; + + Assert(TTS_IS_BUFFERTUPLE(localslot)); + tuple = ExecFetchSlotHeapTuple(localslot, false, NULL); + + /* + * Do not return transaction information for frozen tuples, as their + * original XID is no longer meaningful. + */ + tup_xmin = HeapTupleHeaderGetXmin(tuple->t_data); + 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 +508,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 +518,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..9b51b48232c 100644 --- a/src/test/subscription/t/035_conflicts.pl +++ b/src/test/subscription/t/035_conflicts.pl @@ -209,6 +209,53 @@ 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;"); + +my $log_location = -s $node_A->logfile; + +$node_B->safe_psql( + 'postgres', " + UPDATE tab SET b = 10 WHERE a = 3; + DELETE FROM tab WHERE a = 3; + DELETE FROM tab WHERE a = 4;"); +$node_B->wait_for_catchup($subname_AB); + +my $logfile = slurp_file($node_A->logfile(), $log_location); +unlike( + $logfile, + 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_location = -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_location); + +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 @@ -292,7 +339,7 @@ $node_A->poll_query_until('postgres', "SELECT count(*) = 0 FROM pg_stat_activity WHERE backend_type = 'logical replication apply worker'" ); -my $log_location = -s $node_B->logfile; +$log_location = -s $node_B->logfile; $node_B->safe_psql('postgres', "UPDATE tab SET b = 3 WHERE a = 1;"); $node_A->safe_psql('postgres', "DELETE FROM tab WHERE a = 1;"); @@ -309,7 +356,7 @@ like( $node_A->wait_for_catchup($subname_BA); # Check the conflict detected on Node B -my $logfile = slurp_file($node_B->logfile(), $log_location); +$logfile = slurp_file($node_B->logfile(), $log_location); like( $logfile, qr/conflict detected on relation "public.tab": conflict=delete_origin_differs.* -- 2.54.0 (Apple Git-157)