Thread: 9.2rc1 produces incorrect results
Hello. It took me a while to get a version of this that was independent of my data, but here it is. I don't understandwhat's going wrong but if you change any part of this query (or at least any part I tried), the correct resultis returned.<br /><br />This script will reproduce it:<br /><br />=====<br /><br />create table t1 (id integer primarykey);<br />create table t2 (id integer primary key references t1 (id));<br /><br />insert into t1 (id) select generate_series(1,100000); -- size matters<br /> insert into t2 (id) values (1); -- get a known value in the table<br />insertinto t2 (id) select g from generate_series(2, 100000) g where random() < 0.01; -- size matters again<br /><br/>analyze t1;<br />analyze t2;<br /><br /> with<br />A as (<br /> select <a href="http://t2.id">t2.id</a>,<br /> <a href="http://t2.id">t2.id</a> = 1 as is_something<br /> from t2<br /> join t1 on <a href="http://t1.id">t1.id</a>= <a href="http://t2.id">t2.id</a><br /> left join pg_class pg_c on pg_c.relname = t2.id::text-- I haven't tried on a user table<br /> where pg_c.oid is null<br />),<br /><br />B as (<br /> selectA.id,<br /> row_number() over (partition by A.id) as order -- this seems to be important, too<br /> from A<br />)<br /><br />select A.id, array(select B.id from B where B.id = A.id) from A where A.is_something<br />unionall<br />select A.id, array(select B.id from B where B.id = A.id) from A where A.is_something;<br /><br /> =====<br/><br />As you can (hopefully) see, the two UNIONed queries are identical but do not return the same values. I wishI had the skills to attach a patch to this message, but alas I do not.<br />
Vik Reykja <vikreykja@gmail.com> writes:
> Hello. It took me a while to get a version of this that was independent of
> my data, but here it is. I don't understand what's going wrong but if you
> change any part of this query (or at least any part I tried), the correct
> result is returned.
Huh. 9.1 gets the wrong answer too, so this isn't a (very) new bug;
but curiously, 8.4 and 9.0 seem to get it right. I think this is
probably related somehow to Adam Mackler's recent report --- multiple
scans of the same CTE seems to be a bit of a soft spot :-(
regards, tom lane
I wrote:
> Vik Reykja <vikreykja@gmail.com> writes:
>> Hello. It took me a while to get a version of this that was independent of
>> my data, but here it is. I don't understand what's going wrong but if you
>> change any part of this query (or at least any part I tried), the correct
>> result is returned.
> Huh. 9.1 gets the wrong answer too, so this isn't a (very) new bug;
> but curiously, 8.4 and 9.0 seem to get it right. I think this is
> probably related somehow to Adam Mackler's recent report --- multiple
> scans of the same CTE seems to be a bit of a soft spot :-(
No, I'm mistaken: it's a planner bug. The plan looks like this:
QUERY PLAN
-------------------------------------------------------------------------------------------Result (cost=281.29..290.80
rows=20width=36) CTE a -> Nested Loop (cost=126.96..280.17 rows=19 width=4) -> Merge Right Join
(cost=126.96..220.17rows=19 width=4) Merge Cond: (((pg_c.relname)::text) = ((t2.id)::text))
Filter: (pg_c.oid IS NULL) -> Sort (cost=61.86..63.72 rows=743 width=68) Sort
Key:((pg_c.relname)::text) -> Seq Scan on pg_class pg_c (cost=0.00..26.43 rows=743 width=68)
-> Sort (cost=65.10..67.61 rows=1004 width=4) Sort Key: ((t2.id)::text)
-> Seq Scan on t2 (cost=0.00..15.04 rows=1004 width=4) -> Index Scan using t1_pkey on t1
(cost=0.00..3.14rows=1 width=4) Index Cond: (id = t2.id***) CTE b -> WindowAgg (cost=0.78..1.12
rows=19width=4) -> Sort (cost=0.78..0.83 rows=19 width=4) Sort Key: a.id ->
CTEScan on a (cost=0.00..0.38 rows=19 width=4) -> Append (cost=0.00..9.51 rows=20 width=36) -> CTE Scan on
a (cost=0.00..4.66 rows=10 width=4) Filter: is_something SubPlan 3 -> CTE
Scanon b (cost=0.00..0.43 rows=1 width=4) Filter: (id = a.id***) -> CTE Scan on a
(cost=0.00..4.66rows=10 width=4) Filter: is_something SubPlan 4 -> CTE Scan
onb (cost=0.00..0.43 rows=1 width=4) Filter: (id = a.id***)
(30 rows)
The planner is assigning a single PARAM_EXEC slot for the parameter
passed into the inner indexscan in "CTE a" and for the parameters passed
into the two SubPlans that scan "CTE b" (the items I marked with "***"
above). This is safe enough so far as the two SubPlans are concerned,
because they don't execute concurrently --- but when SubPlan 3 is first
fired, it causes the remainder of CTE a to be computed, so that the
nestloop gets iterated some more times, and that overwrites the value of
"a.id" that already got passed down into SubPlan 3.
The reason this is so hard to replicate is that the PARAM_EXEC slot can
only get re-used for identical-looking Vars (same varno, varlevelsup,
vartype, etc) --- so even granted that you've got the right shape of
plan, minor "unrelated" changes in the query can stop the aliasing
from occurring. Also, inner indexscans weren't using the PARAM_EXEC
mechanism until 9.1, so that's why the example doesn't fail before 9.1.
I've always been a bit suspicious of the theory espoused in
replace_outer_var that aliasing different Params is OK:
* NOTE: in sufficiently complex querytrees, it is possible for the same * varno/abslevel to refer to different
RTEsin 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
correctfield should * get stored into the Param slot at execution in each part of the tree.
but I've never seen a provably wrong case before. Most likely, this has
been broken since we introduced CTEs in 8.4: it's the decoupled timing
of execution of main query and CTE that's needed to allow execution of
different parts of the plan tree to overlap and thus create the risk.
(I get the impression that only recently have people been writing really
complex CTE queries, since we found another fundamental bug with them
just recently.)
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.
regards, tom lane
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;
/*
On Wed, Sep 5, 2012 at 6:09 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I am not in a position to know what's best for the project but my company can't upgrade (from 9.0) until this is fixed. We'll wait for 9.2.1 if we have to. After all, we skipped 9.1.
I wrote:Attached is a draft patch against HEAD for this. I think it makes the
> 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.
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?
I am not in a position to know what's best for the project but my company can't upgrade (from 9.0) until this is fixed. We'll wait for 9.2.1 if we have to. After all, we skipped 9.1.
On 5 September 2012 05:09, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 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?
Just so someone else has tested the case in question, here's the
result this end:
id | array
----+------- 1 | {1} 1 | {1}
(2 rows)
QUERY PLAN
-------------------------------------------------------------------------------------------Result (cost=131.45..133.07
rows=8width=36) CTE a -> Nested Loop (cost=87.18..131.09 rows=7 width=4) -> Merge Right Join
(cost=87.18..123.33rows=7 width=4) Merge Cond: (((pg_c.relname)::text) = ((t2.id)::text))
Filter:(pg_c.oid IS NULL) -> Sort (cost=22.82..23.55 rows=291 width=68) Sort Key:
((pg_c.relname)::text) -> Seq Scan on pg_class pg_c
(cost=0.00..10.91 rows=291 width=68) -> Sort (cost=64.36..66.84 rows=993 width=4)
SortKey: ((t2.id)::text) -> Seq Scan on t2 (cost=0.00..14.93 rows=993 width=4) ->
IndexOnly Scan using t1_pkey on t1 (cost=0.00..1.10
rows=1 width=4) Index Cond: (id = t2.id) CTE b -> WindowAgg (cost=0.24..0.36 rows=7 width=4)
-> Sort (cost=0.24..0.26 rows=7 width=4) Sort Key: a.id -> CTE Scan on a
(cost=0.00..0.14rows=7 width=4) -> Append (cost=0.00..1.62 rows=8 width=36) -> CTE Scan on a
(cost=0.00..0.77rows=4 width=4) Filter: is_something SubPlan 3 -> CTE Scan on
b (cost=0.00..0.16 rows=1 width=4) Filter: (id = a.id) -> CTE Scan on a (cost=0.00..0.77
rows=4width=4) Filter: is_something SubPlan 4 -> CTE Scan on b
(cost=0.00..0.16rows=1 width=4) Filter: (id = a.id)
(30 rows)
As for shipping without the fix, I'm torn on whether to do so or not.
I imagine most productions will wait for a .1 or .2 release, and use
.0 for migration testing. Plus this bug hasn't been hit (or at least
not noticed) during 5 releases of 9.1, and there isn't enough time
left before shipping to expose the changes to enough testing in the
areas affected, so I'd be slightly inclined to push this into 9.1.6
and 9.2.1.
Regards
Thom
Thom Brown <thom@linux.com> writes:
> On 5 September 2012 05:09, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> 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?
> As for shipping without the fix, I'm torn on whether to do so or not.
> I imagine most productions will wait for a .1 or .2 release, and use
> .0 for migration testing. Plus this bug hasn't been hit (or at least
> not noticed) during 5 releases of 9.1, and there isn't enough time
> left before shipping to expose the changes to enough testing in the
> areas affected, so I'd be slightly inclined to push this into 9.1.6
> and 9.2.1.
Yeah, after sleeping on it that's my feeling as well. The patch needs
some rework for back branches anyway (since a nontrivial part of it
is touching LATERAL support that doesn't exist before HEAD). I'll
push the fix to HEAD but wait till after 9.2.0 wrap for the back
branches.
regards, tom lane
BTW, after considerable fooling around with Vik's example, I've been
able to produce a regression test case that fails in all PG versions
with WITH:
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;
The correct answer to this is
id | array
-------------------+------------------------------------- 456 | {456} 4567890123456789 |
{4567890123456789,4567890123456789} 123 | {123} 4567890123456789 |
{4567890123456789,4567890123456789}-4567890123456789| {-4567890123456789}
(5 rows)
as you can soon convince yourself by inspecting the contents of
int8_tbl:
q1 | q2
------------------+------------------- 123 | 456 123 |
45678901234567894567890123456789| 1234567890123456789 | 45678901234567894567890123456789 |
-4567890123456789
(5 rows)
I got that answer with patched HEAD, but all the back branches
give me
id | array
-------------------+------------------------------------- 456 | {4567890123456789,4567890123456789}
4567890123456789| {4567890123456789,4567890123456789} 123 | {123} 4567890123456789 |
{4567890123456789,4567890123456789}-4567890123456789| {-4567890123456789}
(5 rows)
So this does indeed need to be back-patched as far as 8.4.
regards, tom lane
I wrote:
> Attached is a draft patch against HEAD for this.
I've finished back-porting this. I'm not going to commit it until 9.2.0
is definitely gold, but attached is the 9.1 version of the patch, if
you'd like to try it and verify that it fixes your original problem.
regards, tom lane