I wrote:
> The reason this happens is that EvalPlanQualBegin copies down all the
> es_param_exec_vals values from the outer query into the EPQ execution
> state. That's OK for most uses of es_param_exec_vals values, but not
> at all for cteParam Params, which are used as internal communication
> variables within a plan tree.
After further study and caffeine-consumption, I concluded that that's
wrong. There's nothing wrong with reusing the result of a CTE that has
already been scanned by the original query invocation (its output can't
be affected by the new candidate-for-update row). The problem is
that ExecInitCteScan is assuming that when it initializes as a follower,
the tuplestore read pointer it's given by tuplestore_alloc_read_pointer
will be pointing to the start of the tuplestore. That's wrong; the new
read pointer is defined as being a clone of read pointer 0, which is
already at EOF in this scenario. So a simple and localized fix is to
forcibly rewind the new read pointer, as in the attached patch. (This
should have negligible cost as long as the tuplestore hasn't yet spilled
to disk.)
I also considered redefining tuplestore_alloc_read_pointer as creating
a read pointer that points to start-of-tuplestore. The other existing
callers wouldn't care, because they are working with a just-created,
known-empty tuplestore. But there is a risk of breaking third-party
code, so this didn't seem like a back-patchable solution. Also, I think
the reason why tuplestore_alloc_read_pointer was given this behavior is
so that it could succeed even if the tuplestore has already been moved
forward and perhaps had old data truncated off it. With the other
behavior, it would have to have the same failure cases as
tuplestore_rescan.
BTW, I no longer think the recursive-union case is broken; rather, failure
to copy its communication Param would break it, in scenarios where a
WorkTableScan is underneath a SELECT FOR UPDATE that's underneath the
RecursiveUnion. So that's another reason not to mess with the Param
propagation logic.
regards, tom lane
diff --git a/src/backend/executor/nodeCtescan.c b/src/backend/executor/nodeCtescan.c
index 3c2f684..162650a 100644
*** a/src/backend/executor/nodeCtescan.c
--- b/src/backend/executor/nodeCtescan.c
*************** ExecInitCteScan(CteScan *node, EState *e
*** 224,232 ****
--- 224,236 ----
{
/* 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 5898d94..10c784a 100644
*** a/src/test/isolation/expected/eval-plan-qual.out
--- b/src/test/isolation/expected/eval-plan-qual.out
*************** ta_id ta_value tb_row
*** 163,165 ****
--- 163,186 ----
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 de481a3..7ff6f6b 100644
*** a/src/test/isolation/specs/eval-plan-qual.spec
--- b/src/test/isolation/specs/eval-plan-qual.spec
*************** step "readforss" {
*** 103,113 ****
--- 103,129 ----
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"
*************** permutation "writep2" "returningp1" "c1"
*** 118,120 ****
--- 134,137 ----
permutation "wx2" "partiallock" "c2" "c1" "read"
permutation "wx2" "lockwithvalues" "c2" "c1" "read"
permutation "updateforss" "readforss" "c1" "c2"
+ permutation "wrtwcte" "readwcte" "c1" "c2"
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
