diff options
| -rw-r--r-- | src/backend/nodes/outfuncs.c | 5 | ||||
| -rw-r--r-- | src/backend/optimizer/path/allpaths.c | 10 | ||||
| -rw-r--r-- | src/backend/optimizer/plan/createplan.c | 9 | ||||
| -rw-r--r-- | src/backend/optimizer/plan/planner.c | 7 | ||||
| -rw-r--r-- | src/backend/optimizer/plan/subselect.c | 278 | ||||
| -rw-r--r-- | src/backend/optimizer/prep/prepjointree.c | 1 | ||||
| -rw-r--r-- | src/backend/optimizer/prep/prepunion.c | 10 | ||||
| -rw-r--r-- | src/include/nodes/relation.h | 71 | ||||
| -rw-r--r-- | src/test/regress/expected/with.out | 21 | ||||
| -rw-r--r-- | src/test/regress/sql/with.sql | 14 |
10 files changed, 247 insertions, 179 deletions
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 96b09fbce71..a66ea5e048e 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -1659,7 +1659,6 @@ _outPlannerGlobal(StringInfo str, PlannerGlobal *node) WRITE_NODE_TYPE("PLANNERGLOBAL"); /* NB: this isn't a complete set of fields */ - WRITE_NODE_FIELD(paramlist); WRITE_NODE_FIELD(subplans); WRITE_NODE_FIELD(subrtables); WRITE_NODE_FIELD(subrowmarks); @@ -1669,6 +1668,7 @@ _outPlannerGlobal(StringInfo str, PlannerGlobal *node) WRITE_NODE_FIELD(resultRelations); WRITE_NODE_FIELD(relationOids); WRITE_NODE_FIELD(invalItems); + WRITE_INT_FIELD(nParamExec); WRITE_UINT_FIELD(lastPHId); WRITE_UINT_FIELD(lastRowMarkId); WRITE_BOOL_FIELD(transientPlan); @@ -1683,6 +1683,7 @@ _outPlannerInfo(StringInfo str, PlannerInfo *node) WRITE_NODE_FIELD(parse); WRITE_NODE_FIELD(glob); WRITE_UINT_FIELD(query_level); + WRITE_NODE_FIELD(plan_params); WRITE_NODE_FIELD(join_rel_list); WRITE_INT_FIELD(join_cur_level); WRITE_NODE_FIELD(init_plans); @@ -1927,7 +1928,7 @@ _outPlannerParamItem(StringInfo str, PlannerParamItem *node) WRITE_NODE_TYPE("PLANNERPARAMITEM"); WRITE_NODE_FIELD(item); - WRITE_UINT_FIELD(abslevel); + WRITE_INT_FIELD(paramId); } /***************************************************************************** diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index a14b809a14c..7e3f166d152 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -786,6 +786,9 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel, else tuple_fraction = root->tuple_fraction; + /* plan_params should not be in use in current query level */ + Assert(root->plan_params == NIL); + /* Generate the plan for the subquery */ rel->subplan = subquery_planner(root->glob, subquery, root, @@ -794,6 +797,13 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel, rel->subrtable = subroot->parse->rtable; rel->subrowmark = subroot->rowMarks; + /* + * Since we don't yet support LATERAL, it should not be possible for the + * sub-query to have requested parameters of this level. + */ + if (root->plan_params) + elog(ERROR, "unexpected outer reference in subquery in FROM"); + /* Mark rel with estimated output rows, width, etc */ set_subquery_size_estimates(root, rel, subroot); diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index affcf264c35..e791f20dc12 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -185,6 +185,9 @@ create_plan(PlannerInfo *root, Path *best_path) { Plan *plan; + /* plan_params should not be in use in current query level */ + Assert(root->plan_params == NIL); + /* Initialize this module's private workspace in PlannerInfo */ root->curOuterRels = NULL; root->curOuterParams = NIL; @@ -196,6 +199,12 @@ create_plan(PlannerInfo *root, Path *best_path) if (root->curOuterParams != NIL) elog(ERROR, "failed to assign all NestLoopParams to plan nodes"); + /* + * Reset plan_params to ensure param IDs used for nestloop params are not + * re-used later + */ + root->plan_params = NIL; + return plan; } diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 5b4ce491a5c..b68d532d31a 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -156,7 +156,6 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams) glob = makeNode(PlannerGlobal); glob->boundParams = boundParams; - glob->paramlist = NIL; glob->subplans = NIL; glob->subrtables = NIL; glob->subrowmarks = NIL; @@ -166,6 +165,7 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams) glob->resultRelations = NIL; glob->relationOids = NIL; glob->invalItems = NIL; + glob->nParamExec = 0; glob->lastPHId = 0; glob->lastRowMarkId = 0; glob->transientPlan = false; @@ -254,7 +254,7 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams) result->rowMarks = glob->finalrowmarks; result->relationOids = glob->relationOids; result->invalItems = glob->invalItems; - result->nParamExec = list_length(glob->paramlist); + result->nParamExec = glob->nParamExec; return result; } @@ -306,6 +306,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse, root->glob = glob; root->query_level = parent_root ? parent_root->query_level + 1 : 1; root->parent_root = parent_root; + root->plan_params = NIL; root->planner_cxt = CurrentMemoryContext; root->init_plans = NIL; root->cte_plan_ids = NIL; @@ -577,7 +578,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse, * and attach the initPlans to the top plan node. */ if (list_length(glob->subplans) != num_old_subplans || - root->glob->paramlist != NIL) + root->glob->nParamExec > 0) SS_finalize_plan(root, plan, true); /* Return internal info if caller wants it */ diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c index 3eb4814c095..b28d87897c0 100644 --- a/src/backend/optimizer/plan/subselect.c +++ b/src/backend/optimizer/plan/subselect.c @@ -55,6 +55,7 @@ typedef struct finalize_primnode_context static Node *build_subplan(PlannerInfo *root, Plan *plan, List *rtable, List *rowmarks, + List *plan_params, SubLinkType subLinkType, Node *testexpr, bool adjust_testexpr, bool unknownEqFalse); static List *generate_subquery_params(PlannerInfo *root, List *tlist, @@ -83,35 +84,42 @@ static bool finalize_primnode(Node *node, finalize_primnode_context *context); /* - * Select a PARAM_EXEC number to identify the given Var. - * If the Var already has a param slot, return that one. + * Select a PARAM_EXEC number to identify the given Var as a parameter for + * the current subquery, or for a nestloop's inner scan. + * If the Var already has a param in the current context, return that one. */ static int assign_param_for_var(PlannerInfo *root, Var *var) { ListCell *ppl; PlannerParamItem *pitem; - Index abslevel; - int i; + Index levelsup; - abslevel = root->query_level - var->varlevelsup; + /* Find the query level the Var belongs to */ + for (levelsup = var->varlevelsup; levelsup > 0; levelsup--) + root = root->parent_root; - /* If there's already a paramlist entry for this same Var, just use it */ - i = 0; - foreach(ppl, root->glob->paramlist) + /* If there's already a matching PlannerParamItem there, just use it */ + foreach(ppl, root->plan_params) { pitem = (PlannerParamItem *) lfirst(ppl); - if (pitem->abslevel == abslevel && IsA(pitem->item, Var)) + if (IsA(pitem->item, Var)) { Var *pvar = (Var *) pitem->item; + /* + * This comparison must match _equalVar(), except for ignoring + * varlevelsup. Note that _equalVar() ignores the location. + */ if (pvar->varno == var->varno && pvar->varattno == var->varattno && pvar->vartype == var->vartype && - pvar->vartypmod == var->vartypmod) - return i; + pvar->vartypmod == var->vartypmod && + pvar->varcollid == var->varcollid && + pvar->varnoold == var->varnoold && + pvar->varoattno == var->varoattno) + return pitem->paramId; } - i++; } /* Nope, so make a new one */ @@ -120,12 +128,11 @@ assign_param_for_var(PlannerInfo *root, Var *var) pitem = makeNode(PlannerParamItem); pitem->item = (Node *) var; - pitem->abslevel = abslevel; + pitem->paramId = root->glob->nParamExec++; - root->glob->paramlist = lappend(root->glob->paramlist, pitem); + root->plan_params = lappend(root->plan_params, pitem); - /* i is already the correct list index for the new item */ - return i; + return pitem->paramId; } /* @@ -140,16 +147,7 @@ replace_outer_var(PlannerInfo *root, Var *var) Assert(var->varlevelsup > 0 && var->varlevelsup < root->query_level); - /* - * Find the Var in root->glob->paramlist, or add it if not present. - * - * NOTE: in sufficiently complex querytrees, it is possible for the same - * varno/abslevel to refer to different RTEs in different parts of the - * parsetree, so that different fields might end up sharing the same Param - * number. As long as we check the vartype/typmod as well, I believe that - * this sort of aliasing will cause no trouble. The correct field should - * get stored into the Param slot at execution in each part of the tree. - */ + /* Find the Var in the appropriate plan_params, or add it if not present */ i = assign_param_for_var(root, var); retval = makeNode(Param); @@ -158,7 +156,7 @@ replace_outer_var(PlannerInfo *root, Var *var) retval->paramtype = var->vartype; retval->paramtypmod = var->vartypmod; retval->paramcollid = var->varcollid; - retval->location = -1; + retval->location = var->location; return retval; } @@ -167,8 +165,7 @@ replace_outer_var(PlannerInfo *root, Var *var) * Generate a Param node to replace the given Var, which will be supplied * from an upper NestLoop join node. * - * Because we allow nestloop and subquery Params to alias each other, - * this is effectively the same as replace_outer_var, except that we expect + * This is effectively the same as replace_outer_var, except that we expect * the Var to be local to the current query level. */ Param * @@ -187,14 +184,15 @@ assign_nestloop_param_var(PlannerInfo *root, Var *var) retval->paramtype = var->vartype; retval->paramtypmod = var->vartypmod; retval->paramcollid = var->varcollid; - retval->location = -1; + retval->location = var->location; return retval; } /* - * Select a PARAM_EXEC number to identify the given PlaceHolderVar. - * If the PlaceHolderVar already has a param slot, return that one. + * Select a PARAM_EXEC number to identify the given PlaceHolderVar as a + * parameter for the current subquery, or for a nestloop's inner scan. + * If the PHV already has a param in the current context, return that one. * * This is just like assign_param_for_var, except for PlaceHolderVars. */ @@ -203,25 +201,24 @@ assign_param_for_placeholdervar(PlannerInfo *root, PlaceHolderVar *phv) { ListCell *ppl; PlannerParamItem *pitem; - Index abslevel; - int i; + Index levelsup; - abslevel = root->query_level - phv->phlevelsup; + /* Find the query level the PHV belongs to */ + for (levelsup = phv->phlevelsup; levelsup > 0; levelsup--) + root = root->parent_root; - /* If there's already a paramlist entry for this same PHV, just use it */ - i = 0; - foreach(ppl, root->glob->paramlist) + /* If there's already a matching PlannerParamItem there, just use it */ + foreach(ppl, root->plan_params) { pitem = (PlannerParamItem *) lfirst(ppl); - if (pitem->abslevel == abslevel && IsA(pitem->item, PlaceHolderVar)) + if (IsA(pitem->item, PlaceHolderVar)) { PlaceHolderVar *pphv = (PlaceHolderVar *) pitem->item; /* We assume comparing the PHIDs is sufficient */ if (pphv->phid == phv->phid) - return i; + return pitem->paramId; } - i++; } /* Nope, so make a new one */ @@ -234,12 +231,11 @@ assign_param_for_placeholdervar(PlannerInfo *root, PlaceHolderVar *phv) pitem = makeNode(PlannerParamItem); pitem->item = (Node *) phv; - pitem->abslevel = abslevel; + pitem->paramId = root->glob->nParamExec++; - root->glob->paramlist = lappend(root->glob->paramlist, pitem); + root->plan_params = lappend(root->plan_params, pitem); - /* i is already the correct list index for the new item */ - return i; + return pitem->paramId; } /* @@ -256,10 +252,7 @@ replace_outer_placeholdervar(PlannerInfo *root, PlaceHolderVar *phv) Assert(phv->phlevelsup > 0 && phv->phlevelsup < root->query_level); - /* - * Find the PlaceHolderVar in root->glob->paramlist, or add it if not - * present. - */ + /* Find the PHV in the appropriate plan_params, or add it if not present */ i = assign_param_for_placeholdervar(root, phv); retval = makeNode(Param); @@ -309,11 +302,13 @@ replace_outer_agg(PlannerInfo *root, Aggref *agg) { Param *retval; PlannerParamItem *pitem; - Index abslevel; - int i; + Index levelsup; Assert(agg->agglevelsup > 0 && agg->agglevelsup < root->query_level); - abslevel = root->query_level - agg->agglevelsup; + + /* Find the query level the Aggref belongs to */ + for (levelsup = agg->agglevelsup; levelsup > 0; levelsup--) + root = root->parent_root; /* * It does not seem worthwhile to try to match duplicate outer aggs. Just @@ -325,18 +320,17 @@ replace_outer_agg(PlannerInfo *root, Aggref *agg) pitem = makeNode(PlannerParamItem); pitem->item = (Node *) agg; - pitem->abslevel = abslevel; + pitem->paramId = root->glob->nParamExec++; - root->glob->paramlist = lappend(root->glob->paramlist, pitem); - i = list_length(root->glob->paramlist) - 1; + root->plan_params = lappend(root->plan_params, pitem); retval = makeNode(Param); retval->paramkind = PARAM_EXEC; - retval->paramid = i; + retval->paramid = pitem->paramId; retval->paramtype = agg->aggtype; retval->paramtypmod = -1; retval->paramcollid = agg->aggcollid; - retval->location = -1; + retval->location = agg->location; return retval; } @@ -344,29 +338,24 @@ replace_outer_agg(PlannerInfo *root, Aggref *agg) /* * Generate a new Param node that will not conflict with any other. * - * This is used to allocate PARAM_EXEC slots for subplan outputs. + * This is used to create Params representing subplan outputs. + * We don't need to build a PlannerParamItem for such a Param, but we do + * need to record the PARAM_EXEC slot number as being allocated. */ static Param * generate_new_param(PlannerInfo *root, Oid paramtype, int32 paramtypmod, Oid paramcollation) { Param *retval; - PlannerParamItem *pitem; retval = makeNode(Param); retval->paramkind = PARAM_EXEC; - retval->paramid = list_length(root->glob->paramlist); + retval->paramid = root->glob->nParamExec++; retval->paramtype = paramtype; retval->paramtypmod = paramtypmod; retval->paramcollid = paramcollation; retval->location = -1; - pitem = makeNode(PlannerParamItem); - pitem->item = (Node *) retval; - pitem->abslevel = root->query_level; - - root->glob->paramlist = lappend(root->glob->paramlist, pitem); - return retval; } @@ -375,17 +364,13 @@ generate_new_param(PlannerInfo *root, Oid paramtype, int32 paramtypmod, * is not actually used to carry a value at runtime). Such parameters are * used for special runtime signaling purposes, such as connecting a * recursive union node to its worktable scan node or forcing plan - * re-evaluation within the EvalPlanQual mechanism. + * re-evaluation within the EvalPlanQual mechanism. No actual Param node + * exists with this ID, however. */ int SS_assign_special_param(PlannerInfo *root) { - Param *param; - - /* We generate a Param of datatype INTERNAL */ - param = generate_new_param(root, INTERNALOID, -1, InvalidOid); - /* ... but the caller only cares about its ID */ - return param->paramid; + return root->glob->nParamExec++; } /* @@ -446,6 +431,7 @@ make_subplan(PlannerInfo *root, Query *orig_subquery, SubLinkType subLinkType, double tuple_fraction; Plan *plan; PlannerInfo *subroot; + List *plan_params; Node *result; /* @@ -489,6 +475,9 @@ make_subplan(PlannerInfo *root, Query *orig_subquery, SubLinkType subLinkType, else tuple_fraction = 0.0; /* default behavior */ + /* plan_params should not be in use in current query level */ + Assert(root->plan_params == NIL); + /* * Generate the plan for the subquery. */ @@ -497,9 +486,14 @@ make_subplan(PlannerInfo *root, Query *orig_subquery, SubLinkType subLinkType, false, tuple_fraction, &subroot); + /* Isolate the params needed by this specific subplan */ + plan_params = root->plan_params; + root->plan_params = NIL; + /* And convert to SubPlan or InitPlan format. */ result = build_subplan(root, plan, subroot->parse->rtable, subroot->rowMarks, + plan_params, subLinkType, testexpr, true, isTopQual); /* @@ -532,6 +526,10 @@ make_subplan(PlannerInfo *root, Query *orig_subquery, SubLinkType subLinkType, false, 0.0, &subroot); + /* Isolate the params needed by this specific subplan */ + plan_params = root->plan_params; + root->plan_params = NIL; + /* Now we can check if it'll fit in work_mem */ if (subplan_is_hashable(plan)) { @@ -542,6 +540,7 @@ make_subplan(PlannerInfo *root, Query *orig_subquery, SubLinkType subLinkType, hashplan = (SubPlan *) build_subplan(root, plan, subroot->parse->rtable, subroot->rowMarks, + plan_params, ANY_SUBLINK, newtestexpr, false, true); /* Check we got what we expected */ @@ -570,14 +569,14 @@ make_subplan(PlannerInfo *root, Query *orig_subquery, SubLinkType subLinkType, */ static Node * build_subplan(PlannerInfo *root, Plan *plan, List *rtable, List *rowmarks, + List *plan_params, SubLinkType subLinkType, Node *testexpr, bool adjust_testexpr, bool unknownEqFalse) { Node *result; SubPlan *splan; bool isInitPlan; - Bitmapset *tmpset; - int paramid; + ListCell *lc; /* * Initialize the SubPlan node. Note plan_id, plan_name, and cost fields @@ -599,36 +598,26 @@ build_subplan(PlannerInfo *root, Plan *plan, List *rtable, List *rowmarks, * Make parParam and args lists of param IDs and expressions that current * query level will pass to this child plan. */ - tmpset = bms_copy(plan->extParam); - while ((paramid = bms_first_member(tmpset)) >= 0) + foreach(lc, plan_params) { - PlannerParamItem *pitem = list_nth(root->glob->paramlist, paramid); - - if (pitem->abslevel == root->query_level) - { - Node *arg; - - /* - * The Var, PlaceHolderVar, or Aggref has already been adjusted to - * have the correct varlevelsup, phlevelsup, or agglevelsup. We - * probably don't even need to copy it again, but be safe. - */ - arg = copyObject(pitem->item); + PlannerParamItem *pitem = (PlannerParamItem *) lfirst(lc); + Node *arg = pitem->item; - /* - * If it's a PlaceHolderVar or Aggref, its arguments might contain - * SubLinks, which have not yet been processed (see the comments - * for SS_replace_correlation_vars). Do that now. - */ - if (IsA(arg, PlaceHolderVar) || - IsA(arg, Aggref)) - arg = SS_process_sublinks(root, arg, false); + /* + * The Var, PlaceHolderVar, or Aggref has already been adjusted to + * have the correct varlevelsup, phlevelsup, or agglevelsup. + * + * If it's a PlaceHolderVar or Aggref, its arguments might contain + * SubLinks, which have not yet been processed (see the comments for + * SS_replace_correlation_vars). Do that now. + */ + if (IsA(arg, PlaceHolderVar) || + IsA(arg, Aggref)) + arg = SS_process_sublinks(root, arg, false); - splan->parParam = lappend_int(splan->parParam, paramid); - splan->args = lappend(splan->args, arg); - } + splan->parParam = lappend_int(splan->parParam, pitem->paramId); + splan->args = lappend(splan->args, arg); } - bms_free(tmpset); /* * Un-correlated or undirect correlated plans of EXISTS, EXPR, ARRAY, or @@ -1050,9 +1039,7 @@ SS_process_ctes(PlannerInfo *root) Plan *plan; PlannerInfo *subroot; SubPlan *splan; - Bitmapset *tmpset; int paramid; - Param *prm; /* * Ignore SELECT CTEs that are not actually referenced anywhere. @@ -1070,6 +1057,9 @@ SS_process_ctes(PlannerInfo *root) */ subquery = (Query *) copyObject(cte->ctequery); + /* plan_params should not be in use in current query level */ + Assert(root->plan_params == NIL); + /* * Generate the plan for the CTE query. Always plan for full * retrieval --- we don't have enough info to predict otherwise. @@ -1080,6 +1070,14 @@ SS_process_ctes(PlannerInfo *root) &subroot); /* + * Since the current query level doesn't yet contain any RTEs, it + * should not be possible for the CTE to have requested parameters of + * this level. + */ + if (root->plan_params) + elog(ERROR, "unexpected outer reference in CTE query"); + + /* * Make a SubPlan node for it. This is just enough unlike * build_subplan that we can't share code. * @@ -1098,35 +1096,22 @@ SS_process_ctes(PlannerInfo *root) splan->args = NIL; /* - * Make parParam and args lists of param IDs and expressions that - * current query level will pass to this child plan. Even though this - * is an initplan, there could be side-references to earlier - * initplan's outputs, specifically their CTE output parameters. + * The node can't have any inputs (since it's an initplan), so the + * parParam and args lists remain empty. (It could contain references + * to earlier CTEs' output param IDs, but CTE outputs are not + * propagated via the args list.) */ - tmpset = bms_copy(plan->extParam); - while ((paramid = bms_first_member(tmpset)) >= 0) - { - PlannerParamItem *pitem = list_nth(root->glob->paramlist, paramid); - - if (pitem->abslevel == root->query_level) - { - prm = (Param *) pitem->item; - if (!IsA(prm, Param) || - prm->paramtype != INTERNALOID) - elog(ERROR, "bogus local parameter passed to WITH query"); - - splan->parParam = lappend_int(splan->parParam, paramid); - splan->args = lappend(splan->args, copyObject(prm)); - } - } - bms_free(tmpset); /* - * Assign a param to represent the query output. We only really care - * about reserving a parameter ID number. + * Assign a param ID to represent the CTE's output. No ordinary + * "evaluation" of this param slot ever happens, but we use the param + * ID for setParam/chgParam signaling just as if the CTE plan were + * returning a simple scalar output. (Also, the executor abuses the + * ParamExecData slot for this param ID for communication among + * multiple CteScan nodes that might be scanning this CTE.) */ - prm = generate_new_param(root, INTERNALOID, -1, InvalidOid); - splan->setParam = list_make1_int(prm->paramid); + paramid = SS_assign_special_param(root); + splan->setParam = list_make1_int(paramid); /* * Add the subplan and its rtable to the global lists. @@ -1940,7 +1925,7 @@ SS_finalize_plan(PlannerInfo *root, Plan *plan, bool attach_initplans) *initExtParam, *initSetParam; Cost initplan_cost; - int paramid; + PlannerInfo *proot; ListCell *l; /* @@ -1972,31 +1957,36 @@ SS_finalize_plan(PlannerInfo *root, Plan *plan, bool attach_initplans) /* * Now determine the set of params that are validly referenceable in this * query level; to wit, those available from outer query levels plus the - * output parameters of any initPlans. (We do not include output + * output parameters of any local initPlans. (We do not include output * parameters of regular subplans. Those should only appear within the * testexpr of SubPlan nodes, and are taken care of locally within * finalize_primnode. Likewise, special parameters that are generated by * nodes such as ModifyTable are handled within finalize_plan.) - * - * Note: this is a bit overly generous since some parameters of upper - * query levels might belong to query subtrees that don't include this - * query, or might be nestloop params that won't be passed down at all. - * However, valid_params is only a debugging crosscheck, so it doesn't - * seem worth expending lots of cycles to try to be exact. */ valid_params = bms_copy(initSetParam); - paramid = 0; - foreach(l, root->glob->paramlist) + for (proot = root->parent_root; proot != NULL; proot = proot->parent_root) { - PlannerParamItem *pitem = (PlannerParamItem *) lfirst(l); - - if (pitem->abslevel < root->query_level) + /* Include ordinary Var/PHV/Aggref params */ + foreach(l, proot->plan_params) { - /* valid outer-level parameter */ - valid_params = bms_add_member(valid_params, paramid); + PlannerParamItem *pitem = (PlannerParamItem *) lfirst(l); + + valid_params = bms_add_member(valid_params, pitem->paramId); } + /* Include any outputs of outer-level initPlans */ + foreach(l, proot->init_plans) + { + SubPlan *initsubplan = (SubPlan *) lfirst(l); + ListCell *l2; - paramid++; + foreach(l2, initsubplan->setParam) + { + valid_params = bms_add_member(valid_params, lfirst_int(l2)); + } + } + /* Include worktable ID, if a recursive query is being planned */ + if (proot->wt_param_id >= 0) + valid_params = bms_add_member(valid_params, proot->wt_param_id); } /* diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c index ad0580b7723..1750a5e3e1f 100644 --- a/src/backend/optimizer/prep/prepjointree.c +++ b/src/backend/optimizer/prep/prepjointree.c @@ -666,6 +666,7 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte, subroot->glob = root->glob; subroot->query_level = root->query_level; subroot->parent_root = root->parent_root; + subroot->plan_params = NIL; subroot->planner_cxt = CurrentMemoryContext; subroot->init_plans = NIL; subroot->cte_plan_ids = NIL; diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c index f82ab27b9aa..3d9c9768cc3 100644 --- a/src/backend/optimizer/prep/prepunion.c +++ b/src/backend/optimizer/prep/prepunion.c @@ -217,6 +217,9 @@ recurse_set_operations(Node *setOp, PlannerInfo *root, Assert(subquery != NULL); + /* plan_params should not be in use in current query level */ + Assert(root->plan_params == NIL); + /* * Generate plan for primitive subquery */ @@ -226,6 +229,13 @@ recurse_set_operations(Node *setOp, PlannerInfo *root, &subroot); /* + * It should not be possible for the primitive query to contain any + * cross-references to other primitive queries in the setop tree. + */ + if (root->plan_params) + elog(ERROR, "unexpected outer reference in set operation subquery"); + + /* * Estimate number of groups if caller wants it. If the subquery used * grouping or aggregation, its output is probably mostly unique * anyway; otherwise do statistical estimation. diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h index b24ad6ad147..e01887d859b 100644 --- a/src/include/nodes/relation.h +++ b/src/include/nodes/relation.h @@ -76,7 +76,7 @@ typedef struct PlannerGlobal ParamListInfo boundParams; /* Param values provided to planner() */ - List *paramlist; /* to keep track of cross-level Params */ + List *paramlist; /* unused, will be removed in 9.3 */ List *subplans; /* Plans for SubPlan nodes */ @@ -101,6 +101,9 @@ typedef struct PlannerGlobal Index lastRowMarkId; /* highest PlanRowMark ID assigned */ bool transientPlan; /* redo plan when TransactionXmin changes? */ + + /* Added post-release, will be in a saner place in 9.3: */ + int nParamExec; /* number of PARAM_EXEC Params used */ } PlannerGlobal; /* macro for fetching the Plan associated with a SubPlan node */ @@ -234,6 +237,9 @@ typedef struct PlannerInfo /* optional private data for join_search_hook, e.g., GEQO */ void *join_search_private; + + /* Added post-release, will be in a saner place in 9.3: */ + List *plan_params; /* list of PlannerParamItems, see below */ } PlannerInfo; @@ -1427,23 +1433,26 @@ typedef struct MinMaxAggInfo } MinMaxAggInfo; /* - * glob->paramlist keeps track of the PARAM_EXEC slots that we have decided - * we need for the query. At runtime these slots are used to pass values - * around from one plan node to another. They can be used to pass values - * down into subqueries (for outer references in subqueries), or up out of - * subqueries (for the results of a subplan), or from a NestLoop plan node - * into its inner relation (when the inner scan is parameterized with values - * from the outer relation). The n'th entry in the list (n counts from 0) - * corresponds to Param->paramid = n. - * - * Each paramlist item shows the absolute query level it is associated with, - * where the outermost query is level 1 and nested subqueries have higher - * numbers. The item the parameter slot represents can be one of four kinds: - * - * A Var: the slot represents a variable of that level that must be passed + * At runtime, PARAM_EXEC slots are used to pass values around from one plan + * node to another. They can be used to pass values down into subqueries (for + * outer references in subqueries), or up out of subqueries (for the results + * of a subplan), or from a NestLoop plan node into its inner relation (when + * the inner scan is parameterized with values from the outer relation). + * The planner is responsible for assigning nonconflicting PARAM_EXEC IDs to + * the PARAM_EXEC Params it generates. + * + * Outer references are managed via root->plan_params, which is a list of + * PlannerParamItems. While planning a subquery, each parent query level's + * plan_params contains the values required from it by the current subquery. + * During create_plan(), we use plan_params to track values that must be + * passed from outer to inner sides of NestLoop plan nodes. + * + * The item a PlannerParamItem represents can be one of three kinds: + * + * A Var: the slot represents a variable of this level that must be passed * down because subqueries have outer references to it, or must be passed - * from a NestLoop node of that level to its inner scan. The varlevelsup - * value in the Var will always be zero. + * from a NestLoop node to its inner scan. The varlevelsup value in the Var + * will always be zero. * * A PlaceHolderVar: this works much like the Var case, except that the * entry is a PlaceHolderVar node with a contained expression. The PHV @@ -1455,25 +1464,27 @@ typedef struct MinMaxAggInfo * subquery. The Aggref itself has agglevelsup = 0, and its argument tree * is adjusted to match in level. * - * A Param: the slot holds the result of a subplan (it is a setParam item - * for that subplan). The absolute level shown for such items corresponds - * to the parent query of the subplan. - * * Note: we detect duplicate Var and PlaceHolderVar parameters and coalesce - * them into one slot, but we do not bother to do this for Aggrefs, and it - * would be incorrect to do so for Param slots. Duplicate detection is - * actually *necessary* for NestLoop parameters since it serves to match up - * the usage of a Param (in the inner scan) with the assignment of the value - * (in the NestLoop node). This might result in the same PARAM_EXEC slot being - * used by multiple NestLoop nodes or SubPlan nodes, but no harm is done since - * the same value would be assigned anyway. + * them into one slot, but we do not bother to do that for Aggrefs. + * The scope of duplicate-elimination only extends across the set of + * parameters passed from one query level into a single subquery, or for + * nestloop parameters across the set of nestloop parameters used in a single + * query level. So there is no possibility of a PARAM_EXEC slot being used + * for conflicting purposes. + * + * In addition, PARAM_EXEC slots are assigned for Params representing outputs + * from subplans (values that are setParam items for those subplans). These + * IDs need not be tracked via PlannerParamItems, since we do not need any + * duplicate-elimination nor later processing of the represented expressions. + * Instead, we just record the assignment of the slot number by incrementing + * root->glob->nParamExec. */ typedef struct PlannerParamItem { NodeTag type; - Node *item; /* the Var, PlaceHolderVar, Aggref, or Param */ - Index abslevel; /* its absolute query level */ + Node *item; /* the Var, PlaceHolderVar, or Aggref */ + int paramId; /* its assigned PARAM_EXEC slot number */ } PlannerParamItem; #endif /* RELATION_H */ diff --git a/src/test/regress/expected/with.out b/src/test/regress/expected/with.out index 5e2906f4373..fe0e6020658 100644 --- a/src/test/regress/expected/with.out +++ b/src/test/regress/expected/with.out @@ -1211,6 +1211,27 @@ ERROR: recursive reference to query "outermost" must not appear within a subque LINE 2: WITH innermost as (SELECT 2 FROM outermost) ^ -- +-- This test will fail with the old implementation of PARAM_EXEC parameter +-- assignment, because the "q1" Var passed down to A's targetlist subselect +-- looks exactly like the "A.id" Var passed down to C's subselect, causing +-- the old code to give them the same runtime PARAM_EXEC slot. But the +-- lifespans of the two parameters overlap, thanks to B also reading A. +-- +with +A as ( select q2 as id, (select q1) as x from int8_tbl ), +B as ( select id, row_number() over (partition by id) as r from A ), +C as ( select A.id, array(select B.id from B where B.id = A.id) from A ) +select * from C; + id | ?column? +-------------------+------------------------------------- + 456 | {456} + 4567890123456789 | {4567890123456789,4567890123456789} + 123 | {123} + 4567890123456789 | {4567890123456789,4567890123456789} + -4567890123456789 | {-4567890123456789} +(5 rows) + +-- -- Test CTEs read in non-initialization orders -- WITH RECURSIVE diff --git a/src/test/regress/sql/with.sql b/src/test/regress/sql/with.sql index 684c8f13db1..609f51b0c95 100644 --- a/src/test/regress/sql/with.sql +++ b/src/test/regress/sql/with.sql @@ -575,6 +575,20 @@ WITH RECURSIVE outermost(x) AS ( SELECT * FROM outermost; -- +-- This test will fail with the old implementation of PARAM_EXEC parameter +-- assignment, because the "q1" Var passed down to A's targetlist subselect +-- looks exactly like the "A.id" Var passed down to C's subselect, causing +-- the old code to give them the same runtime PARAM_EXEC slot. But the +-- lifespans of the two parameters overlap, thanks to B also reading A. +-- + +with +A as ( select q2 as id, (select q1) as x from int8_tbl ), +B as ( select id, row_number() over (partition by id) as r from A ), +C as ( select A.id, array(select B.id from B where B.id = A.id) from A ) +select * from C; + +-- -- Test CTEs read in non-initialization orders -- |