From 8552f9b903a099d82763460cb7ca28715d0ad2bb Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 22 Sep 2016 11:34:44 -0400 Subject: [PATCH] Be sure to rewind the tuplestore read pointer in non-leader CTEScan nodes. ExecInitCteScan supposed that it didn't have to do anything to the extra tuplestore read pointer it gets from tuplestore_alloc_read_pointer. However, it needs this read pointer to be positioned at the start of the tuplestore, while tuplestore_alloc_read_pointer is actually defined as cloning the current position of read pointer 0. In normal situations that accidentally works because we initialize the whole plan tree at once, before anything gets read. But it fails in an EvalPlanQual recheck, as illustrated in bug #14328 from Dima Pavlov. To fix, just forcibly rewind the pointer after tuplestore_alloc_read_pointer. The cost of doing so is negligible unless the tuplestore is already in TSS_READFILE state, which wouldn't happen in normal cases. We could consider altering tuplestore's API to make that case cheaper, but that would make for a more invasive back-patch and it doesn't seem worth it. This has been broken probably for as long as we've had CTEs, so back-patch to all supported branches. Discussion: <32468.1474548308@sss.pgh.pa.us> --- src/backend/executor/nodeCtescan.c | 4 ++++ .../isolation/expected/eval-plan-qual.out | 21 +++++++++++++++++ src/test/isolation/specs/eval-plan-qual.spec | 23 +++++++++++++++++++ 3 files changed, 48 insertions(+) diff --git a/src/backend/executor/nodeCtescan.c b/src/backend/executor/nodeCtescan.c index 7e10b8a530d..f39bbf405f7 100644 --- a/src/backend/executor/nodeCtescan.c +++ b/src/backend/executor/nodeCtescan.c @@ -224,9 +224,13 @@ ExecInitCteScan(CteScan *node, EState *estate, int eflags) { /* Not the leader */ Assert(IsA(scanstate->leader, CteScanState)); + /* Create my own read pointer, and ensure it is at start */ scanstate->readptr = tuplestore_alloc_read_pointer(scanstate->leader->cte_table, scanstate->eflags); + tuplestore_select_read_pointer(scanstate->leader->cte_table, + scanstate->readptr); + tuplestore_rescan(scanstate->leader->cte_table); } /* diff --git a/src/test/isolation/expected/eval-plan-qual.out b/src/test/isolation/expected/eval-plan-qual.out index 433533e6117..99c4137e14a 100644 --- a/src/test/isolation/expected/eval-plan-qual.out +++ b/src/test/isolation/expected/eval-plan-qual.out @@ -104,3 +104,24 @@ a b c 2 2 2 2 3 0 step c2: COMMIT; + +starting permutation: wrtwcte readwcte c1 c2 +step wrtwcte: UPDATE table_a SET value = 'tableAValue2' WHERE id = 1; +step readwcte: + WITH + cte1 AS ( + SELECT id FROM table_b WHERE value = 'tableBValue' + ), + cte2 AS ( + SELECT * FROM table_a + WHERE id = (SELECT id FROM cte1) + FOR UPDATE + ) + SELECT * FROM cte2; + +step c1: COMMIT; +step c2: COMMIT; +step readwcte: <... completed> +id value + +1 tableAValue2 diff --git a/src/test/isolation/specs/eval-plan-qual.spec b/src/test/isolation/specs/eval-plan-qual.spec index 6fb24322863..f15dee385fc 100644 --- a/src/test/isolation/specs/eval-plan-qual.spec +++ b/src/test/isolation/specs/eval-plan-qual.spec @@ -16,12 +16,18 @@ setup INSERT INTO c1 SELECT 0, a / 3, a % 3 FROM generate_series(0, 9) a; INSERT INTO c2 SELECT 1, a / 3, a % 3 FROM generate_series(0, 9) a; INSERT INTO c3 SELECT 2, a / 3, a % 3 FROM generate_series(0, 9) a; + + CREATE TABLE table_a (id integer, value text); + CREATE TABLE table_b (id integer, value text); + INSERT INTO table_a VALUES (1, 'tableAValue'); + INSERT INTO table_b VALUES (1, 'tableBValue'); } teardown { DROP TABLE accounts; DROP TABLE p CASCADE; + DROP TABLE table_a, table_b; } session "s1" @@ -67,11 +73,27 @@ step "returningp1" { WITH u AS ( UPDATE p SET b = b WHERE a > 0 RETURNING * ) SELECT * FROM u; } +step "wrtwcte" { UPDATE table_a SET value = 'tableAValue2' WHERE id = 1; } step "c2" { COMMIT; } session "s3" setup { BEGIN ISOLATION LEVEL READ COMMITTED; } step "read" { SELECT * FROM accounts ORDER BY accountid; } + +# this test exercises EvalPlanQual with a CTE, cf bug #14328 +step "readwcte" { + WITH + cte1 AS ( + SELECT id FROM table_b WHERE value = 'tableBValue' + ), + cte2 AS ( + SELECT * FROM table_a + WHERE id = (SELECT id FROM cte1) + FOR UPDATE + ) + SELECT * FROM cte2; +} + teardown { COMMIT; } permutation "wx1" "wx2" "c1" "c2" "read" @@ -79,3 +101,4 @@ permutation "wy1" "wy2" "c1" "c2" "read" permutation "upsert1" "upsert2" "c1" "c2" "read" permutation "readp1" "writep1" "readp2" "c1" "c2" permutation "writep2" "returningp1" "c1" "c2" +permutation "wrtwcte" "readwcte" "c1" "c2"