Re: 9.2rc1 produces incorrect results - Mailing list pgsql-hackers
From | Tom Lane |
---|---|
Subject | Re: 9.2rc1 produces incorrect results |
Date | |
Msg-id | 3521.1346818143@sss.pgh.pa.us Whole thread Raw |
In response to | Re: 9.2rc1 produces incorrect results (Tom Lane <tgl@sss.pgh.pa.us>) |
Responses |
Re: 9.2rc1 produces incorrect results
Re: 9.2rc1 produces incorrect results Re: 9.2rc1 produces incorrect results |
List | pgsql-hackers |
I wrote: > I think probably the best fix is to rejigger things so that Params > assigned by different executions of SS_replace_correlation_vars and > createplan.c can't share PARAM_EXEC numbers. This will result in > rather larger ecxt_param_exec_vals arrays at runtime, but the array > entries aren't very large, so I don't think it'll matter. Attached is a draft patch against HEAD for this. I think it makes the planner's handling of outer-level Params far less squishy than it's ever been, but it is rather a large change. Not sure whether to risk pushing it into 9.2 right now, or wait till after we cut 9.2.0 ... thoughts? regards, tom lane diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 1f2bb6cc72f1242f14d55eee7cdc8e0e0d0775a9..02a0f62a53a4e3d06a3ad48d523e959d5d6b2ab7 100644 *** a/src/backend/nodes/outfuncs.c --- b/src/backend/nodes/outfuncs.c *************** _outPlannerGlobal(StringInfo str, const *** 1666,1672 **** WRITE_NODE_TYPE("PLANNERGLOBAL"); /* NB: this isn't a complete set of fields */ - WRITE_NODE_FIELD(paramlist); WRITE_NODE_FIELD(subplans); WRITE_BITMAPSET_FIELD(rewindPlanIDs); WRITE_NODE_FIELD(finalrtable); --- 1666,1671 ---- *************** _outPlannerGlobal(StringInfo str, const *** 1674,1679 **** --- 1673,1679 ---- 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); *************** _outPlannerInfo(StringInfo str, const Pl *** 1688,1693 **** --- 1688,1694 ---- WRITE_NODE_FIELD(parse); WRITE_NODE_FIELD(glob); WRITE_UINT_FIELD(query_level); + WRITE_NODE_FIELD(plan_params); WRITE_BITMAPSET_FIELD(all_baserels); WRITE_NODE_FIELD(join_rel_list); WRITE_INT_FIELD(join_cur_level); *************** _outRelOptInfo(StringInfo str, const Rel *** 1754,1759 **** --- 1755,1761 ---- WRITE_FLOAT_FIELD(allvisfrac, "%.6f"); WRITE_NODE_FIELD(subplan); WRITE_NODE_FIELD(subroot); + WRITE_NODE_FIELD(subplan_params); /* we don't try to print fdwroutine or fdw_private */ WRITE_NODE_FIELD(baserestrictinfo); WRITE_NODE_FIELD(joininfo); *************** _outPlannerParamItem(StringInfo str, con *** 1950,1956 **** WRITE_NODE_TYPE("PLANNERPARAMITEM"); WRITE_NODE_FIELD(item); ! WRITE_UINT_FIELD(abslevel); } /***************************************************************************** --- 1952,1958 ---- WRITE_NODE_TYPE("PLANNERPARAMITEM"); WRITE_NODE_FIELD(item); ! WRITE_INT_FIELD(paramId); } /***************************************************************************** diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index 69a1b93b33746370457bff2daf4d4ece66535803..458dae0489c029bd743c75c82f8e5102067e89bf 100644 *** a/src/backend/optimizer/path/allpaths.c --- b/src/backend/optimizer/path/allpaths.c *************** set_subquery_pathlist(PlannerInfo *root, *** 1145,1150 **** --- 1145,1153 ---- 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, *************** set_subquery_pathlist(PlannerInfo *root, *** 1152,1157 **** --- 1155,1164 ---- &subroot); rel->subroot = subroot; + /* Isolate the params needed by this specific subplan */ + rel->subplan_params = root->plan_params; + root->plan_params = NIL; + /* * It's possible that constraint exclusion proved the subquery empty. If * so, it's convenient to turn it back into a dummy path so that we will diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 5d3b293b88a3ee030adae2260520eda69caad4b7..030f420c90eb37946ee333250de54af61d9b82d7 100644 *** a/src/backend/optimizer/plan/createplan.c --- b/src/backend/optimizer/plan/createplan.c *************** static HashJoin *create_hashjoin_plan(Pl *** 84,90 **** Plan *outer_plan, Plan *inner_plan); static Node *replace_nestloop_params(PlannerInfo *root, Node *expr); static Node *replace_nestloop_params_mutator(Node *node, PlannerInfo *root); ! static void identify_nestloop_extparams(PlannerInfo *root, Plan *subplan); static List *fix_indexqual_references(PlannerInfo *root, IndexPath *index_path); static List *fix_indexorderby_references(PlannerInfo *root, IndexPath *index_path); static Node *fix_indexqual_operand(Node *node, IndexOptInfo *index, int indexcol); --- 84,91 ---- Plan *outer_plan, Plan *inner_plan); static Node *replace_nestloop_params(PlannerInfo *root, Node *expr); static Node *replace_nestloop_params_mutator(Node *node, PlannerInfo *root); ! static void process_subquery_nestloop_params(PlannerInfo *root, ! List *subplan_params); static List *fix_indexqual_references(PlannerInfo *root, IndexPath *index_path); static List *fix_indexorderby_references(PlannerInfo *root, IndexPath *index_path); static Node *fix_indexqual_operand(Node *node, IndexOptInfo *index, int indexcol); *************** create_plan(PlannerInfo *root, Path *bes *** 188,193 **** --- 189,197 ---- { 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; *************** create_plan(PlannerInfo *root, Path *bes *** 199,204 **** --- 203,214 ---- 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; } *************** create_subqueryscan_plan(PlannerInfo *ro *** 1662,1668 **** { scan_clauses = (List *) replace_nestloop_params(root, (Node *) scan_clauses); ! identify_nestloop_extparams(root, best_path->parent->subplan); } scan_plan = make_subqueryscan(tlist, --- 1672,1679 ---- { scan_clauses = (List *) replace_nestloop_params(root, (Node *) scan_clauses); ! process_subquery_nestloop_params(root, ! best_path->parent->subplan_params); } scan_plan = make_subqueryscan(tlist, *************** replace_nestloop_params_mutator(Node *no *** 2620,2649 **** } /* ! * identify_nestloop_extparams ! * Identify extParams of a parameterized subquery that need to be fed * from an outer nestloop. * * The subplan's references to the outer variables are already represented * as PARAM_EXEC Params, so we need not modify the subplan here. What we * do need to do is add entries to root->curOuterParams to signal the parent * nestloop plan node that it must provide these values. */ static void ! identify_nestloop_extparams(PlannerInfo *root, Plan *subplan) { ! Bitmapset *tmpset; ! int paramid; ! /* Examine each extParam of the subquery's plan */ ! tmpset = bms_copy(subplan->extParam); ! while ((paramid = bms_first_member(tmpset)) >= 0) { ! PlannerParamItem *pitem = list_nth(root->glob->paramlist, paramid); ! ! /* Ignore anything coming from an upper query level */ ! if (pitem->abslevel != root->query_level) ! continue; if (IsA(pitem->item, Var)) { --- 2631,2656 ---- } /* ! * process_subquery_nestloop_params ! * Handle params of a parameterized subquery that need to be fed * from an outer nestloop. * + * Currently, that would be *all* params that a subquery in FROM has demanded + * from the current query level, since they must be LATERAL references. + * * The subplan's references to the outer variables are already represented * as PARAM_EXEC Params, so we need not modify the subplan here. What we * do need to do is add entries to root->curOuterParams to signal the parent * nestloop plan node that it must provide these values. */ static void ! process_subquery_nestloop_params(PlannerInfo *root, List *subplan_params) { ! ListCell *ppl; ! foreach(ppl, subplan_params) { ! PlannerParamItem *pitem = (PlannerParamItem *) lfirst(ppl); if (IsA(pitem->item, Var)) { *************** identify_nestloop_extparams(PlannerInfo *** 2651,2664 **** NestLoopParam *nlp; ListCell *lc; ! /* If not from a nestloop outer rel, nothing to do */ if (!bms_is_member(var->varno, root->curOuterRels)) ! continue; /* Is this param already listed in root->curOuterParams? */ foreach(lc, root->curOuterParams) { nlp = (NestLoopParam *) lfirst(lc); ! if (nlp->paramno == paramid) { Assert(equal(var, nlp->paramval)); /* Present, so nothing to do */ --- 2658,2671 ---- NestLoopParam *nlp; ListCell *lc; ! /* If not from a nestloop outer rel, complain */ if (!bms_is_member(var->varno, root->curOuterRels)) ! elog(ERROR, "non-LATERAL parameter required by subquery"); /* Is this param already listed in root->curOuterParams? */ foreach(lc, root->curOuterParams) { nlp = (NestLoopParam *) lfirst(lc); ! if (nlp->paramno == pitem->paramId) { Assert(equal(var, nlp->paramval)); /* Present, so nothing to do */ *************** identify_nestloop_extparams(PlannerInfo *** 2669,2675 **** { /* No, so add it */ nlp = makeNode(NestLoopParam); ! nlp->paramno = paramid; nlp->paramval = copyObject(var); root->curOuterParams = lappend(root->curOuterParams, nlp); } --- 2676,2682 ---- { /* No, so add it */ nlp = makeNode(NestLoopParam); ! nlp->paramno = pitem->paramId; nlp->paramval = copyObject(var); root->curOuterParams = lappend(root->curOuterParams, nlp); } *************** identify_nestloop_extparams(PlannerInfo *** 2680,2701 **** NestLoopParam *nlp; ListCell *lc; ! /* ! * If not from a nestloop outer rel, nothing to do. We use ! * bms_overlap as a cheap/quick test to see if the PHV might be ! * evaluated in the outer rels, and then grab its PlaceHolderInfo ! * to tell for sure. ! */ ! if (!bms_overlap(phv->phrels, root->curOuterRels)) ! continue; if (!bms_is_subset(find_placeholder_info(root, phv, false)->ph_eval_at, root->curOuterRels)) ! continue; /* Is this param already listed in root->curOuterParams? */ foreach(lc, root->curOuterParams) { nlp = (NestLoopParam *) lfirst(lc); ! if (nlp->paramno == paramid) { Assert(equal(phv, nlp->paramval)); /* Present, so nothing to do */ --- 2687,2701 ---- NestLoopParam *nlp; ListCell *lc; ! /* If not from a nestloop outer rel, complain */ if (!bms_is_subset(find_placeholder_info(root, phv, false)->ph_eval_at, root->curOuterRels)) ! elog(ERROR, "non-LATERAL parameter required by subquery"); /* Is this param already listed in root->curOuterParams? */ foreach(lc, root->curOuterParams) { nlp = (NestLoopParam *) lfirst(lc); ! if (nlp->paramno == pitem->paramId) { Assert(equal(phv, nlp->paramval)); /* Present, so nothing to do */ *************** identify_nestloop_extparams(PlannerInfo *** 2706,2718 **** { /* No, so add it */ nlp = makeNode(NestLoopParam); ! nlp->paramno = paramid; nlp->paramval = copyObject(phv); root->curOuterParams = lappend(root->curOuterParams, nlp); } } } - bms_free(tmpset); } /* --- 2706,2719 ---- { /* No, so add it */ nlp = makeNode(NestLoopParam); ! nlp->paramno = pitem->paramId; nlp->paramval = copyObject(phv); root->curOuterParams = lappend(root->curOuterParams, nlp); } } + else + elog(ERROR, "unexpected type of subquery parameter"); } } /* diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 4284eed47b929f7c9cca8c75bcde3487fc13c65f..385e64647ea0dea00c15889064421a01ad2dab79 100644 *** a/src/backend/optimizer/plan/planner.c --- b/src/backend/optimizer/plan/planner.c *************** standard_planner(Query *parse, int curso *** 155,161 **** glob = makeNode(PlannerGlobal); glob->boundParams = boundParams; - glob->paramlist = NIL; glob->subplans = NIL; glob->subroots = NIL; glob->rewindPlanIDs = NULL; --- 155,160 ---- *************** standard_planner(Query *parse, int curso *** 164,169 **** --- 163,169 ---- glob->resultRelations = NIL; glob->relationOids = NIL; glob->invalItems = NIL; + glob->nParamExec = 0; glob->lastPHId = 0; glob->lastRowMarkId = 0; glob->transientPlan = false; *************** standard_planner(Query *parse, int curso *** 243,249 **** result->rowMarks = glob->finalrowmarks; result->relationOids = glob->relationOids; result->invalItems = glob->invalItems; ! result->nParamExec = list_length(glob->paramlist); return result; } --- 243,249 ---- result->rowMarks = glob->finalrowmarks; result->relationOids = glob->relationOids; result->invalItems = glob->invalItems; ! result->nParamExec = glob->nParamExec; return result; } *************** subquery_planner(PlannerGlobal *glob, Qu *** 295,300 **** --- 295,301 ---- 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; *************** subquery_planner(PlannerGlobal *glob, Qu *** 586,592 **** * and attach the initPlans to the top plan node. */ if (list_length(glob->subplans) != num_old_subplans || ! root->glob->paramlist != NIL) SS_finalize_plan(root, plan, true); /* Return internal info if caller wants it */ --- 587,593 ---- * and attach the initPlans to the top plan node. */ if (list_length(glob->subplans) != num_old_subplans || ! 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 4c2e82128181b8bbbfb4c527b0db8eed1714b6df..305888214a57d699a348a5db6acf28a6c6befe46 100644 *** a/src/backend/optimizer/plan/subselect.c --- b/src/backend/optimizer/plan/subselect.c *************** typedef struct finalize_primnode_context *** 54,59 **** --- 54,60 ---- static Node *build_subplan(PlannerInfo *root, Plan *plan, PlannerInfo *subroot, + List *plan_params, SubLinkType subLinkType, Node *testexpr, bool adjust_testexpr, bool unknownEqFalse); static List *generate_subquery_params(PlannerInfo *root, List *tlist, *************** static bool finalize_primnode(Node *node *** 82,116 **** /* ! * Select a PARAM_EXEC number to identify the given Var. ! * If the Var already has a param slot, return that one. */ static int assign_param_for_var(PlannerInfo *root, Var *var) { ListCell *ppl; PlannerParamItem *pitem; ! Index abslevel; ! int i; ! abslevel = root->query_level - var->varlevelsup; ! /* If there's already a paramlist entry for this same Var, just use it */ ! i = 0; ! foreach(ppl, root->glob->paramlist) { pitem = (PlannerParamItem *) lfirst(ppl); ! if (pitem->abslevel == abslevel && IsA(pitem->item, Var)) { Var *pvar = (Var *) pitem->item; if (pvar->varno == var->varno && pvar->varattno == var->varattno && pvar->vartype == var->vartype && ! pvar->vartypmod == var->vartypmod) ! return i; } - i++; } /* Nope, so make a new one */ --- 83,124 ---- /* ! * 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 levelsup; ! /* Find the query level the Var belongs to */ ! for (levelsup = var->varlevelsup; levelsup > 0; levelsup--) ! root = root->parent_root; ! /* If there's already a matching PlannerParamItem there, just use it */ ! foreach(ppl, root->plan_params) { pitem = (PlannerParamItem *) lfirst(ppl); ! 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 && ! pvar->varcollid == var->varcollid && ! pvar->varnoold == var->varnoold && ! pvar->varoattno == var->varoattno) ! return pitem->paramId; } } /* Nope, so make a new one */ *************** assign_param_for_var(PlannerInfo *root, *** 119,130 **** pitem = makeNode(PlannerParamItem); pitem->item = (Node *) var; ! pitem->abslevel = abslevel; ! root->glob->paramlist = lappend(root->glob->paramlist, pitem); ! /* i is already the correct list index for the new item */ ! return i; } /* --- 127,137 ---- pitem = makeNode(PlannerParamItem); pitem->item = (Node *) var; ! pitem->paramId = root->glob->nParamExec++; ! root->plan_params = lappend(root->plan_params, pitem); ! return pitem->paramId; } /* *************** replace_outer_var(PlannerInfo *root, Var *** 139,154 **** 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. ! */ i = assign_param_for_var(root, var); retval = makeNode(Param); --- 146,152 ---- Assert(var->varlevelsup > 0 && var->varlevelsup < root->query_level); ! /* Find the Var in the appropriate plan_params, or add it if not present */ i = assign_param_for_var(root, var); retval = makeNode(Param); *************** replace_outer_var(PlannerInfo *root, Var *** 157,163 **** retval->paramtype = var->vartype; retval->paramtypmod = var->vartypmod; retval->paramcollid = var->varcollid; ! retval->location = -1; return retval; } --- 155,161 ---- retval->paramtype = var->vartype; retval->paramtypmod = var->vartypmod; retval->paramcollid = var->varcollid; ! retval->location = var->location; return retval; } *************** replace_outer_var(PlannerInfo *root, Var *** 166,173 **** * 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 * the Var to be local to the current query level. */ Param * --- 164,170 ---- * Generate a Param node to replace the given Var, which will be supplied * from an upper NestLoop join node. * ! * This is effectively the same as replace_outer_var, except that we expect * the Var to be local to the current query level. */ Param * *************** assign_nestloop_param_var(PlannerInfo *r *** 186,199 **** retval->paramtype = var->vartype; retval->paramtypmod = var->vartypmod; retval->paramcollid = var->varcollid; ! retval->location = -1; return retval; } /* ! * Select a PARAM_EXEC number to identify the given PlaceHolderVar. ! * If the PlaceHolderVar already has a param slot, return that one. * * This is just like assign_param_for_var, except for PlaceHolderVars. */ --- 183,197 ---- retval->paramtype = var->vartype; retval->paramtypmod = var->vartypmod; retval->paramcollid = var->varcollid; ! retval->location = var->location; return retval; } /* ! * 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. */ *************** assign_param_for_placeholdervar(PlannerI *** 202,226 **** { ListCell *ppl; PlannerParamItem *pitem; ! Index abslevel; ! int i; ! abslevel = root->query_level - phv->phlevelsup; ! /* If there's already a paramlist entry for this same PHV, just use it */ ! i = 0; ! foreach(ppl, root->glob->paramlist) { pitem = (PlannerParamItem *) lfirst(ppl); ! if (pitem->abslevel == abslevel && IsA(pitem->item, PlaceHolderVar)) { PlaceHolderVar *pphv = (PlaceHolderVar *) pitem->item; /* We assume comparing the PHIDs is sufficient */ if (pphv->phid == phv->phid) ! return i; } - i++; } /* Nope, so make a new one */ --- 200,223 ---- { ListCell *ppl; PlannerParamItem *pitem; ! Index levelsup; ! /* Find the query level the PHV belongs to */ ! for (levelsup = phv->phlevelsup; levelsup > 0; levelsup--) ! root = root->parent_root; ! /* If there's already a matching PlannerParamItem there, just use it */ ! foreach(ppl, root->plan_params) { pitem = (PlannerParamItem *) lfirst(ppl); ! if (IsA(pitem->item, PlaceHolderVar)) { PlaceHolderVar *pphv = (PlaceHolderVar *) pitem->item; /* We assume comparing the PHIDs is sufficient */ if (pphv->phid == phv->phid) ! return pitem->paramId; } } /* Nope, so make a new one */ *************** assign_param_for_placeholdervar(PlannerI *** 233,244 **** pitem = makeNode(PlannerParamItem); pitem->item = (Node *) phv; ! pitem->abslevel = abslevel; ! root->glob->paramlist = lappend(root->glob->paramlist, pitem); ! /* i is already the correct list index for the new item */ ! return i; } /* --- 230,240 ---- pitem = makeNode(PlannerParamItem); pitem->item = (Node *) phv; ! pitem->paramId = root->glob->nParamExec++; ! root->plan_params = lappend(root->plan_params, pitem); ! return pitem->paramId; } /* *************** replace_outer_placeholdervar(PlannerInfo *** 255,264 **** Assert(phv->phlevelsup > 0 && phv->phlevelsup < root->query_level); ! /* ! * Find the PlaceHolderVar in root->glob->paramlist, or add it if not ! * present. ! */ i = assign_param_for_placeholdervar(root, phv); retval = makeNode(Param); --- 251,257 ---- Assert(phv->phlevelsup > 0 && phv->phlevelsup < root->query_level); ! /* Find the PHV in the appropriate plan_params, or add it if not present */ i = assign_param_for_placeholdervar(root, phv); retval = makeNode(Param); *************** replace_outer_agg(PlannerInfo *root, Agg *** 308,318 **** { Param *retval; PlannerParamItem *pitem; ! Index abslevel; ! int i; Assert(agg->agglevelsup > 0 && agg->agglevelsup < root->query_level); ! abslevel = root->query_level - agg->agglevelsup; /* * It does not seem worthwhile to try to match duplicate outer aggs. Just --- 301,313 ---- { Param *retval; PlannerParamItem *pitem; ! Index levelsup; Assert(agg->agglevelsup > 0 && agg->agglevelsup < root->query_level); ! ! /* 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 *************** replace_outer_agg(PlannerInfo *root, Agg *** 324,341 **** pitem = makeNode(PlannerParamItem); pitem->item = (Node *) agg; ! pitem->abslevel = abslevel; ! root->glob->paramlist = lappend(root->glob->paramlist, pitem); ! i = list_length(root->glob->paramlist) - 1; retval = makeNode(Param); retval->paramkind = PARAM_EXEC; ! retval->paramid = i; retval->paramtype = agg->aggtype; retval->paramtypmod = -1; retval->paramcollid = agg->aggcollid; ! retval->location = -1; return retval; } --- 319,335 ---- pitem = makeNode(PlannerParamItem); pitem->item = (Node *) agg; ! pitem->paramId = root->glob->nParamExec++; ! root->plan_params = lappend(root->plan_params, pitem); retval = makeNode(Param); retval->paramkind = PARAM_EXEC; ! retval->paramid = pitem->paramId; retval->paramtype = agg->aggtype; retval->paramtypmod = -1; retval->paramcollid = agg->aggcollid; ! retval->location = agg->location; return retval; } *************** replace_outer_agg(PlannerInfo *root, Agg *** 343,371 **** /* * Generate a new Param node that will not conflict with any other. * ! * This is used to allocate PARAM_EXEC slots for subplan outputs. */ 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->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; } --- 337,360 ---- /* * Generate a new Param node that will not conflict with any other. * ! * 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; retval = makeNode(Param); retval->paramkind = PARAM_EXEC; ! retval->paramid = root->glob->nParamExec++; retval->paramtype = paramtype; retval->paramtypmod = paramtypmod; retval->paramcollid = paramcollation; retval->location = -1; return retval; } *************** generate_new_param(PlannerInfo *root, Oi *** 374,390 **** * 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. */ 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; } /* --- 363,375 ---- * 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. No actual Param node ! * exists with this ID, however. */ int SS_assign_special_param(PlannerInfo *root) { ! return root->glob->nParamExec++; } /* *************** make_subplan(PlannerInfo *root, Query *o *** 445,450 **** --- 430,436 ---- double tuple_fraction; Plan *plan; PlannerInfo *subroot; + List *plan_params; Node *result; /* *************** make_subplan(PlannerInfo *root, Query *o *** 488,493 **** --- 474,482 ---- 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. */ *************** make_subplan(PlannerInfo *root, Query *o *** 496,503 **** false, tuple_fraction, &subroot); /* And convert to SubPlan or InitPlan format. */ ! result = build_subplan(root, plan, subroot, subLinkType, testexpr, true, isTopQual); /* --- 485,496 ---- 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, plan_params, subLinkType, testexpr, true, isTopQual); /* *************** make_subplan(PlannerInfo *root, Query *o *** 530,535 **** --- 523,532 ---- 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)) { *************** make_subplan(PlannerInfo *root, Query *o *** 538,543 **** --- 535,541 ---- /* OK, convert to SubPlan format. */ hashplan = (SubPlan *) build_subplan(root, plan, subroot, + plan_params, ANY_SUBLINK, newtestexpr, false, true); /* Check we got what we expected */ *************** make_subplan(PlannerInfo *root, Query *o *** 566,579 **** */ static Node * build_subplan(PlannerInfo *root, Plan *plan, PlannerInfo *subroot, SubLinkType subLinkType, Node *testexpr, bool adjust_testexpr, bool unknownEqFalse) { Node *result; SubPlan *splan; bool isInitPlan; ! Bitmapset *tmpset; ! int paramid; /* * Initialize the SubPlan node. Note plan_id, plan_name, and cost fields --- 564,577 ---- */ static Node * build_subplan(PlannerInfo *root, Plan *plan, PlannerInfo *subroot, + List *plan_params, SubLinkType subLinkType, Node *testexpr, bool adjust_testexpr, bool unknownEqFalse) { Node *result; SubPlan *splan; bool isInitPlan; ! ListCell *lc; /* * Initialize the SubPlan node. Note plan_id, plan_name, and cost fields *************** build_subplan(PlannerInfo *root, Plan *p *** 595,630 **** * 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) { ! 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); ! /* ! * 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); ! } } - bms_free(tmpset); /* * Un-correlated or undirect correlated plans of EXISTS, EXPR, ARRAY, or --- 593,618 ---- * Make parParam and args lists of param IDs and expressions that current * query level will pass to this child plan. */ ! foreach(lc, plan_params) { ! PlannerParamItem *pitem = (PlannerParamItem *) lfirst(lc); ! Node *arg = pitem->item; ! /* ! * 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, pitem->paramId); ! splan->args = lappend(splan->args, arg); } /* * Un-correlated or undirect correlated plans of EXISTS, EXPR, ARRAY, or *************** SS_process_ctes(PlannerInfo *root) *** 1045,1053 **** Plan *plan; PlannerInfo *subroot; SubPlan *splan; - Bitmapset *tmpset; int paramid; - Param *prm; /* * Ignore SELECT CTEs that are not actually referenced anywhere. --- 1033,1039 ---- *************** SS_process_ctes(PlannerInfo *root) *** 1065,1070 **** --- 1051,1059 ---- */ 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. *************** SS_process_ctes(PlannerInfo *root) *** 1074,1079 **** --- 1063,1072 ---- cte->cterecursive, 0.0, &subroot); + /* There should not be any side-references in the CTE query */ + 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. *************** SS_process_ctes(PlannerInfo *root) *** 1093,1127 **** 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. */ - 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. */ ! prm = generate_new_param(root, INTERNALOID, -1, InvalidOid); ! splan->setParam = list_make1_int(prm->paramid); /* * Add the subplan and its PlannerInfo to the global lists. --- 1086,1107 ---- splan->args = NIL; /* ! * 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.) */ /* ! * 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.) */ ! paramid = SS_assign_special_param(root); ! splan->setParam = list_make1_int(paramid); /* * Add the subplan and its PlannerInfo to the global lists. *************** SS_finalize_plan(PlannerInfo *root, Plan *** 1932,1938 **** *initExtParam, *initSetParam; Cost initplan_cost; ! int paramid; ListCell *l; /* --- 1912,1918 ---- *initExtParam, *initSetParam; Cost initplan_cost; ! PlannerInfo *proot; ListCell *l; /* *************** SS_finalize_plan(PlannerInfo *root, Plan *** 1964,1994 **** /* * 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 * 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) { ! PlannerParamItem *pitem = (PlannerParamItem *) lfirst(l); ! ! if (pitem->abslevel < root->query_level) { ! /* valid outer-level parameter */ ! valid_params = bms_add_member(valid_params, paramid); } ! paramid++; } /* --- 1944,1979 ---- /* * 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 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.) */ valid_params = bms_copy(initSetParam); ! for (proot = root->parent_root; proot != NULL; proot = proot->parent_root) { ! /* Include ordinary Var/PHV/Aggref params */ ! foreach(l, proot->plan_params) { ! 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; ! 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 3d1fcdc16c94fc314c8b5af05399794782a1bbbe..44b5d6758e7ef49ebb3816996f47cd6dbac208a7 100644 *** a/src/backend/optimizer/prep/prepjointree.c --- b/src/backend/optimizer/prep/prepjointree.c *************** pull_up_simple_subquery(PlannerInfo *roo *** 799,804 **** --- 799,805 ---- 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 94373b087c3ec2fe572a51e34321bc773c66ec20..c66ea7bc6f44290c2fc4117127a138b18e52cf23 100644 *** a/src/backend/optimizer/prep/prepunion.c --- b/src/backend/optimizer/prep/prepunion.c *************** recurse_set_operations(Node *setOp, Plan *** 238,243 **** --- 238,246 ---- */ rel = build_simple_rel(root, rtr->rtindex, RELOPT_BASEREL); + /* plan_params should not be in use in current query level */ + Assert(root->plan_params == NIL); + /* * Generate plan for primitive subquery */ *************** recurse_set_operations(Node *setOp, Plan *** 250,255 **** --- 253,262 ---- rel->subplan = subplan; rel->subroot = subroot; + /* There should not be any side-references in the primitive query */ + 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 diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c index 3878c3752fef56e43bbe12d8c1613698993d0d1d..f724714e49fe03d34a837cd8edd157664d3098a4 100644 *** a/src/backend/optimizer/util/relnode.c --- b/src/backend/optimizer/util/relnode.c *************** build_simple_rel(PlannerInfo *root, int *** 119,124 **** --- 119,125 ---- rel->allvisfrac = 0; rel->subplan = NULL; rel->subroot = NULL; + rel->subplan_params = NIL; rel->fdwroutine = NULL; rel->fdw_private = NULL; rel->baserestrictinfo = NIL; *************** build_join_rel(PlannerInfo *root, *** 379,384 **** --- 380,386 ---- joinrel->allvisfrac = 0; joinrel->subplan = NULL; joinrel->subroot = NULL; + joinrel->subplan_params = NIL; joinrel->fdwroutine = NULL; joinrel->fdw_private = NULL; joinrel->baserestrictinfo = NIL; diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h index af9425ad9b309fbecd65d1a83a229ddfd447f382..2b2742d7ef5bbc1ae2ddd3e2bf81f986e80524d9 100644 *** a/src/include/nodes/relation.h --- b/src/include/nodes/relation.h *************** typedef struct PlannerGlobal *** 75,82 **** ParamListInfo boundParams; /* Param values provided to planner() */ - List *paramlist; /* to keep track of cross-level Params */ - List *subplans; /* Plans for SubPlan nodes */ List *subroots; /* PlannerInfos for SubPlan nodes */ --- 75,80 ---- *************** typedef struct PlannerGlobal *** 93,98 **** --- 91,98 ---- List *invalItems; /* other dependencies, as PlanInvalItems */ + int nParamExec; /* number of PARAM_EXEC Params used */ + Index lastPHId; /* highest PlaceHolderVar ID assigned */ Index lastRowMarkId; /* highest PlanRowMark ID assigned */ *************** typedef struct PlannerInfo *** 127,132 **** --- 127,134 ---- struct PlannerInfo *parent_root; /* NULL at outermost Query */ + List *plan_params; /* list of PlannerParamItems, see below */ + /* * simple_rel_array holds pointers to "base rels" and "other rels" (see * comments for RelOptInfo for more info). It is indexed by rangetable *************** typedef struct PlannerInfo *** 344,349 **** --- 346,352 ---- * allvisfrac - fraction of disk pages that are marked all-visible * subplan - plan for subquery (NULL if it's not a subquery) * subroot - PlannerInfo for subquery (NULL if it's not a subquery) + * subplan_params - list of PlannerParamItems to be passed to subquery * fdwroutine - function hooks for FDW, if foreign table (else NULL) * fdw_private - private state for FDW, if foreign table (else NULL) * *************** typedef struct RelOptInfo *** 436,441 **** --- 439,445 ---- /* use "struct Plan" to avoid including plannodes.h here */ struct Plan *subplan; /* if subquery */ PlannerInfo *subroot; /* if subquery */ + List *subplan_params; /* if subquery */ /* use "struct FdwRoutine" to avoid including fdwapi.h here */ struct FdwRoutine *fdwroutine; /* if foreign table */ void *fdw_private; /* if foreign table */ *************** typedef struct MinMaxAggInfo *** 1507,1529 **** } 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 * 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. * * A PlaceHolderVar: this works much like the Var case, except that the * entry is a PlaceHolderVar node with a contained expression. The PHV --- 1511,1536 ---- } MinMaxAggInfo; /* ! * 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 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 *************** typedef struct MinMaxAggInfo *** 1535,1559 **** * 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. */ typedef struct PlannerParamItem { NodeTag type; ! Node *item; /* the Var, PlaceHolderVar, Aggref, or Param */ ! Index abslevel; /* its absolute query level */ } PlannerParamItem; /* --- 1542,1568 ---- * subquery. The Aggref itself has agglevelsup = 0, and its argument tree * is adjusted to match in level. * * Note: we detect duplicate Var and PlaceHolderVar parameters and coalesce ! * 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, or Aggref */ ! int paramId; /* its assigned PARAM_EXEC slot number */ } PlannerParamItem; /*
pgsql-hackers by date: