From 9f6ad28b0ed4ab11aae810d7fd323fd66344ac66 Mon Sep 17 00:00:00 2001 From: Andrey Borodin Date: Sun, 23 Aug 2026 15:23:49 +0300 Subject: [PATCH v1 2/2] Detect SSI conflicts with summarized predicate locks CheckTargetForConflictsIn() uses a transaction XID horizon to decide whether a committed SIREAD lock holder overlapped the writer. The OldCommittedSxact dummy has no valid horizon, so this test ignores locks after SSI summarizes them and can allow write skew to commit. Summarized predicate locks already retain the latest commit sequence number among the transactions represented by the lock. Compare that with the writer snapshot sequence number to determine whether at least one of those transactions overlapped the writer. Back-patch to all supported branches. Discussion: https://postgr.es/m/CA%2BCOZaCtK%3DUQbeQwdAoRw27J%2B58bJBC%2ByNHP4OH2%2By_t2UtFAg%40mail.gmail.com --- src/backend/storage/lmgr/predicate.c | 35 +++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c index bc9b5191a4c..9c543a7602c 100644 --- a/src/backend/storage/lmgr/predicate.c +++ b/src/backend/storage/lmgr/predicate.c @@ -3924,6 +3924,33 @@ XidIsConcurrent(TransactionId xid) return pg_lfind32(xid, snap->xip, snap->xcnt); } +/* + * Does this predicate lock belong to a transaction which overlaps ours? + * + * Normal SERIALIZABLEXACTs retain an XID horizon for this test. The dummy + * transaction used for summarized locks has no such horizon, but each of its + * locks retains the latest commit sequence number among the transactions + * folded into it. If the latest commit occurred after our snapshot, at least + * one transaction represented by the lock overlapped ours. + */ +static bool +PredicateLockIsForOverlappingTransaction(const PREDICATELOCK *predlock) +{ + SERIALIZABLEXACT *sxact = predlock->tag.myXact; + + if (sxact == OldCommittedSxact) + { + Assert(predlock->commitSeqNo != 0); + Assert(predlock->commitSeqNo != InvalidSerCommitSeqNo); + return predlock->commitSeqNo > + MySerializableXact->SeqNo.lastCommitBeforeSnapshot; + } + + return !SxactIsCommitted(sxact) || + TransactionIdPrecedes(GetTransactionSnapshot()->xmin, + sxact->finishedBefore); +} + bool CheckForSerializableConflictOutNeeded(Relation relation, Snapshot snapshot) { @@ -4160,9 +4187,7 @@ CheckTargetForConflictsIn(PREDICATELOCKTARGETTAG *targettag) } } else if (!SxactIsDoomed(sxact) - && (!SxactIsCommitted(sxact) - || TransactionIdPrecedes(GetTransactionSnapshot()->xmin, - sxact->finishedBefore)) + && PredicateLockIsForOverlappingTransaction(predlock) && !RWConflictExists(sxact, MySerializableXact)) { LWLockRelease(SerializableXactHashLock); @@ -4173,9 +4198,7 @@ CheckTargetForConflictsIn(PREDICATELOCKTARGETTAG *targettag) * transaction may have flagged a conflict. */ if (!SxactIsDoomed(sxact) - && (!SxactIsCommitted(sxact) - || TransactionIdPrecedes(GetTransactionSnapshot()->xmin, - sxact->finishedBefore)) + && PredicateLockIsForOverlappingTransaction(predlock) && !RWConflictExists(sxact, MySerializableXact)) { FlagRWConflict(sxact, MySerializableXact); -- That's all, folks. May the source be with you.