Skip to content

Commit 05eb923

Browse files
tglsfdctglsfdc
committed
Avoid sharing PARAM_EXEC slots between different levels of NestLoop.
Up to now, createplan.c attempted to share PARAM_EXEC slots for NestLoopParams across different plan levels, if the same underlying Var was being fed down to different righthand-side subplan trees by different NestLoops. This was, I think, more of an artifact of using subselect.c's PlannerParamItem infrastructure than an explicit design goal, but anyway that was the end result. This works well enough as long as the plan tree is executing synchronously, but the feature whereby Gather can execute the parallelized subplan locally breaks it. An upper NestLoop node might execute for a row retrieved from a parallel worker, and assign a value for a PARAM_EXEC slot from that row, while the leader's copy of the parallelized subplan is suspended with a different active value of the row the Var comes from. When control eventually returns to the leader's subplan, it gets the wrong answers if the same PARAM_EXEC slot is being used within the subplan, as reported in bug #15577 from Bartosz Polnik. This is pretty reminiscent of the problem fixed in commit 46c508f, and the proper fix seems to be the same: don't try to share PARAM_EXEC slots across different levels of controlling NestLoop nodes. This requires decoupling NestLoopParam handling from PlannerParamItem handling, although the logic remains somewhat similar. To avoid bizarre division of labor between subselect.c and createplan.c, I decided to move all the param-slot-assignment logic for both cases out of those files and put it into a new file paramassign.c. Hopefully it's a bit better documented now, too. A regression test case for this might be nice, but we don't know a test case that triggers the problem with a suitably small amount of data. Back-patch to 9.6 where we added Gather nodes. It's conceivable that related problems exist in older branches; but without some evidence for that, I'll leave the older branches alone. Discussion: https://postgr.es/m/15577-ca61ab18904af852@postgresql.org
1 parent c559d8d commit 05eb923

File tree

7 files changed

+683
-537
lines changed

7 files changed

+683
-537
lines changed

‎src/backend/optimizer/plan/createplan.c

Lines changed: 10 additions & 172 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
#include <limits.h>
2020
#include <math.h>
2121

22-
#include "access/stratnum.h"
2322
#include "access/sysattr.h"
2423
#include "catalog/pg_class.h"
2524
#include "foreign/fdwapi.h"
@@ -29,11 +28,11 @@
2928
#include "nodes/nodeFuncs.h"
3029
#include "optimizer/clauses.h"
3130
#include "optimizer/cost.h"
31+
#include "optimizer/paramassign.h"
3232
#include "optimizer/paths.h"
3333
#include "optimizer/placeholder.h"
3434
#include "optimizer/plancat.h"
3535
#include "optimizer/planmain.h"
36-
#include "optimizer/planner.h"
3736
#include "optimizer/predtest.h"
3837
#include "optimizer/restrictinfo.h"
3938
#include "optimizer/subselect.h"
@@ -152,8 +151,6 @@ static MergeJoin *create_mergejoin_plan(PlannerInfo *root, MergePath *best_path)
152151
static HashJoin *create_hashjoin_plan(PlannerInfo *root, HashPath *best_path);
153152
static Node *replace_nestloop_params(PlannerInfo *root, Node *expr);
154153
static Node *replace_nestloop_params_mutator(Node *node, PlannerInfo *root);
155-
static void process_subquery_nestloop_params(PlannerInfo *root,
156-
List *subplan_params);
157154
static List *fix_indexqual_references(PlannerInfo *root, IndexPath *index_path);
158155
static List *fix_indexorderby_references(PlannerInfo *root, IndexPath *index_path);
159156
static Node *fix_indexqual_operand(Node *node, IndexOptInfo *index, int indexcol);
@@ -312,7 +309,7 @@ create_plan(PlannerInfo *root, Path *best_path)
312309
/* plan_params should not be in use in current query level */
313310
Assert(root->plan_params == NIL);
314311

315-
/* Initialize this module's private workspace in PlannerInfo */
312+
/* Initialize this module's workspace in PlannerInfo */
316313
root->curOuterRels = NULL;
317314
root->curOuterParams = NIL;
318315

