]> granicus.if.org Git - postgresql/commitdiff
Fix PARAM_EXEC assignment mechanism to be safe in the presence of WITH.
authorTom Lane <tgl@sss.pgh.pa.us>
Sat, 8 Sep 2012 00:38:50 +0000 (20:38 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Sat, 8 Sep 2012 00:38:50 +0000 (20:38 -0400)
The planner previously assumed that parameter Vars having the same absolute
query level, varno, and varattno could safely be assigned the same runtime
PARAM_EXEC slot, even though they might be different Vars appearing in
different subqueries.  This was (probably) safe before the introduction of
CTEs, but the lazy-evalution mechanism used for CTEs means that a CTE can
be executed during execution of some other subquery, causing the lifespan
of Params at the same syntactic nesting level as the CTE to overlap with
use of the same slots inside the CTE.  In 9.1 we created additional hazards
by using the same parameter-assignment technology for nestloop inner scan
parameters, but it was broken before that, as illustrated by the added
regression test.

To fix, restructure the planner's management of PlannerParamItems so that
items having different semantic lifespans are kept rigorously separated.
This will probably result in complex queries using more runtime PARAM_EXEC
slots than before, but the slots are cheap enough that this hardly matters.
Also, stop generating PlannerParamItems containing Params for subquery
outputs: all we really need to do is reserve the PARAM_EXEC slot number,
and that now only takes incrementing a counter.  The planning code is
simpler and probably faster than before, as well as being more correct.

Per report from Vik Reykja.

Back-patch of commit 46c508fbcf98ac334f1e831d21021d731c882fbb into all
branches that support WITH.

src/backend/nodes/outfuncs.c
src/backend/optimizer/path/allpaths.c
src/backend/optimizer/plan/planner.c
src/backend/optimizer/plan/subselect.c
src/backend/optimizer/prep/prepjointree.c
src/backend/optimizer/prep/prepunion.c
src/include/nodes/relation.h
src/test/regress/expected/with.out
src/test/regress/sql/with.sql

index 15f0c94f1b459590704b1c525eb1cb001bb31072..2fe05fe26a56adb86a48ac0a93260c522b812d9a 100644 (file)
@@ -1459,13 +1459,13 @@ _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_BITMAPSET_FIELD(rewindPlanIDs);
        WRITE_NODE_FIELD(finalrtable);
        WRITE_NODE_FIELD(relationOids);
        WRITE_NODE_FIELD(invalItems);
+       WRITE_INT_FIELD(nParamExec);
        WRITE_UINT_FIELD(lastPHId);
        WRITE_BOOL_FIELD(transientPlan);
 }
@@ -1479,6 +1479,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_NODE_FIELD(resultRelations);
        WRITE_NODE_FIELD(returningLists);
@@ -1699,7 +1700,7 @@ _outPlannerParamItem(StringInfo str, PlannerParamItem *node)
        WRITE_NODE_TYPE("PLANNERPARAMITEM");
 
        WRITE_NODE_FIELD(item);
-       WRITE_UINT_FIELD(abslevel);
+       WRITE_INT_FIELD(paramId);
 }
 
 /*****************************************************************************
index 7704091db2309828ce3c68f95664dbdf11217251..7ca9becc1195fb306b4c17c24c3b55382b94b946 100644 (file)
@@ -626,6 +626,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,
@@ -633,6 +636,13 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
                                                                        &subroot);
        rel->subrtable = subroot->parse->rtable;
 
+       /*
+        * 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");
+
        /* Copy number of output rows from subplan */
        rel->tuples = rel->subplan->plan_rows;
 
index 50200805a300b69d9f370c70d88530f65dc5552e..d0331ee8d132b2c2cabb3696c312814f75ca01ba 100644 (file)
@@ -148,13 +148,13 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
        glob = makeNode(PlannerGlobal);
 
        glob->boundParams = boundParams;
-       glob->paramlist = NIL;
        glob->subplans = NIL;
        glob->subrtables = NIL;
        glob->rewindPlanIDs = NULL;
        glob->finalrtable = NIL;
        glob->relationOids = NIL;
        glob->invalItems = NIL;
+       glob->nParamExec = 0;
        glob->lastPHId = 0;
        glob->transientPlan = false;
 
@@ -230,7 +230,7 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
        result->rowMarks = parse->rowMarks;
        result->relationOids = glob->relationOids;
        result->invalItems = glob->invalItems;
-       result->nParamExec = list_length(glob->paramlist);
+       result->nParamExec = glob->nParamExec;
 
        return result;
 }
