Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitd0d4404

Browse files
committed
Account for optimized MinMax aggregates during SS_finalize_plan.
We are capable of optimizing MIN() and MAX() aggregates on indexedcolumns into subqueries that exploit the index, rather than the normalthing of scanning the whole table. When we do this, we replace theAggref node(s) with Params referencing subquery outputs. Such Paramsreally ought to be included in the per-plan-node extParam/allParamsets computed by SS_finalize_plan. However, we've never done soup to now because of an ancient implementation choice to performthat substitution during set_plan_references, which runs afterSS_finalize_plan, so that SS_finalize_plan never sees these Params.This seems like clearly a bug, yet there have been no field reportsof problems that could trace to it. This may be because the typesof Plan nodes that could contain Aggrefs do not have any of therescan optimizations that are controlled by extParam/allParam.Nonetheless it seems certain to bite us someday, so let's fix itin a self-contained patch that can be back-patched if we find acase in which there's a live bug pre-v17.The cleanest fix would be to perform a separate tree walk to dothese substitutions before SS_finalize_plan runs. That seemsunattractive, first because a whole-tree mutation pass is expensive,and second because we lack infrastructure for visiting expressionsubtrees in a Plan tree, so that we'd need a new function knowingas much as SS_finalize_plan knows about that. I also consideredswapping the order of SS_finalize_plan and set_plan_references,but that fell foul of various assumptions that seem tricky to fix.So the approach adopted here is to teach SS_finalize_plan itselfto check for such Aggrefs. I refactored things a bit in setrefs.cto avoid having three copies of the code that does that.Given the lack of any currently-known bug, no test case here.Discussion:https://postgr.es/m/2391880.1689025003@sss.pgh.pa.us
1 parentb8d3dae commitd0d4404

File tree

3 files changed

+66
-29
lines changed

3 files changed

+66
-29
lines changed

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

Lines changed: 42 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2181,22 +2181,14 @@ fix_scan_expr_mutator(Node *node, fix_scan_expr_context *context)
21812181
if (IsA(node,Aggref))
21822182
{
21832183
Aggref*aggref= (Aggref*)node;
2184+
Param*aggparam;
21842185

21852186
/* See if the Aggref should be replaced by a Param */
2186-
if(context->root->minmax_aggs!=NIL&&
2187-
list_length(aggref->args)==1)
2187+
aggparam=find_minmax_agg_replacement_param(context->root,aggref);
2188+
if (aggparam!=NULL)
21882189
{
2189-
TargetEntry*curTarget= (TargetEntry*)linitial(aggref->args);
2190-
ListCell*lc;
2191-
2192-
foreach(lc,context->root->minmax_aggs)
2193-
{
2194-
MinMaxAggInfo*mminfo= (MinMaxAggInfo*)lfirst(lc);
2195-
2196-
if (mminfo->aggfnoid==aggref->aggfnoid&&
2197-
equal(mminfo->target,curTarget->expr))
2198-
return (Node*)copyObject(mminfo->param);
2199-
}
2190+
/* Make a copy of the Param for paranoia's sake */
2191+
return (Node*)copyObject(aggparam);
22002192
}
22012193
/* If no match, just fall through to process it normally */
22022194
}
@@ -3225,22 +3217,14 @@ fix_upper_expr_mutator(Node *node, fix_upper_expr_context *context)
32253217
if (IsA(node,Aggref))
32263218
{
32273219
Aggref*aggref= (Aggref*)node;
3220+
Param*aggparam;
32283221

32293222
/* See if the Aggref should be replaced by a Param */
3230-
if(context->root->minmax_aggs!=NIL&&
3231-
list_length(aggref->args)==1)
3223+
aggparam=find_minmax_agg_replacement_param(context->root,aggref);
3224+
if (aggparam!=NULL)
32323225
{
3233-
TargetEntry*curTarget= (TargetEntry*)linitial(aggref->args);
3234-
ListCell*lc;
3235-
3236-
foreach(lc,context->root->minmax_aggs)
3237-
{
3238-
MinMaxAggInfo*mminfo= (MinMaxAggInfo*)lfirst(lc);
3239-
3240-
if (mminfo->aggfnoid==aggref->aggfnoid&&
3241-
equal(mminfo->target,curTarget->expr))
3242-
return (Node*)copyObject(mminfo->param);
3243-
}
3226+
/* Make a copy of the Param for paranoia's sake */
3227+
return (Node*)copyObject(aggparam);
32443228
}
32453229
/* If no match, just fall through to process it normally */
32463230
}
@@ -3395,6 +3379,38 @@ set_windowagg_runcondition_references(PlannerInfo *root,
33953379
returnnewlist;
33963380
}
33973381

