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>

diff --git a/src/backend/executor/nodeCtescan.c b/src/backend/executor/nodeCtescan.c
index 3c2f684a062d577f5eb0c66e9b5e6cfc5a6f8289..162650ad8abe92a5772fbbdced47b4b3241d9fda 100644 (file)
--- 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 5898d94ff17f34c17cdcf220d183e74b5836d408..10c784a05f181f24e791db8824a5d70c25e00df8 100644 (file)
--- a/src/test/isolation/expected/eval-plan-qual.out
+++ b/src/test/isolation/expected/eval-plan-qual.out
@@ -163,3 +163,24 @@ ta_id          ta_value       tb_row
 
 1              newTableAValue (1,tableBValue)
 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;
+ <waiting ...>
+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 de481a3cecebd85b7084b1acb0d57941f40b5083..7ff6f6b8cc90e9bd5357ac7c1c1d0d9e8159cd3c 100644 (file)
--- a/src/test/isolation/specs/eval-plan-qual.spec
+++ b/src/test/isolation/specs/eval-plan-qual.spec
@@ -103,11 +103,27 @@ step "readforss"  {
        FROM table_a ta
        WHERE ta.id = 1 FOR UPDATE OF ta;
 }
+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"
@@ -118,3 +134,4 @@ permutation "writep2" "returningp1" "c1" "c2"
 permutation "wx2" "partiallock" "c2" "c1" "read"
 permutation "wx2" "lockwithvalues" "c2" "c1" "read"
 permutation "updateforss" "readforss" "c1" "c2"
+permutation "wrtwcte" "readwcte" "c1" "c2"