From 2fa26b50ceada4edc8f9ee637057197c02fdcbe8 Mon Sep 17 00:00:00 2001 From: Andrey Borodin Date: Sun, 23 Aug 2026 15:23:49 +0300 Subject: [PATCH 1/4] 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 0ae85b7d5b4..d829cef07c9 100644 --- a/src/backend/storage/lmgr/predicate.c +++ b/src/backend/storage/lmgr/predicate.c @@ -3916,6 +3916,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) { @@ -4152,9 +4179,7 @@ CheckTargetForConflictsIn(PREDICATELOCKTARGETTAG *targettag) } } else if (!SxactIsDoomed(sxact) - && (!SxactIsCommitted(sxact) - || TransactionIdPrecedes(GetTransactionSnapshot()->xmin, - sxact->finishedBefore)) + && PredicateLockIsForOverlappingTransaction(predlock) && !RWConflictExists(sxact, MySerializableXact)) { LWLockRelease(SerializableXactHashLock); @@ -4165,9 +4190,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); -- 2.55.0