database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-07-27 12:41:57 +03:00
Code Activity

Fix MULTIEXPR_SUBLINK with partitioned target tables, yet again.

Browse Source 
We already tried to fix this in commits 3f7323cbb et al (and follow-on
fixes), but now it emerges that there are still unfixed cases;
moreover, these cases affect all branches not only pre-v14.  I thought
we had eliminated all cases of making multiple clones of an UPDATE's
target list when we nuked inheritance_planner.  But it turns out we
still do that in some partitioned-UPDATE cases, notably including
INSERT ... ON CONFLICT UPDATE, because ExecInitPartitionInfo thinks
it's okay to clone and modify the parent's targetlist.

This fix is based on a suggestion from Andres Freund: let's stop
abusing the ParamExecData.execPlan mechanism, which was only ever
meant to handle initplans, and instead solve the execution timing
problem by having the expression compiler move MULTIEXPR_SUBLINK steps
to the front of their expression step lists.  This is feasible because
(a) all branches still in support compile the entire targetlist of
an UPDATE into a single ExprState, and (b) we know that all
MULTIEXPR_SUBLINKs do need to be evaluated --- none could be buried
inside a CASE, for example.  There is a minor semantics change
concerning the order of execution of the MULTIEXPR's subquery versus
other parts of the parent targetlist, but that seems like something
we can get away with.  By doing that, we no longer need to worry
about whether different clones of a MULTIEXPR_SUBLINK share output
Params; their usage of that data structure won't overlap.

Per bug #17800 from Alexander Lakhin.  Back-patch to all supported
branches.  In v13 and earlier, we can revert 3f7323cbb and follow-on
fixes; however, I chose to keep the SubPlan.subLinkId field added
in ccbb54c72.  We don't need that anymore in the core code, but it's
cheap enough to fill, and removing a plan node field in a minor
release seems like it'd be asking for trouble.

Andres Freund and Tom Lane

Discussion: https://postgr.es/m/17800-ff90866b3906c964@postgresql.org
This commit is contained in:
Tom Lane
2023-02-25 14:44:14 -05:00
parent 39ad791e85
commit 1e199c2599
8 changed files with 305 additions and 326 deletions
Download Patch File Download Diff File Expand all files Collapse all files

129
src/backend/optimizer/plan/subselect.c
View File

@ -86,7 +86,6 @@ static bool subplan_is_hashable(Plan *plan);
static bool testexpr_is_hashable(Node *testexpr, List *param_ids);
static bool test_opexpr_is_hashable(OpExpr *testexpr, List *param_ids);
static bool hash_ok_operator(OpExpr *expr);
static bool SS_make_multiexprs_unique_walker(Node *node, void *context);
static bool contain_dml(Node *node);
static bool contain_dml_walker(Node *node, void *context);
static bool contain_outer_selfref(Node *node);
@ -853,134 +852,6 @@ hash_ok_operator(OpExpr *expr)
}
}
/*
* SS_make_multiexprs_unique
*
* After cloning an UPDATE targetlist that contains MULTIEXPR_SUBLINK
* SubPlans, inheritance_planner() must call this to assign new, unique Param
* IDs to the cloned MULTIEXPR_SUBLINKs' output parameters. See notes in
* ExecScanSubPlan.
*
* We do not need to renumber Param IDs for MULTIEXPR_SUBLINK plans that are
* initplans, because those don't have input parameters that could cause
* confusion. Such initplans will not appear in the targetlist anyway, but
* they still complicate matters because the surviving regular subplans might
* not have consecutive subLinkIds.
*/
void
SS_make_multiexprs_unique(PlannerInfo *root, PlannerInfo *subroot)
{
List *param_mapping = NIL;
ListCell *lc;
/*
* Find MULTIEXPR SubPlans in the cloned query. We need only look at the
* top level of the targetlist.
*/
foreach(lc, subroot->parse->targetList)
{
TargetEntry *tent = (TargetEntry *) lfirst(lc);
SubPlan *splan;
Plan *plan;
List *params;
int oldId,
newId;
ListCell *lc2;
if (!IsA(tent->expr, SubPlan))
continue;
splan = (SubPlan *) tent->expr;
if (splan->subLinkType != MULTIEXPR_SUBLINK)
continue;
/* Found one, get the associated subplan */
plan = (Plan *) list_nth(root->glob->subplans, splan->plan_id - 1);
/*
* Generate new PARAM_EXEC Param nodes, and overwrite splan->setParam
* with their IDs. This is just like what build_subplan did when it
* made the SubPlan node we're cloning. But because the param IDs are
* assigned globally, we'll get new IDs. (We assume here that the
* subroot's tlist is a clone we can scribble on.)
*/
params = generate_subquery_params(root,
plan->targetlist,
&splan->setParam);
/*
* Append the new replacement-Params list to root->multiexpr_params.
* Then its index in that list becomes the new subLinkId of the
* SubPlan.
*/
root->multiexpr_params = lappend(root->multiexpr_params, params);
oldId = splan->subLinkId;
newId = list_length(root->multiexpr_params);
Assert(newId > oldId);
splan->subLinkId = newId;
/*
* Add a mapping entry to param_mapping so that we can update the
* associated Params below. Leave zeroes in the list for any
* subLinkIds we don't encounter; those must have been converted to
* initplans.
*/
while (list_length(param_mapping) < oldId)
param_mapping = lappend_int(param_mapping, 0);
lc2 = list_nth_cell(param_mapping, oldId - 1);
lfirst_int(lc2) = newId;
}
/*
* Unless all the MULTIEXPRs were converted to initplans, we must now find
* the Param nodes that reference the MULTIEXPR outputs and update their
* sublink IDs so they'll reference the new outputs. While such Params
* must be in the cloned targetlist, they could be buried under stuff such
* as FieldStores and SubscriptingRefs and type coercions.
*/
if (param_mapping != NIL)
SS_make_multiexprs_unique_walker((Node *) subroot->parse->targetList,
(void *) param_mapping);
}
/*
* Locate PARAM_MULTIEXPR Params in an expression tree, and update as needed.
* (We can update-in-place because the tree was already copied.)
*/
static bool
SS_make_multiexprs_unique_walker(Node *node, void *context)
{
if (node == NULL)
return false;
if (IsA(node, Param))
{
Param *p = (Param *) node;
List *param_mapping = (List *) context;
int subqueryid;
int colno;
int newId;
if (p->paramkind != PARAM_MULTIEXPR)
return false;
subqueryid = p->paramid >> 16;
colno = p->paramid & 0xFFFF;
/*
* If subqueryid doesn't have a mapping entry, it must refer to an
* initplan, so don't change the Param.
*/
Assert(subqueryid > 0);
if (subqueryid > list_length(param_mapping))
return false;
newId = list_nth_int(param_mapping, subqueryid - 1);
if (newId == 0)
return false;
p->paramid = (newId << 16) + colno;
return false;
}
return expression_tree_walker(node, SS_make_multiexprs_unique_walker,
context);
}
/*
* SS_process_ctes: process a query's WITH list