@@ -282,6 +282,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;
index e8ffb9009c5558bd2bd314a20b89e69e545a1ea4..896d3865ab7fb38d80ff1203d44e1a4469116119 100644 (file)
@@ -53,6 +53,7 @@ typedef struct finalize_primnode_context
 
 
 static Node *build_subplan(PlannerInfo *root, Plan *plan, List *rtable,
+                         List *plan_params,
                          SubLinkType subLinkType, Node *testexpr,
                          bool adjust_testexpr, bool unknownEqFalse);
 static List *generate_subquery_params(PlannerInfo *root, List *tlist,
@@ -89,36 +90,34 @@ replace_outer_var(PlannerInfo *root, Var *var)
        Param      *retval;
        ListCell   *ppl;
        PlannerParamItem *pitem;
-       Index           abslevel;
-       int                     i;
+       Index           levelsup;
 
        Assert(var->varlevelsup > 0 && var->varlevelsup < root->query_level);
-       abslevel = root->query_level - var->varlevelsup;
 
-       /*
-        * If there's already a paramlist entry for this same Var, just use it.
-        * 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 = 0;
-       foreach(ppl, root->glob->paramlist)
+       /* 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 (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)
+                               pvar->vartypmod == var->vartypmod &&
+                               pvar->varnoold == var->varnoold &&
+                               pvar->varoattno == var->varoattno)
                                break;
                }
-               i++;
        }
 
        if (!ppl)
@@ -129,18 +128,17 @@ replace_outer_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);
-               /* i is already the correct index for the new item */
+               root->plan_params = lappend(root->plan_params, pitem);
        }
 
        retval = makeNode(Param);
        retval->paramkind = PARAM_EXEC;
-       retval->paramid = i;
+       retval->paramid = pitem->paramId;
        retval->paramtype = var->vartype;
        retval->paramtypmod = var->vartypmod;
-       retval->location = -1;
+       retval->location = var->location;
 
        return retval;
 }
@@ -157,18 +155,19 @@ replace_outer_placeholdervar(PlannerInfo *root, PlaceHolderVar *phv)
        Param      *retval;
        ListCell   *ppl;
        PlannerParamItem *pitem;
-       Index           abslevel;
-       int                     i;
+       Index           levelsup;
 
        Assert(phv->phlevelsup > 0 && phv->phlevelsup < root->query_level);
-       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)
+       /* 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 (pitem->abslevel == abslevel && IsA(pitem->item, PlaceHolderVar))
+               if (IsA(pitem->item, PlaceHolderVar))
                {
                        PlaceHolderVar *pphv = (PlaceHolderVar *) pitem->item;
 
@@ -176,7 +175,6 @@ replace_outer_placeholdervar(PlannerInfo *root, PlaceHolderVar *phv)
                        if (pphv->phid == phv->phid)
                                break;
                }
-               i++;
        }
 
        if (!ppl)
@@ -188,15 +186,14 @@ replace_outer_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);
-               /* i is already the correct index for the new item */
+               root->plan_params = lappend(root->plan_params, pitem);
        }
 
        retval = makeNode(Param);
        retval->paramkind = PARAM_EXEC;
-       retval->paramid = i;
+       retval->paramid = pitem->paramId;
        retval->paramtype = exprType((Node *) phv->phexpr);
        retval->paramtypmod = exprTypmod((Node *) phv->phexpr);
        retval->location = -1;
@@ -213,11 +210,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
@@ -229,17 +228,16 @@ 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->location = -1;
+       retval->location = agg->location;
 
        return retval;
 }
@@ -247,42 +245,34 @@ 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)
 {
        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->location = -1;
 
-       pitem = makeNode(PlannerParamItem);
-       pitem->item = (Node *) retval;
-       pitem->abslevel = root->query_level;
-
-       root->glob->paramlist = lappend(root->glob->paramlist, pitem);
-
        return retval;
 }
 
 /*
  * Assign a (nonnegative) PARAM_EXEC ID for a recursive query's worktable.
+ * (This is now also used for CTE output params, but I'm not changing the
+ * function name in 8.4.)
  */
 int
 SS_assign_worktable_param(PlannerInfo *root)
 {
-       Param      *param;
-
-       /* We generate a Param of datatype INTERNAL */
-       param = generate_new_param(root, INTERNALOID, -1);
-       /* ... but the caller only cares about its ID */
-       return param->paramid;
+       return root->glob->nParamExec++;
 }
 
 /*
@@ -339,6 +329,7 @@ make_subplan(PlannerInfo *root, Query *orig_subquery, SubLinkType subLinkType,
        double          tuple_fraction;
        Plan       *plan;
        PlannerInfo *subroot;
+       List       *plan_params;
        Node       *result;
 
        /*
@@ -382,6 +373,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.
         */
@@ -390,8 +384,13 @@ 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,
+                                                  plan_params,
                                                   subLinkType, testexpr, true, isTopQual);
 
        /*
@@ -424,6 +423,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))
                        {
@@ -433,6 +436,7 @@ make_subplan(PlannerInfo *root, Query *orig_subquery, SubLinkType subLinkType,
                                /* OK, convert to SubPlan format. */
                                hashplan = (SubPlan *) build_subplan(root, plan,
                                                                                                         subroot->parse->rtable,
+                                                                                                        plan_params,
                                                                                                         ANY_SUBLINK, newtestexpr,
                                                                                                         false, true);
                                /* Check we got what we expected */
