From c9d0b873f6a093de44edb95ba17076cf9ffe9198 Mon Sep 17 00:00:00 2001 From: Sophie Alpert Date: Sat, 6 Sep 2025 00:30:34 -0700 Subject: [PATCH] Fix missing EvalPlanQual recheck for TID scans --- src/backend/executor/nodeTidrangescan.c | 5 +- src/backend/executor/nodeTidscan.c | 18 +++-- .../isolation/expected/eval-plan-qual.out | 70 +++++++++++++++++++ src/test/isolation/specs/eval-plan-qual.spec | 13 ++++ 4 files changed, 99 insertions(+), 7 deletions(-) diff --git a/src/backend/executor/nodeTidrangescan.c b/src/backend/executor/nodeTidrangescan.c index 26f7420b64b..ef40be856f6 100644 --- a/src/backend/executor/nodeTidrangescan.c +++ b/src/backend/executor/nodeTidrangescan.c @@ -274,7 +274,10 @@ TidRangeNext(TidRangeScanState *node) static bool TidRangeRecheck(TidRangeScanState *node, TupleTableSlot *slot) { - return true; + if (!TidRangeEval(node)) + return false; + return ItemPointerCompare(&node->trss_mintid, &slot->tts_tid) <= 0 + && ItemPointerCompare(&slot->tts_tid, &node->trss_maxtid) <= 0; } /* ---------------------------------------------------------------- diff --git a/src/backend/executor/nodeTidscan.c b/src/backend/executor/nodeTidscan.c index 5e56e29a15f..c1499cded33 100644 --- a/src/backend/executor/nodeTidscan.c +++ b/src/backend/executor/nodeTidscan.c @@ -402,12 +402,18 @@ TidNext(TidScanState *node) static bool TidRecheck(TidScanState *node, TupleTableSlot *slot) { - /* - * XXX shouldn't we check here to make sure tuple matches TID list? In - * runtime-key case this is not certain, is it? However, in the WHERE - * CURRENT OF case it might not match anyway ... - */ - return true; + void *match; + + if (node->tss_isCurrentOf) + /* WHERE CURRENT OF always intends to resolve to the latest tuple */ + return true; + + if (node->tss_TidList == NULL) + TidListEval(node); + + match = bsearch(&slot->tts_tid, node->tss_TidList, node->tss_NumTids, + sizeof(ItemPointerData), itemptr_comparator); + return match != NULL; } diff --git a/src/test/isolation/expected/eval-plan-qual.out b/src/test/isolation/expected/eval-plan-qual.out index 032d4208d51..5687911920b 100644 --- a/src/test/isolation/expected/eval-plan-qual.out +++ b/src/test/isolation/expected/eval-plan-qual.out @@ -1218,6 +1218,76 @@ subid|id (1 row) +starting permutation: tid1 tid2 c1 c2 read +step tid1: UPDATE accounts SET balance = balance + 100 WHERE ctid = '(0,1)' RETURNING accountid, balance; +accountid|balance +---------+------- +checking | 700 +(1 row) + +step tid2: UPDATE accounts SET balance = balance + 200 WHERE ctid = '(0,1)' RETURNING accountid, balance; +step c1: COMMIT; +step tid2: <... completed> +accountid|balance +---------+------- +(0 rows) + +step c2: COMMIT; +step read: SELECT * FROM accounts ORDER BY accountid; +accountid|balance|balance2 +---------+-------+-------- +checking | 700| 1400 +savings | 600| 1200 +(2 rows) + + +starting permutation: tid1 tidsucceed2 c1 c2 read +step tid1: UPDATE accounts SET balance = balance + 100 WHERE ctid = '(0,1)' RETURNING accountid, balance; +accountid|balance +---------+------- +checking | 700 +(1 row) + +step tidsucceed2: UPDATE accounts SET balance = balance + 200 WHERE ctid = '(0,1)' OR ctid = '(0,3)' RETURNING accountid, balance; +step c1: COMMIT; +step tidsucceed2: <... completed> +accountid|balance +---------+------- +checking | 900 +(1 row) + +step c2: COMMIT; +step read: SELECT * FROM accounts ORDER BY accountid; +accountid|balance|balance2 +---------+-------+-------- +checking | 900| 1800 +savings | 600| 1200 +(2 rows) + + +starting permutation: tidrange1 tidrange2 c1 c2 read +step tidrange1: UPDATE accounts SET balance = balance + 100 WHERE ctid < '(0,2)' RETURNING accountid, balance; +accountid|balance +---------+------- +checking | 700 +(1 row) + +step tidrange2: UPDATE accounts SET balance = balance + 200 WHERE ctid < '(0,2)' RETURNING accountid, balance; +step c1: COMMIT; +step tidrange2: <... completed> +accountid|balance +---------+------- +(0 rows) + +step c2: COMMIT; +step read: SELECT * FROM accounts ORDER BY accountid; +accountid|balance|balance2 +---------+-------+-------- +checking | 700| 1400 +savings | 600| 1200 +(2 rows) + + starting permutation: simplepartupdate conditionalpartupdate c1 c2 read_part step simplepartupdate: update parttbl set b = b + 10; diff --git a/src/test/isolation/specs/eval-plan-qual.spec b/src/test/isolation/specs/eval-plan-qual.spec index 07307e623e4..c4f98c78f68 100644 --- a/src/test/isolation/specs/eval-plan-qual.spec +++ b/src/test/isolation/specs/eval-plan-qual.spec @@ -99,6 +99,11 @@ step upsert1 { WHERE NOT EXISTS (SELECT 1 FROM upsert); } +# ctid predicate +# same filter in both sessions; only one should succeed +step tid1 { UPDATE accounts SET balance = balance + 100 WHERE ctid = '(0,1)' RETURNING accountid, balance; } +step tidrange1 { UPDATE accounts SET balance = balance + 100 WHERE ctid < '(0,2)' RETURNING accountid, balance; } + # tests with table p check inheritance cases: # readp1/writep1/readp2 tests a bug where nodeLockRows did the wrong thing # when the first updated tuple was in a non-first child table. @@ -241,6 +246,11 @@ step updateforcip3 { step wrtwcte { UPDATE table_a SET value = 'tableAValue2' WHERE id = 1; } step wrjt { UPDATE jointest SET data = 42 WHERE id = 7; } +step tid2 { UPDATE accounts SET balance = balance + 200 WHERE ctid = '(0,1)' RETURNING accountid, balance; } +step tidrange2 { UPDATE accounts SET balance = balance + 200 WHERE ctid < '(0,2)' RETURNING accountid, balance; } +# here, recheck succeeds; (0,3) is the id that step tid1 will assign +step tidsucceed2 { UPDATE accounts SET balance = balance + 200 WHERE ctid = '(0,1)' OR ctid = '(0,3)' RETURNING accountid, balance; } + step conditionalpartupdate { update parttbl set c = -c where b < 10; } @@ -392,6 +402,9 @@ permutation wrtwcte readwcte c1 c2 permutation wrjt selectjoinforupdate c2 c1 permutation wrjt selectresultforupdate c2 c1 permutation wrtwcte multireadwcte c1 c2 +permutation tid1 tid2 c1 c2 read +permutation tid1 tidsucceed2 c1 c2 read +permutation tidrange1 tidrange2 c1 c2 read permutation simplepartupdate conditionalpartupdate c1 c2 read_part permutation simplepartupdate complexpartupdate c1 c2 read_part -- 2.39.5 (Apple Git-154)