Skip to content

Commit ac06ea8

Browse files
committed
Add missing EPQ recheck for TID Range Scan
The EvalPlanQual recheck for TID Range Scan wasn't rechecking the TID qual still passed after following update chains. This could result in tuples being updated or deleted by plans using TID Range Scans where the ctid of the new (updated) tuple no longer matches the clause of the scan. This isn't desired behavior, and isn't consistent with what would happen if the chosen plan had used an Index or Seq Scan, and that could lead to hard to predict behavior for scans that contain TID quals and other quals as the planner has freedom to choose TID Range or some other non-TID scan method for such queries, and the chosen plan could change at any moment. Here we fix this by properly implementing the recheck function for TID Range Scans. Backpatch to 14, where TID Range Scans were added Reported-by: Sophie Alpert <pg@sophiebits.com> Author: Sophie Alpert <pg@sophiebits.com> Author: David Rowley <dgrowleyml@gmail.com> Reviewed-by: David Rowley <dgrowleyml@gmail.com> Reviewed-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://postgr.es/m/4a6268ff-3340-453a-9bf5-c98d51a6f729@app.fastmail.com Backpatch-through: 14
1 parent dee21ea commit ac06ea8

File tree

3 files changed

+37
-1
lines changed

3 files changed

+37
-1
lines changed

src/backend/executor/nodeTidrangescan.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,16 @@ TidRangeNext(TidRangeScanState *node)
274274
static bool
275275
TidRangeRecheck(TidRangeScanState *node, TupleTableSlot *slot)
276276
{
277+
if (!TidRangeEval(node))
278+
return false;
279+
280+
Assert(ItemPointerIsValid(&slot->tts_tid));
281+
282+
/* Recheck the ctid is still within range */
283+
if (ItemPointerCompare(&slot->tts_tid, &node->trss_mintid) < 0 ||
284+
ItemPointerCompare(&slot->tts_tid, &node->trss_maxtid) > 0)
285+
return false;
286+
277287
return true;
278288
}
279289

src/test/isolation/expected/eval-plan-qual.out

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1265,6 +1265,29 @@ savings | 600| 1200
12651265
(2 rows)
12661266

12671267

1268+
starting permutation: tidrange1 tidrange2 c1 c2 read
1269+
step tidrange1: UPDATE accounts SET balance = balance + 100 WHERE ctid BETWEEN '(0,1)' AND '(0,1)' RETURNING accountid, balance;
1270+
accountid|balance
1271+
---------+-------
1272+
checking | 700
1273+
(1 row)
1274+
1275+
step tidrange2: UPDATE accounts SET balance = balance + 200 WHERE ctid BETWEEN '(0,1)' AND '(0,1)' RETURNING accountid, balance; <waiting ...>
1276+
step c1: COMMIT;
1277+
step tidrange2: <... completed>
1278+
accountid|balance
1279+
---------+-------
1280+
(0 rows)
1281+
1282+
step c2: COMMIT;
1283+
step read: SELECT * FROM accounts ORDER BY accountid;
1284+
accountid|balance|balance2
1285+
---------+-------+--------
1286+
checking | 700| 1400
1287+
savings | 600| 1200
1288+
(2 rows)
1289+
1290+
12681291
starting permutation: tid1 tid2 r1 c2 read
12691292
step tid1: UPDATE accounts SET balance = balance + 100 WHERE ctid = '(0,1)' RETURNING accountid, balance;
12701293
accountid|balance

src/test/isolation/specs/eval-plan-qual.spec

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,9 @@ step upsert1 {
9999
WHERE NOT EXISTS (SELECT 1 FROM upsert);
100100
}
101101

102-
# Tests for Tid Scan
102+
# Tests for Tid / Tid Range Scan
103103
step tid1 { UPDATE accounts SET balance = balance + 100 WHERE ctid = '(0,1)' RETURNING accountid, balance; }
104+
step tidrange1 { UPDATE accounts SET balance = balance + 100 WHERE ctid BETWEEN '(0,1)' AND '(0,1)' RETURNING accountid, balance; }
104105

105106
# tests with table p check inheritance cases:
106107
# readp1/writep1/readp2 tests a bug where nodeLockRows did the wrong thing
@@ -245,6 +246,7 @@ step wrtwcte { UPDATE table_a SET value = 'tableAValue2' WHERE id = 1; }
245246
step wrjt { UPDATE jointest SET data = 42 WHERE id = 7; }
246247

247248
step tid2 { UPDATE accounts SET balance = balance + 200 WHERE ctid = '(0,1)' RETURNING accountid, balance; }
249+
step tidrange2 { UPDATE accounts SET balance = balance + 200 WHERE ctid BETWEEN '(0,1)' AND '(0,1)' RETURNING accountid, balance; }
248250
# here, recheck succeeds; (0,3) is the id that step tid1 will assign
249251
step tidsucceed2 { UPDATE accounts SET balance = balance + 200 WHERE ctid = '(0,1)' OR ctid = '(0,3)' RETURNING accountid, balance; }
250252

@@ -401,6 +403,7 @@ permutation wrjt selectresultforupdate c2 c1
401403
permutation wrtwcte multireadwcte c1 c2
402404
permutation tid1 tid2 c1 c2 read
403405
permutation tid1 tidsucceed2 c1 c2 read
406+
permutation tidrange1 tidrange2 c1 c2 read
404407
# test that a rollback on s1 has s2 perform the update on the original row
405408
permutation tid1 tid2 r1 c2 read
406409

0 commit comments

Comments
 (0)