@@ -461,14 +465,14 @@ make_subplan(PlannerInfo *root, Query *orig_subquery, SubLinkType subLinkType,
  */
 static Node *
 build_subplan(PlannerInfo *root, Plan *plan, List *rtable,
+                         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
@@ -489,36 +493,26 @@ build_subplan(PlannerInfo *root, Plan *plan, List *rtable,
         * 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);
+               PlannerParamItem *pitem = (PlannerParamItem *) lfirst(lc);
+               Node       *arg = pitem->item;
 
-               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);
+               /*
+                * 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
@@ -938,9 +932,7 @@ SS_process_ctes(PlannerInfo *root)
                Plan       *plan;
                PlannerInfo *subroot;
                SubPlan    *splan;
-               Bitmapset  *tmpset;
                int                     paramid;
-               Param      *prm;
 
                /*
                 * Ignore CTEs that are not actually referenced anywhere.
@@ -958,6 +950,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.
@@ -967,6 +962,14 @@ SS_process_ctes(PlannerInfo *root)
                                                                cte->cterecursive, 0.0,
                                                                &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.
@@ -985,35 +988,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);
-               splan->setParam = list_make1_int(prm->paramid);
+               paramid = SS_assign_worktable_param(root);
+               splan->setParam = list_make1_int(paramid);
 
                /*
                 * Add the subplan and its rtable to the global lists.
@@ -1815,7 +1805,7 @@ SS_finalize_plan(PlannerInfo *root, Plan *plan, bool attach_initplans)
                           *initExtParam,
                           *initSetParam;
        Cost            initplan_cost;
-       int                     paramid;
+       PlannerInfo *proot;
        ListCell   *l;
 
        /*
@@ -1847,29 +1837,35 @@ 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.)
-        *
-        * 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.  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);
        }
        /* Also include the recursion working table, if any */
        if (root->wt_param_id >= 0)
index 4333d1e5ad6990d2985491d349cf57012c8ee916..639f0369dee2427bd98f8ff72a9ca1b9a791a634 100644 (file)
@@ -664,6 +664,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;
index 4ab71b9fe3972511b8d28801575ae71cca61a272..fb72508dff13db9db3bc0691a1e586006c680058 100644 (file)
@@ -211,6 +211,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
                 */
@@ -219,6 +222,13 @@ recurse_set_operations(Node *setOp, PlannerInfo *root,
                                                                   false, tuple_fraction,
                                                                   &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
index c2274bf48ce46623b0a17943ac23ebb950a49401..3726d608dbec059ffc2b54c46a983b314bfe3a2c 100644 (file)
@@ -62,7 +62,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 */
 
@@ -79,6 +79,9 @@ typedef struct PlannerGlobal
        Index           lastPHId;               /* highest PlaceHolderVar 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 */
@@ -197,6 +200,9 @@ typedef struct PlannerInfo
 
        bool            hasInheritedTarget;     /* true if parse->resultRelation is an
                                                                         * inheritance child rel */
+
+       /* Added post-release, will be in a saner place in 9.3: */
+       List       *plan_params;        /* list of PlannerParamItems, see below */
 } PlannerInfo;
 
 
@@ -1310,22 +1316,25 @@ typedef struct PlaceHolderInfo
 } PlaceHolderInfo;
 
 /*
- * 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
- * either down into subqueries (for outer references in subqueries) or up out
- * of subqueries (for the results of a subplan).  The n'th entry in the list
- * (n counts from 0) corresponds to Param->paramid = n.
+ * 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).
+ * The planner is responsible for assigning nonconflicting PARAM_EXEC IDs to
+ * the PARAM_EXEC Params it generates.
  *
- * 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:
+ * 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.
  *
- * A Var: the slot represents a variable of that level that must be passed
+ * 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.  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
+ * entry is a PlaceHolderVar node with a contained expression. The PHV
  * will have phlevelsup = 0, and the contained expression is adjusted
  * to match in level.
  *
@@ -1334,19 +1343,25 @@ typedef struct PlaceHolderInfo
  * 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 do this for Aggref or Param slots.
+ * 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.  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 */
index 65921393a6a3e7b91565ccab2ccf9632d0a9f823..dff442753c2586e50aec8ae3c113a9ed4df2069e 100644 (file)
@@ -1077,6 +1077,27 @@ SELECT * FROM outermost;
 ERROR:  recursive reference to query "outermost" must not appear within a subquery
 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
 --
index 697eaa71f404fb0541eef8476359de16f2cc44a8..c1c39327eabe25cca5d0da44b0649c728fea8c29 100644 (file)
@@ -536,6 +536,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
 --