@@ -1531,7 +1528,7 @@ create_gather_plan(PlannerInfo *root, GatherPath *best_path)
15311528
gather_plan = make_gather(tlist,
15321529
NIL,
15331530
best_path->num_workers,
1534-
SS_assign_special_param(root),
1531+
assign_special_exec_param(root),
15351532
best_path->single_copy,
15361533
subplan);
15371534

@@ -1567,7 +1564,7 @@ create_gather_merge_plan(PlannerInfo *root, GatherMergePath *best_path)
15671564
copy_generic_path_info(&gm_plan->plan, &best_path->path);
15681565

15691566
/* Assign the rescan Param. */
1570-
gm_plan->rescan_param = SS_assign_special_param(root);
1567+
gm_plan->rescan_param = assign_special_exec_param(root);
15711568

15721569
/* Gather Merge is pointless with no pathkeys; use Gather instead. */
15731570
Assert(pathkeys != NIL);
@@ -3706,9 +3703,6 @@ create_nestloop_plan(PlannerInfo *root,
37063703
Relids outerrelids;
37073704
List *nestParams;
37083705
Relids saveOuterRels = root->curOuterRels;
3709-
ListCell *cell;
3710-
ListCell *prev;
3711-
ListCell *next;
37123706

37133707
/* NestLoop can project, so no need to be picky about child tlists */
37143708
outer_plan = create_plan_recurse(root, best_path->outerjoinpath, 0);
@@ -3752,38 +3746,10 @@ create_nestloop_plan(PlannerInfo *root,
37523746

37533747
/*
37543748
* Identify any nestloop parameters that should be supplied by this join
3755-
* node, and move them from root->curOuterParams to the nestParams list.
3749+
* node, and remove them from root->curOuterParams.
37563750
*/
37573751
outerrelids = best_path->outerjoinpath->parent->relids;
3758-
nestParams = NIL;
3759-
prev = NULL;
3760-
for (cell = list_head(root->curOuterParams); cell; cell = next)
3761-
{
3762-
NestLoopParam *nlp = (NestLoopParam *) lfirst(cell);
3763-
3764-
next = lnext(cell);
3765-
if (IsA(nlp->paramval, Var) &&
3766-
bms_is_member(nlp->paramval->varno, outerrelids))
3767-
{
3768-
root->curOuterParams = list_delete_cell(root->curOuterParams,
3769-
cell, prev);
3770-
nestParams = lappend(nestParams, nlp);
3771-
}
3772-
else if (IsA(nlp->paramval, PlaceHolderVar) &&
3773-
bms_overlap(((PlaceHolderVar *) nlp->paramval)->phrels,
3774-
outerrelids) &&
3775-
bms_is_subset(find_placeholder_info(root,
3776-
(PlaceHolderVar *) nlp->paramval,
3777-
false)->ph_eval_at,
3778-
outerrelids))
3779-
{
3780-
root->curOuterParams = list_delete_cell(root->curOuterParams,
3781-
cell, prev);
3782-
nestParams = lappend(nestParams, nlp);
3783-
}
3784-
else
3785-
prev = cell;
3786-
}
3752+
nestParams = identify_current_nestloop_params(root, outerrelids);
37873753

37883754
join_plan = make_nestloop(tlist,
37893755
joinclauses,
@@ -4283,42 +4249,18 @@ replace_nestloop_params_mutator(Node *node, PlannerInfo *root)
42834249
if (IsA(node, Var))
42844250
{
42854251
Var *var = (Var *) node;
4286-
Param *param;
4287-
NestLoopParam *nlp;
4288-
ListCell *lc;
42894252

42904253
/* Upper-level Vars should be long gone at this point */
42914254
Assert(var->varlevelsup == 0);
42924255
/* If not to be replaced, we can just return the Var unmodified */
42934256
if (!bms_is_member(var->varno, root->curOuterRels))
42944257
return node;
4295-
/* Create a Param representing the Var */
4296-
param = assign_nestloop_param_var(root, var);
4297-
/* Is this param already listed in root->curOuterParams? */
4298-
foreach(lc, root->curOuterParams)
4299-
{
4300-
nlp = (NestLoopParam *) lfirst(lc);
4301-
if (nlp->paramno == param->paramid)
4302-
{
4303-
Assert(equal(var, nlp->paramval));
4304-
/* Present, so we can just return the Param */
4305-
return (Node *) param;
4306-
}
4307-
}
4308-
/* No, so add it */
4309-
nlp = makeNode(NestLoopParam);
4310-
nlp->paramno = param->paramid;
4311-
nlp->paramval = var;
4312-
root->curOuterParams = lappend(root->curOuterParams, nlp);
4313-
/* And return the replacement Param */
4314-
return (Node *) param;
4258+
/* Replace the Var with a nestloop Param */
4259+
return (Node *) replace_nestloop_param_var(root, var);
43154260
}
43164261
if (IsA(node, PlaceHolderVar))
43174262
{
43184263
PlaceHolderVar *phv = (PlaceHolderVar *) node;
4319-
Param *param;
4320-
NestLoopParam *nlp;
4321-
ListCell *lc;
43224264

43234265
/* Upper-level PlaceHolderVars should be long gone at this point */
43244266
Assert(phv->phlevelsup == 0);
@@ -4355,118 +4297,14 @@ replace_nestloop_params_mutator(Node *node, PlannerInfo *root)
43554297
root);
43564298
return (Node *) newphv;
43574299
}
4358-
/* Create a Param representing the PlaceHolderVar */
4359-
param = assign_nestloop_param_placeholdervar(root, phv);
4360-
/* Is this param already listed in root->curOuterParams? */
4361-
foreach(lc, root->curOuterParams)
4362-
{
4363-
nlp = (NestLoopParam *) lfirst(lc);
4364-
if (nlp->paramno == param->paramid)
4365-
{
4366-
Assert(equal(phv, nlp->paramval));
4367-
/* Present, so we can just return the Param */
4368-
return (Node *) param;
4369-
}
4370-
}
4371-
/* No, so add it */
4372-
nlp = makeNode(NestLoopParam);
4373-
nlp->paramno = param->paramid;
4374-
nlp->paramval = (Var *) phv;
4375-
root->curOuterParams = lappend(root->curOuterParams, nlp);
4376-
/* And return the replacement Param */
4377-
return (Node *) param;
4300+
/* Replace the PlaceHolderVar with a nestloop Param */
4301+
return (Node *) replace_nestloop_param_placeholdervar(root, phv);
43784302
}
43794303
return expression_tree_mutator(node,
43804304
replace_nestloop_params_mutator,
43814305
(void *) root);
43824306
}
43834307