3382+
/*
3383+
* find_minmax_agg_replacement_param
3384+
*If the given Aggref is one that we are optimizing into a subquery
3385+
*(cf. planagg.c), then return the Param that should replace it.
3386+
*Else return NULL.
3387+
*
3388+
* This is exported so that SS_finalize_plan can use it before setrefs.c runs.
3389+
* Note that it will not find anything until we have built a Plan from a
3390+
* MinMaxAggPath, as root->minmax_aggs will never be filled otherwise.
3391+
*/
3392+
Param*
3393+
find_minmax_agg_replacement_param(PlannerInfo*root,Aggref*aggref)
3394+
{
3395+
if (root->minmax_aggs!=NIL&&
3396+
list_length(aggref->args)==1)
3397+
{
3398+
TargetEntry*curTarget= (TargetEntry*)linitial(aggref->args);
3399+
ListCell*lc;
3400+
3401+
foreach(lc,root->minmax_aggs)
3402+
{
3403+
MinMaxAggInfo*mminfo= (MinMaxAggInfo*)lfirst(lc);
3404+
3405+
if (mminfo->aggfnoid==aggref->aggfnoid&&
3406+
equal(mminfo->target,curTarget->expr))
3407+
returnmminfo->param;
3408+
}
3409+
}
3410+
returnNULL;
3411+
}
3412+
3413+
33983414
/*****************************************************************************
33993415
*QUERY DEPENDENCY MANAGEMENT
34003416
*****************************************************************************/

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

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2835,8 +2835,8 @@ finalize_plan(PlannerInfo *root, Plan *plan,
28352835
}
28362836

28372837
/*
2838-
* finalize_primnode: add IDs of all PARAM_EXEC paramsappearing in the given
2839-
* expression tree to the result set.
2838+
* finalize_primnode: add IDs of all PARAM_EXEC paramsthat appear (or will
2839+
*appear) in the givenexpression tree to the result set.
28402840
*/
28412841
staticbool
28422842
finalize_primnode(Node*node,finalize_primnode_context*context)
@@ -2853,7 +2853,26 @@ finalize_primnode(Node *node, finalize_primnode_context *context)
28532853
}
28542854
return false;/* no more to do here */
28552855
}
2856-
if (IsA(node,SubPlan))
2856+
elseif (IsA(node,Aggref))
2857+
{
2858+
/*
2859+
* Check to see if the aggregate will be replaced by a Param
2860+
* referencing a subquery output during setrefs.c. If so, we must
2861+
* account for that Param here. (For various reasons, it's not
2862+
* convenient to perform that substitution earlier than setrefs.c, nor
2863+
* to perform this processing after setrefs.c. Thus we need a wart
2864+
* here.)
2865+
*/
2866+
Aggref*aggref= (Aggref*)node;
2867+
Param*aggparam;
2868+
2869+
aggparam=find_minmax_agg_replacement_param(context->root,aggref);
2870+
if (aggparam!=NULL)
2871+
context->paramids=bms_add_member(context->paramids,
2872+
aggparam->paramid);
2873+
/* Fall through to examine the agg's arguments */
2874+
}
2875+
elseif (IsA(node,SubPlan))
28572876
{
28582877
SubPlan*subplan= (SubPlan*)node;
28592878
Plan*plan=planner_subplan_get_plan(context->root,subplan);

‎src/include/optimizer/planmain.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,8 @@ extern bool innerrel_is_unique(PlannerInfo *root,
110110
*/
111111
externPlan*set_plan_references(PlannerInfo*root,Plan*plan);
112112
externbooltrivial_subqueryscan(SubqueryScan*plan);
113+
externParam*find_minmax_agg_replacement_param(PlannerInfo*root,
114+
Aggref*aggref);
113115
externvoidrecord_plan_function_dependency(PlannerInfo*root,Oidfuncid);
114116
externvoidrecord_plan_type_dependency(PlannerInfo*root,Oidtypid);
115117
externboolextract_query_dependencies_walker(Node*node,PlannerInfo*context);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp