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

Commitc1d9579

Browse files
committed
Avoid listing ungrouped Vars in the targetlist of Agg-underneath-Window.
Regular aggregate functions in combination with, or within the argumentsof, window functions are OK per spec; they have the semantics that theaggregate output rows are computed and then we run the window functionsover that row set. (Thus, this combination is not really useful unlessthere's a GROUP BY so that more than one aggregate output row is possible.)The case without GROUP BY could fail, as recently reported by Jeff Davis,because sloppy construction of the Agg node's targetlist resulted in extrareferences to possibly-ungrouped Vars appearing outside the aggregatefunction calls themselves. See the added regression test case for anexample.Fixing this requires modifying the API of flatten_tlist and its underlyingfunction pull_var_clause. I chose to make pull_var_clause's API foraggregates identical to what it was already doing for placeholders, sincethe useful behaviors turn out to be the same (error, report node as-is, orrecurse into it). I also tightened the error checking in this area a bit:if it was ever valid to see an uplevel Var, Aggref, or PlaceHolderVar here,that was a long time ago, so complain instead of ignoring them.Backpatch into 9.1. The failure exists in 8.4 and 9.0 as well, but seeingthat it only occurs in a basically-useless corner case, it doesn't seemworth the risks of changing a function API in a minor release. There mightbe third-party code using pull_var_clause.
1 parent846af54 commitc1d9579

File tree

18 files changed

+119
-78
lines changed

18 files changed

+119
-78
lines changed

‎src/backend/catalog/heap.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1874,7 +1874,9 @@ StoreRelCheck(Relation rel, char *ccname, Node *expr,
18741874
* in check constraints; it would fail to examine the contents of
18751875
* subselects.
18761876
*/
1877-
varList=pull_var_clause(expr,PVC_REJECT_PLACEHOLDERS);
1877+
varList=pull_var_clause(expr,
1878+
PVC_REJECT_AGGREGATES,
1879+
PVC_REJECT_PLACEHOLDERS);
18781880
keycount=list_length(varList);
18791881

18801882
if (keycount>0)
@@ -2171,7 +2173,9 @@ AddRelationNewConstraints(Relation rel,
21712173
List*vars;
21722174
char*colname;
21732175

2174-
vars=pull_var_clause(expr,PVC_REJECT_PLACEHOLDERS);
2176+
vars=pull_var_clause(expr,
2177+
PVC_REJECT_AGGREGATES,
2178+
PVC_REJECT_PLACEHOLDERS);
21752179

21762180
/* eliminate duplicates */
21772181
vars=list_union(NIL,vars);

‎src/backend/commands/trigger.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,9 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
313313
* subselects in WHEN clauses; it would fail to examine the contents
314314
* of subselects.
315315
*/
316-
varList=pull_var_clause(whenClause,PVC_REJECT_PLACEHOLDERS);
316+
varList=pull_var_clause(whenClause,
317+
PVC_REJECT_AGGREGATES,
318+
PVC_REJECT_PLACEHOLDERS);
317319
foreach(lc,varList)
318320
{
319321
Var*var= (Var*)lfirst(lc);

‎src/backend/optimizer/path/allpaths.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1304,15 +1304,18 @@ qual_is_pushdown_safe(Query *subquery, Index rti, Node *qual,
13041304

13051305
/*
13061306
* It would be unsafe to push down window function calls, but at least for
1307-
* the moment we could never see any in a qual anyhow.
1307+
* the moment we could never see any in a qual anyhow. (The same applies
1308+
* to aggregates, which we check for in pull_var_clause below.)
13081309
*/
13091310
Assert(!contain_window_function(qual));
13101311

13111312
/*
13121313
* Examine all Vars used in clause; since it's a restriction clause, all
13131314
* such Vars must refer to subselect output columns.
13141315
*/
1315-
vars=pull_var_clause(qual,PVC_INCLUDE_PLACEHOLDERS);
1316+
vars=pull_var_clause(qual,
1317+
PVC_REJECT_AGGREGATES,
1318+
PVC_INCLUDE_PLACEHOLDERS);
13161319
foreach(vl,vars)
13171320
{
13181321
Var*var= (Var*)lfirst(vl);

‎src/backend/optimizer/path/equivclass.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -847,6 +847,7 @@ generate_base_implied_equalities_no_const(PlannerInfo *root,
847847
{
848848
EquivalenceMember*cur_em= (EquivalenceMember*)lfirst(lc);
849849
List*vars=pull_var_clause((Node*)cur_em->em_expr,
850+
PVC_RECURSE_AGGREGATES,
850851
PVC_INCLUDE_PLACEHOLDERS);
851852

852853
add_vars_to_targetlist(root,vars,ec->ec_relids);

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3552,6 +3552,7 @@ prepare_sort_from_pathkeys(PlannerInfo *root, Plan *lefttree, List *pathkeys,
35523552
continue;
35533553
sortexpr=em->em_expr;
35543554
exprvars=pull_var_clause((Node*)sortexpr,
3555+
PVC_RECURSE_AGGREGATES,
35553556
PVC_INCLUDE_PLACEHOLDERS);
35563557
foreach(k,exprvars)
35573558
{

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ void
132132
build_base_rel_tlists(PlannerInfo*root,List*final_tlist)
133133
{
134134
List*tlist_vars=pull_var_clause((Node*)final_tlist,
135+
PVC_RECURSE_AGGREGATES,
135136
PVC_INCLUDE_PLACEHOLDERS);
136137

137138
if (tlist_vars!=NIL)
@@ -1030,7 +1031,9 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
10301031
*/
10311032
if (bms_membership(relids)==BMS_MULTIPLE)
10321033
{
1033-
List*vars=pull_var_clause(clause,PVC_INCLUDE_PLACEHOLDERS);
1034+
List*vars=pull_var_clause(clause,
1035+
PVC_RECURSE_AGGREGATES,
1036+
PVC_INCLUDE_PLACEHOLDERS);
10341037

10351038
add_vars_to_targetlist(root,vars,relids);
10361039
list_free(vars);

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

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1474,11 +1474,16 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
14741474
* step. That's handled internally by make_sort_from_pathkeys,
14751475
* but we need the copyObject steps here to ensure that each plan
14761476
* node has a separately modifiable tlist.
1477+
*
1478+
* Note: it's essential here to use PVC_INCLUDE_AGGREGATES so that
1479+
* Vars mentioned only in aggregate expressions aren't pulled out
1480+
* as separate targetlist entries. Otherwise we could be putting
1481+
* ungrouped Vars directly into an Agg node's tlist, resulting in
1482+
* undefined behavior.
14771483
*/
1478-
window_tlist=flatten_tlist(tlist);
1479-
if (parse->hasAggs)
1480-
window_tlist=add_to_flat_tlist(window_tlist,
1481-
pull_agg_clause((Node*)tlist));
1484+
window_tlist=flatten_tlist(tlist,
1485+
PVC_INCLUDE_AGGREGATES,
1486+
PVC_INCLUDE_PLACEHOLDERS);
14821487
window_tlist=add_volatile_sort_exprs(window_tlist,tlist,
14831488
activeWindows);
14841489
result_plan->targetlist= (List*)copyObject(window_tlist);
@@ -2577,14 +2582,18 @@ make_subplanTargetList(PlannerInfo *root,
25772582
}
25782583

25792584
/*
2580-
* Otherwise, start with a "flattened" tlist (having just thevars
2581-
* mentioned in the targetlist and HAVING qual --- but not upper-level
2582-
*Vars; they will be replaced by Params later on). Note this includes
2583-
*vars used in resjunk items, so we are covering the needs of ORDER BY
2584-
*and window specifications.
2585+
* Otherwise, start with a "flattened" tlist (having just theVars
2586+
* mentioned in the targetlist and HAVING qual). Note this includes Vars
2587+
*used in resjunk items, so we are covering the needs of ORDER BY and
2588+
*window specifications. Vars used within Aggrefs will be pulled out
2589+
*here, too.
25852590
*/
2586-
sub_tlist=flatten_tlist(tlist);
2587-
extravars=pull_var_clause(parse->havingQual,PVC_INCLUDE_PLACEHOLDERS);
2591+
sub_tlist=flatten_tlist(tlist,
2592+
PVC_RECURSE_AGGREGATES,
2593+
PVC_INCLUDE_PLACEHOLDERS);
2594+
extravars=pull_var_clause(parse->havingQual,
2595+
PVC_RECURSE_AGGREGATES,
2596+
PVC_INCLUDE_PLACEHOLDERS);
25882597
sub_tlist=add_to_flat_tlist(sub_tlist,extravars);
25892598
list_free(extravars);
25902599
*need_tlist_eval= false;/* only eval if not flat tlist */

‎src/backend/optimizer/prep/preptlist.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
154154
ListCell*l;
155155

156156
vars=pull_var_clause((Node*)parse->returningList,
157+
PVC_RECURSE_AGGREGATES,
157158
PVC_INCLUDE_PLACEHOLDERS);
158159
foreach(l,vars)
159160
{

‎src/backend/optimizer/util/clauses.c

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,6 @@ typedef struct
8484
}inline_error_callback_arg;
8585

8686
staticboolcontain_agg_clause_walker(Node*node,void*context);
87-
staticboolpull_agg_clause_walker(Node*node,List**context);
8887
staticboolcount_agg_clauses_walker(Node*node,
8988
count_agg_clauses_context*context);
9089
staticboolfind_window_functions_walker(Node*node,WindowFuncLists*lists);
@@ -418,41 +417,6 @@ contain_agg_clause_walker(Node *node, void *context)
418417
returnexpression_tree_walker(node,contain_agg_clause_walker,context);
419418
}
420419

421-
/*
422-
* pull_agg_clause
423-
* Recursively search for Aggref nodes within a clause.
424-
*
425-
* Returns a List of all Aggrefs found.
426-
*
427-
* This does not descend into subqueries, and so should be used only after
428-
* reduction of sublinks to subplans, or in contexts where it's known there
429-
* are no subqueries. There mustn't be outer-aggregate references either.
430-
*/
431-
List*
432-
pull_agg_clause(Node*clause)
433-
{
434-
List*result=NIL;
435-
436-
(void)pull_agg_clause_walker(clause,&result);
437-
returnresult;
438-
}
439-
440-
staticbool
441-
pull_agg_clause_walker(Node*node,List**context)
442-
{
443-
if (node==NULL)
444-
return false;
445-
if (IsA(node,Aggref))
446-
{
447-
Assert(((Aggref*)node)->agglevelsup==0);
448-
*context=lappend(*context,node);
449-
return false;/* no need to descend into arguments */
450-
}
451-
Assert(!IsA(node,SubLink));
452-
returnexpression_tree_walker(node,pull_agg_clause_walker,
453-
(void*)context);
454-
}
455-
456420
/*
457421
* count_agg_clauses
458422
* Recursively count the Aggref nodes in an expression tree, and

‎src/backend/optimizer/util/placeholder.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,9 @@ find_placeholders_in_qual(PlannerInfo *root, Node *qual, Relids relids)
201201
* pull_var_clause does more than we need here, but it'll do and it's
202202
* convenient to use.
203203
*/
204-
vars=pull_var_clause(qual,PVC_INCLUDE_PLACEHOLDERS);
204+
vars=pull_var_clause(qual,
205+
PVC_RECURSE_AGGREGATES,
206+
PVC_INCLUDE_PLACEHOLDERS);
205207
foreach(vl,vars)
206208
{
207209
PlaceHolderVar*phv= (PlaceHolderVar*)lfirst(vl);
@@ -348,6 +350,7 @@ fix_placeholder_input_needed_levels(PlannerInfo *root)
348350
if (bms_membership(eval_at)==BMS_MULTIPLE)
349351
{
350352
List*vars=pull_var_clause((Node*)phinfo->ph_var->phexpr,
353+
PVC_RECURSE_AGGREGATES,
351354
PVC_INCLUDE_PLACEHOLDERS);
352355

353356
add_vars_to_targetlist(root,vars,eval_at);

‎src/backend/optimizer/util/tlist.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,8 @@ tlist_member_ignore_relabel(Node *node, List *targetlist)
7676
* flatten_tlist
7777
* Create a target list that only contains unique variables.
7878
*
79-
* Note that Vars with varlevelsup > 0 are not included in the output
80-
* tlist. We expect that those will eventually be replaced with Params,
81-
* but that probably has not happened at the time this routine is called.
79+
* Aggrefs and PlaceHolderVars in the input are treated according to
80+
* aggbehavior and phbehavior, for which see pull_var_clause().
8281
*
8382
* 'tlist' is the current target list
8483
*
@@ -88,10 +87,12 @@ tlist_member_ignore_relabel(Node *node, List *targetlist)
8887
* Copying the Var nodes is probably overkill, but be safe for now.
8988
*/
9089
List*
91-
flatten_tlist(List*tlist)
90+
flatten_tlist(List*tlist,PVCAggregateBehavioraggbehavior,
91+
PVCPlaceHolderBehaviorphbehavior)
9292
{
9393
List*vlist=pull_var_clause((Node*)tlist,
94-
PVC_INCLUDE_PLACEHOLDERS);
94+
aggbehavior,
95+
phbehavior);
9596
List*new_tlist;
9697

9798
new_tlist=add_to_flat_tlist(NIL,vlist);

‎src/backend/optimizer/util/var.c

Lines changed: 42 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ typedef struct
5656
typedefstruct
5757
{
5858
List*varlist;
59-
PVCPlaceHolderBehaviorbehavior;
59+
PVCAggregateBehavioraggbehavior;
60+
PVCPlaceHolderBehaviorphbehavior;
6061
}pull_var_clause_context;
6162

6263
typedefstruct
@@ -616,16 +617,22 @@ find_minimum_var_level_walker(Node *node,
616617
* pull_var_clause
617618
* Recursively pulls all Var nodes from an expression clause.
618619
*
619-
* PlaceHolderVars are handled according to 'behavior':
620+
* Aggrefs are handled according to 'aggbehavior':
621+
*PVC_REJECT_AGGREGATESthrow error if Aggref found
622+
*PVC_INCLUDE_AGGREGATESinclude Aggrefs in output list
623+
*PVC_RECURSE_AGGREGATESrecurse into Aggref arguments
624+
* Vars within an Aggref's expression are included only in the last case.
625+
*
626+
* PlaceHolderVars are handled according to 'phbehavior':
620627
*PVC_REJECT_PLACEHOLDERSthrow error if PlaceHolderVar found
621628
*PVC_INCLUDE_PLACEHOLDERSinclude PlaceHolderVars in output list
622-
*PVC_RECURSE_PLACEHOLDERSrecurse into PlaceHolderVarargument
629+
*PVC_RECURSE_PLACEHOLDERSrecurse into PlaceHolderVararguments
623630
* Vars within a PHV's expression are included only in the last case.
624631
*
625632
* CurrentOfExpr nodes are ignored in all cases.
626633
*
627-
* Upper-level vars (with varlevelsup > 0)are notincluded.
628-
*(These probably represent errors too, but we don't complain.)
634+
* Upper-level vars (with varlevelsup > 0)should notbe seen here,
635+
*likewise for upper-level Aggrefs and PlaceHolderVars.
629636
*
630637
* Returns list of nodes found.Note the nodes themselves are not
631638
* copied, only referenced.
@@ -634,12 +641,14 @@ find_minimum_var_level_walker(Node *node,
634641
* of sublinks to subplans!
635642
*/
636643
List*
637-
pull_var_clause(Node*node,PVCPlaceHolderBehaviorbehavior)
644+
pull_var_clause(Node*node,PVCAggregateBehavioraggbehavior,
645+
PVCPlaceHolderBehaviorphbehavior)
638646
{
639647
pull_var_clause_contextcontext;
640648

641649
context.varlist=NIL;
642-
context.behavior=behavior;
650+
context.aggbehavior=aggbehavior;
651+
context.phbehavior=phbehavior;
643652

644653
pull_var_clause_walker(node,&context);
645654
returncontext.varlist;
@@ -652,20 +661,40 @@ pull_var_clause_walker(Node *node, pull_var_clause_context *context)
652661
return false;
653662
if (IsA(node,Var))
654663
{
655-
if (((Var*)node)->varlevelsup==0)
656-
context->varlist=lappend(context->varlist,node);
664+
if (((Var*)node)->varlevelsup!=0)
665+
elog(ERROR,"Upper-level Var found where not expected");
666+
context->varlist=lappend(context->varlist,node);
657667
return false;
658668
}
659-
if (IsA(node,PlaceHolderVar))
669+
elseif (IsA(node,Aggref))
670+
{
671+
if (((Aggref*)node)->agglevelsup!=0)
672+
elog(ERROR,"Upper-level Aggref found where not expected");
673+
switch (context->aggbehavior)
674+
{
675+
casePVC_REJECT_AGGREGATES:
676+
elog(ERROR,"Aggref found where not expected");
677+
break;
678+
casePVC_INCLUDE_AGGREGATES:
679+
context->varlist=lappend(context->varlist,node);
680+
/* we do NOT descend into the contained expression */
681+
return false;
682+
casePVC_RECURSE_AGGREGATES:
683+
/* ignore the aggregate, look at its argument instead */
684+
break;
685+
}
686+
}
687+
elseif (IsA(node,PlaceHolderVar))
660688
{
661-
switch (context->behavior)
689+
if (((PlaceHolderVar*)node)->phlevelsup!=0)
690+
elog(ERROR,"Upper-level PlaceHolderVar found where not expected");
691+
switch (context->phbehavior)
662692
{
663693
casePVC_REJECT_PLACEHOLDERS:
664694
elog(ERROR,"PlaceHolderVar found where not expected");
665695
break;
666696
casePVC_INCLUDE_PLACEHOLDERS:
667-
if (((PlaceHolderVar*)node)->phlevelsup==0)
668-
context->varlist=lappend(context->varlist,node);
697+
context->varlist=lappend(context->varlist,node);
669698
/* we do NOT descend into the contained expression */
670699
return false;
671700
casePVC_RECURSE_PLACEHOLDERS:

‎src/backend/utils/adt/selfuncs.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3090,7 +3090,9 @@ estimate_num_groups(PlannerInfo *root, List *groupExprs, double input_rows)
30903090
* PlaceHolderVar doesn't change the number of groups, which boils
30913091
* down to ignoring the possible addition of nulls to the result set).
30923092
*/
3093-
varshere=pull_var_clause(groupexpr,PVC_RECURSE_PLACEHOLDERS);
3093+
varshere=pull_var_clause(groupexpr,
3094+
PVC_RECURSE_AGGREGATES,
3095+
PVC_RECURSE_PLACEHOLDERS);
30943096

30953097
/*
30963098
* If we find any variable-free GROUP BY item, then either it is a

‎src/include/optimizer/clauses.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ extern Expr *make_ands_explicit(List *andclauses);
4848
externList*make_ands_implicit(Expr*clause);
4949

5050
externboolcontain_agg_clause(Node*clause);
51-
externList*pull_agg_clause(Node*clause);
5251
externvoidcount_agg_clauses(PlannerInfo*root,Node*clause,
5352
AggClauseCosts*costs);
5453

‎src/include/optimizer/tlist.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,14 @@
1414
#ifndefTLIST_H
1515
#defineTLIST_H
1616

17-
#include"nodes/relation.h"
17+
#include"optimizer/var.h"
1818

1919

2020
externTargetEntry*tlist_member(Node*node,List*targetlist);
2121
externTargetEntry*tlist_member_ignore_relabel(Node*node,List*targetlist);
2222

23-
externList*flatten_tlist(List*tlist);
23+
externList*flatten_tlist(List*tlist,PVCAggregateBehavioraggbehavior,
24+
PVCPlaceHolderBehaviorphbehavior);
2425
externList*add_to_flat_tlist(List*tlist,List*exprs);
2526

2627
externList*get_tlist_exprs(List*tlist,boolincludeJunk);

0 commit comments

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