From 9f6ad28b0ed4ab11aae810d7fd323fd66344ac66 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <amborodin@acm.org>
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.

