From 0a7abcd4c983e25a4efb46a4a4942aef92c70338 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 13 Oct 2008 00:41:41 +0000 Subject: [PATCH] Fix corner case wherein a WorkTableScan node could get initialized before the RecursiveUnion to which it refers. It turns out that we can just postpone the relevant initialization steps until the first exec call for the node, by which time the ancestor node must surely be initialized. Per report from Greg Stark. --- src/backend/executor/nodeWorktablescan.c | 64 +++++++++++------- src/test/regress/expected/with.out | 83 ++++++++++++++++++++++++ src/test/regress/sql/with.sql | 21 ++++++ 3 files changed, 144 insertions(+), 24 deletions(-) diff --git a/src/backend/executor/nodeWorktablescan.c b/src/backend/executor/nodeWorktablescan.c index 2f09c5e22d..a597240fe6 100644 --- a/src/backend/executor/nodeWorktablescan.c +++ b/src/backend/executor/nodeWorktablescan.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/executor/nodeWorktablescan.c,v 1.1 2008/10/04 21:56:53 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/executor/nodeWorktablescan.c,v 1.2 2008/10/13 00:41:40 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -63,6 +63,40 @@ WorkTableScanNext(WorkTableScanState *node) TupleTableSlot * ExecWorkTableScan(WorkTableScanState *node) { + /* + * On the first call, find the ancestor RecursiveUnion's state + * via the Param slot reserved for it. (We can't do this during node + * init because there are corner cases where we'll get the init call + * before the RecursiveUnion does.) + */ + if (node->rustate == NULL) + { + WorkTableScan *plan = (WorkTableScan *) node->ss.ps.plan; + EState *estate = node->ss.ps.state; + ParamExecData *param; + + param = &(estate->es_param_exec_vals[plan->wtParam]); + Assert(param->execPlan == NULL); + Assert(!param->isnull); + node->rustate = (RecursiveUnionState *) DatumGetPointer(param->value); + Assert(node->rustate && IsA(node->rustate, RecursiveUnionState)); + + /* + * The scan tuple type (ie, the rowtype we expect to find in the work + * table) is the same as the result rowtype of the ancestor + * RecursiveUnion node. Note this depends on the assumption that + * RecursiveUnion doesn't allow projection. + */ + ExecAssignScanType(&node->ss, + ExecGetResultType(&node->rustate->ps)); + + /* + * Now we can initialize the projection info. This must be + * completed before we can call ExecScan(). + */ + ExecAssignScanProjectionInfo(&node->ss); + } + /* * use WorkTableScanNext as access method */ @@ -78,7 +112,6 @@ WorkTableScanState * ExecInitWorkTableScan(WorkTableScan *node, EState *estate, int eflags) { WorkTableScanState *scanstate; - ParamExecData *prmdata; /* check for unsupported flags */ Assert(!(eflags & EXEC_FLAG_MARK)); @@ -95,16 +128,7 @@ ExecInitWorkTableScan(WorkTableScan *node, EState *estate, int eflags) scanstate = makeNode(WorkTableScanState); scanstate->ss.ps.plan = (Plan *) node; scanstate->ss.ps.state = estate; - - /* - * Find the ancestor RecursiveUnion's state - * via the Param slot reserved for it. - */ - prmdata = &(estate->es_param_exec_vals[node->wtParam]); - Assert(prmdata->execPlan == NULL); - Assert(!prmdata->isnull); - scanstate->rustate = (RecursiveUnionState *) DatumGetPointer(prmdata->value); - Assert(scanstate->rustate && IsA(scanstate->rustate, RecursiveUnionState)); + scanstate->rustate = NULL; /* we'll set this later */ /* * Miscellaneous initialization @@ -132,19 +156,9 @@ ExecInitWorkTableScan(WorkTableScan *node, EState *estate, int eflags) ExecInitScanTupleSlot(estate, &scanstate->ss); /* - * The scan tuple type (ie, the rowtype we expect to find in the work - * table) is the same as the result rowtype of the ancestor RecursiveUnion - * node. Note this depends on the assumption that RecursiveUnion doesn't - * allow projection. - */ - ExecAssignScanType(&scanstate->ss, - ExecGetResultType(&scanstate->rustate->ps)); - - /* - * Initialize result tuple type and projection info. + * Initialize result tuple type, but not yet projection info. */ ExecAssignResultTypeFromTL(&scanstate->ss.ps); - ExecAssignScanProjectionInfo(&scanstate->ss); scanstate->ss.ps.ps_TupFromTlist = false; @@ -190,5 +204,7 @@ void ExecWorkTableScanReScan(WorkTableScanState *node, ExprContext *exprCtxt) { ExecClearTuple(node->ss.ps.ps_ResultTupleSlot); - tuplestore_rescan(node->rustate->working_table); + /* No need (or way) to rescan if ExecWorkTableScan not called yet */ + if (node->rustate) + tuplestore_rescan(node->rustate->working_table); } diff --git a/src/test/regress/expected/with.out b/src/test/regress/expected/with.out index 4760aa9d9f..fe7561065e 100644 --- a/src/test/regress/expected/with.out +++ b/src/test/regress/expected/with.out @@ -292,6 +292,89 @@ SELECT pg_get_viewdef('vsubdepartment'::regclass, true); FROM subdepartment; (1 row) +-- corner case in which sub-WITH gets initialized first +with recursive q as ( + select * from department + union all + (with x as (select * from q) + select * from x) + ) +select * from q limit 24; + id | parent_department | name +----+-------------------+------ + 0 | | ROOT + 1 | 0 | A + 2 | 1 | B + 3 | 2 | C + 4 | 2 | D + 5 | 0 | E + 6 | 4 | F + 7 | 5 | G + 0 | | ROOT + 1 | 0 | A + 2 | 1 | B + 3 | 2 | C + 4 | 2 | D + 5 | 0 | E + 6 | 4 | F + 7 | 5 | G + 0 | | ROOT + 1 | 0 | A + 2 | 1 | B + 3 | 2 | C + 4 | 2 | D + 5 | 0 | E + 6 | 4 | F + 7 | 5 | G +(24 rows) + +with recursive q as ( + select * from department + union all + (with recursive x as ( + select * from department + union all + (select * from q union all select * from x) + ) + select * from x) + ) +select * from q limit 32; + id | parent_department | name +----+-------------------+------ + 0 | | ROOT + 1 | 0 | A + 2 | 1 | B + 3 | 2 | C + 4 | 2 | D + 5 | 0 | E + 6 | 4 | F + 7 | 5 | G + 0 | | ROOT + 1 | 0 | A + 2 | 1 | B + 3 | 2 | C + 4 | 2 | D + 5 | 0 | E + 6 | 4 | F + 7 | 5 | G + 0 | | ROOT + 1 | 0 | A + 2 | 1 | B + 3 | 2 | C + 4 | 2 | D + 5 | 0 | E + 6 | 4 | F + 7 | 5 | G + 0 | | ROOT + 1 | 0 | A + 2 | 1 | B + 3 | 2 | C + 4 | 2 | D + 5 | 0 | E + 6 | 4 | F + 7 | 5 | G +(32 rows) + -- recursive term has sub-UNION WITH RECURSIVE t(i,j) AS ( VALUES (1,2) diff --git a/src/test/regress/sql/with.sql b/src/test/regress/sql/with.sql index 60c545d067..54d311101d 100644 --- a/src/test/regress/sql/with.sql +++ b/src/test/regress/sql/with.sql @@ -178,6 +178,27 @@ SELECT * FROM vsubdepartment ORDER BY name; SELECT pg_get_viewdef('vsubdepartment'::regclass); SELECT pg_get_viewdef('vsubdepartment'::regclass, true); +-- corner case in which sub-WITH gets initialized first +with recursive q as ( + select * from department + union all + (with x as (select * from q) + select * from x) + ) +select * from q limit 24; + +with recursive q as ( + select * from department + union all + (with recursive x as ( + select * from department + union all + (select * from q union all select * from x) + ) + select * from x) + ) +select * from q limit 32; + -- recursive term has sub-UNION WITH RECURSIVE t(i,j) AS ( VALUES (1,2)