4384-
/*
4385-
* process_subquery_nestloop_params
4386-
* Handle params of a parameterized subquery that need to be fed
4387-
* from an outer nestloop.
4388-
*
4389-
* Currently, that would be *all* params that a subquery in FROM has demanded
4390-
* from the current query level, since they must be LATERAL references.
4391-
*
4392-
* The subplan's references to the outer variables are already represented
4393-
* as PARAM_EXEC Params, so we need not modify the subplan here. What we
4394-
* do need to do is add entries to root->curOuterParams to signal the parent
4395-
* nestloop plan node that it must provide these values.
4396-
*/
4397-
static void
4398-
process_subquery_nestloop_params(PlannerInfo *root, List *subplan_params)
4399-
{
4400-
ListCell *ppl;
4401-
4402-
foreach(ppl, subplan_params)
4403-
{
4404-
PlannerParamItem *pitem = (PlannerParamItem *) lfirst(ppl);
4405-
4406-
if (IsA(pitem->item, Var))
4407-
{
4408-
Var *var = (Var *) pitem->item;
4409-
NestLoopParam *nlp;
4410-
ListCell *lc;
4411-
4412-
/* If not from a nestloop outer rel, complain */
4413-
if (!bms_is_member(var->varno, root->curOuterRels))
4414-
elog(ERROR, "non-LATERAL parameter required by subquery");
4415-
/* Is this param already listed in root->curOuterParams? */
4416-
foreach(lc, root->curOuterParams)
4417-
{
4418-
nlp = (NestLoopParam *) lfirst(lc);
4419-
if (nlp->paramno == pitem->paramId)
4420-
{
4421-
Assert(equal(var, nlp->paramval));
4422-
/* Present, so nothing to do */
4423-
break;
4424-
}
4425-
}
4426-
if (lc == NULL)
4427-
{
4428-
/* No, so add it */
4429-
nlp = makeNode(NestLoopParam);
4430-
nlp->paramno = pitem->paramId;
4431-
nlp->paramval = copyObject(var);
4432-
root->curOuterParams = lappend(root->curOuterParams, nlp);
4433-
}
4434-
}
4435-
else if (IsA(pitem->item, PlaceHolderVar))
4436-
{
4437-
PlaceHolderVar *phv = (PlaceHolderVar *) pitem->item;
4438-
NestLoopParam *nlp;
4439-
ListCell *lc;
4440-
4441-
/* If not from a nestloop outer rel, complain */
4442-
if (!bms_is_subset(find_placeholder_info(root, phv, false)->ph_eval_at,
4443-
root->curOuterRels))
4444-
elog(ERROR, "non-LATERAL parameter required by subquery");
4445-
/* Is this param already listed in root->curOuterParams? */
4446-
foreach(lc, root->curOuterParams)
4447-
{
4448-
nlp = (NestLoopParam *) lfirst(lc);
4449-
if (nlp->paramno == pitem->paramId)
4450-
{
4451-
Assert(equal(phv, nlp->paramval));
4452-
/* Present, so nothing to do */
4453-
break;
4454-
}
4455-
}
4456-
if (lc == NULL)
4457-
{
4458-
/* No, so add it */
4459-
nlp = makeNode(NestLoopParam);
4460-
nlp->paramno = pitem->paramId;
4461-
nlp->paramval = (Var *) copyObject(phv);
4462-
root->curOuterParams = lappend(root->curOuterParams, nlp);
4463-
}
4464-
}
4465-
else
4466-
elog(ERROR, "unexpected type of subquery parameter");
4467-
}
4468-
}
4469-
44704308
/*
44714309
* fix_indexqual_references
44724310
* Adjust indexqual clauses to the form the executor's indexqual

‎src/backend/optimizer/plan/planner.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
#endif
4040
#include "optimizer/clauses.h"
4141
#include "optimizer/cost.h"
42+
#include "optimizer/paramassign.h"
4243
#include "optimizer/pathnode.h"
4344
#include "optimizer/paths.h"
4445
#include "optimizer/plancat.h"
@@ -625,7 +626,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
625626
root->inhTargetKind = INHKIND_NONE;
626627
root->hasRecursion = hasRecursion;
627628
if (hasRecursion)
628-
root->wt_param_id = SS_assign_special_param(root);
629+
root->wt_param_id = assign_special_exec_param(root);
629630
else
630631
root->wt_param_id = -1;
631632
root->non_recursive_path = NULL;
@@ -1640,7 +1641,7 @@ inheritance_planner(PlannerInfo *root)
16401641
returningLists,
16411642
rowMarks,
16421643
NULL,
1643-
SS_assign_special_param(root)));
1644+
assign_special_exec_param(root)));
16441645
}
16451646

16461647
/*--------------------
@@ -2155,7 +2156,7 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
21552156
{
21562157
path = (Path *) create_lockrows_path(root, final_rel, path,
21572158
root->rowMarks,
2158-
SS_assign_special_param(root));
2159+
assign_special_exec_param(root));
21592160
}
21602161

21612162
/*
@@ -2217,7 +2218,7 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
22172218
returningLists,
22182219
rowMarks,
22192220
parse->onConflict,
2220-
SS_assign_special_param(root));
2221+
assign_special_exec_param(root));
22212222
}
22222223

22232224
/* And shove it into final_rel */

0 commit comments

Comments
 (0)