Skip to content

Commit 364a9f4

Browse files
tglsfdctglsfdc
committed
Refactor pull_var_clause's API to make it less tedious to extend.
In commit 1d97c19 and later c1d9579, we extended pull_var_clause's API by adding enum-type arguments. That's sort of a pain to maintain, though, because it means every time we add a new behavior we must touch every last one of the call sites, even if there's a reasonable default behavior that most of them could use. Let's switch over to using a bitmask of flags, instead; that seems more maintainable and might save a nanosecond or two as well. This commit changes no behavior in itself, though I'm going to follow it up with one that does add a new behavior. In passing, remove flatten_tlist(), which has not been used since 9.1 and would otherwise need the same API changes. Removing these enums means that optimizer/tlist.h no longer needs to depend on optimizer/var.h. Changing that caused a number of C files to need addition of #include "optimizer/var.h" (probably we can thank old runs of pgrminclude for that); but on balance it seems like a good change anyway.
1 parent 37c5486 commit 364a9f4

File tree

20 files changed

+94
-128
lines changed

20 files changed

+94
-128
lines changed

‎contrib/postgres_fdw/deparse.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -734,7 +734,6 @@ build_tlist_to_deparse(RelOptInfo *foreignrel)
734734
tlist = add_to_flat_tlist(tlist, foreignrel->reltarget.exprs);
735735
tlist = add_to_flat_tlist(tlist,
736736
pull_var_clause((Node *) fpinfo->local_conds,
737-
PVC_REJECT_AGGREGATES,
738737
PVC_RECURSE_PLACEHOLDERS));
739738

740739
return tlist;

‎src/backend/catalog/heap.c

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2006,9 +2006,7 @@ StoreRelCheck(Relation rel, char *ccname, Node *expr,
20062006
* in check constraints; it would fail to examine the contents of
20072007
* subselects.
20082008
*/
2009-
varList = pull_var_clause(expr,
2010-
PVC_REJECT_AGGREGATES,
2011-
PVC_REJECT_PLACEHOLDERS);
2009+
varList = pull_var_clause(expr, 0);
20122010
keycount = list_length(varList);
20132011

20142012
if (keycount > 0)
@@ -2323,9 +2321,7 @@ AddRelationNewConstraints(Relation rel,
23232321
List *vars;
23242322
char *colname;
23252323

2326-
vars = pull_var_clause(expr,
2327-
PVC_REJECT_AGGREGATES,
2328-
PVC_REJECT_PLACEHOLDERS);
2324+
vars = pull_var_clause(expr, 0);
23292325

23302326
/* eliminate duplicates */
23312327
vars = list_union(NIL, vars);

‎src/backend/commands/trigger.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -352,9 +352,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
352352
* subselects in WHEN clauses; it would fail to examine the contents
353353
* of subselects.
354354
*/
355-
varList = pull_var_clause(whenClause,
356-
PVC_REJECT_AGGREGATES,
357-
PVC_REJECT_PLACEHOLDERS);
355+
varList = pull_var_clause(whenClause, 0);
358356
foreach(lc, varList)
359357
{
360358
Var *var = (Var *) lfirst(lc);

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2509,9 +2509,7 @@ qual_is_pushdown_safe(Query *subquery, Index rti, Node *qual,
25092509
* Examine all Vars used in clause; since it's a restriction clause, all
25102510
* such Vars must refer to subselect output columns.
25112511
*/
2512-
vars = pull_var_clause(qual,
2513-
PVC_REJECT_AGGREGATES,
2514-
PVC_INCLUDE_PLACEHOLDERS);
2512+
vars = pull_var_clause(qual, PVC_INCLUDE_PLACEHOLDERS);
25152513
foreach(vl, vars)
25162514
{
25172515
Var *var = (Var *) lfirst(vl);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -910,7 +910,7 @@ generate_base_implied_equalities_no_const(PlannerInfo *root,
910910
{
911911
EquivalenceMember *cur_em = (EquivalenceMember *) lfirst(lc);
912912
List *vars = pull_var_clause((Node *) cur_em->em_expr,
913-
PVC_RECURSE_AGGREGATES,
913+
PVC_RECURSE_AGGREGATES |
914914
PVC_INCLUDE_PLACEHOLDERS);
915915

916916
add_vars_to_targetlist(root, vars, ec->ec_relids, false);

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "optimizer/paths.h"
3030
#include "optimizer/planmain.h"
3131
#include "optimizer/tlist.h"
32+
#include "optimizer/var.h"
3233
#include "utils/lsyscache.h"
3334

3435
/* local functions */

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5335,7 +5335,7 @@ prepare_sort_from_pathkeys(Plan *lefttree, List *pathkeys,
53355335

53365336
sortexpr = em->em_expr;
53375337
exprvars = pull_var_clause((Node *) sortexpr,
5338-
PVC_INCLUDE_AGGREGATES,
5338+
PVC_INCLUDE_AGGREGATES |
53395339
PVC_INCLUDE_PLACEHOLDERS);
53405340
foreach(k, exprvars)
53415341
{

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ void
146146
build_base_rel_tlists(PlannerInfo *root, List *final_tlist)
147147
{
148148
List *tlist_vars = pull_var_clause((Node *) final_tlist,
149-
PVC_RECURSE_AGGREGATES,
149+
PVC_RECURSE_AGGREGATES |
150150
PVC_INCLUDE_PLACEHOLDERS);
151151

152152
if (tlist_vars != NIL)
@@ -161,7 +161,7 @@ build_base_rel_tlists(PlannerInfo *root, List *final_tlist)
161161
if (root->parse->havingQual)
162162
{
163163
List *having_vars = pull_var_clause(root->parse->havingQual,
164-
PVC_RECURSE_AGGREGATES,
164+
PVC_RECURSE_AGGREGATES |
165165
PVC_INCLUDE_PLACEHOLDERS);
166166

167167
if (having_vars != NIL)
@@ -1787,7 +1787,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
17871787
if (bms_membership(relids) == BMS_MULTIPLE)
17881788
{
17891789
List *vars = pull_var_clause(clause,
1790-
PVC_RECURSE_AGGREGATES,
1790+
PVC_RECURSE_AGGREGATES |
17911791
PVC_INCLUDE_PLACEHOLDERS);
17921792

17931793
add_vars_to_targetlist(root, vars, relids, false);

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
#include "optimizer/prep.h"
4444
#include "optimizer/subselect.h"
4545
#include "optimizer/tlist.h"
46+
#include "optimizer/var.h"
4647
#include "parser/analyze.h"
4748
#include "parser/parsetree.h"
4849
#include "parser/parse_agg.h"
@@ -3840,7 +3841,7 @@ make_group_input_target(PlannerInfo *root, List *tlist)
38403841
* pulled out here, too.
38413842
*/
38423843
non_group_vars = pull_var_clause((Node *) non_group_cols,
3843-
PVC_RECURSE_AGGREGATES,
3844+
PVC_RECURSE_AGGREGATES |
38443845
PVC_INCLUDE_PLACEHOLDERS);
38453846
sub_tlist = add_to_flat_tlist(sub_tlist, non_group_vars);
38463847

@@ -4088,7 +4089,7 @@ make_window_input_target(PlannerInfo *root,
40884089
* at higher levels.
40894090
*/
40904091
flattenable_vars = pull_var_clause((Node *) flattenable_cols,
4091-
PVC_INCLUDE_AGGREGATES,
4092+
PVC_INCLUDE_AGGREGATES |
40924093
PVC_INCLUDE_PLACEHOLDERS);
40934094
new_tlist = add_to_flat_tlist(new_tlist, flattenable_vars);
40944095

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

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1641,12 +1641,12 @@ set_join_references(PlannerInfo *root, Join *join, int rtoffset)
16411641
*
16421642
* In most cases, we have to match up individual Vars in the tlist and
16431643
* qual expressions with elements of the subplan's tlist (which was
1644-
* generated by flatten_tlist() from these selfsame expressions, so it
1645-
* should have all the required variables). There is an important exception,
1646-
* however: GROUP BY and ORDER BY expressions will have been pushed into the
1647-
* subplan tlist unflattened. If these values are also needed in the output
1648-
* then we want to reference the subplan tlist element rather than recomputing
1649-
* the expression.
1644+
* generated by flattening these selfsame expressions, so it should have all
1645+
* the required variables). There is an important exception, however:
1646+
* depending on where we are in the plan tree, sort/group columns may have
1647+
* been pushed into the subplan tlist unflattened. If these values are also
1648+
* needed in the output then we want to reference the subplan tlist element
1649+
* rather than recomputing the expression.
16501650
*/
16511651
static void
16521652
set_upper_references(PlannerInfo *root, Plan *plan, int rtoffset)
@@ -2129,7 +2129,8 @@ fix_join_expr_mutator(Node *node, fix_join_expr_context *context)
21292129
*
21302130
* An error is raised if no matching var can be found in the subplan tlist
21312131
* --- so this routine should only be applied to nodes whose subplans'
2132-
* targetlists were generated via flatten_tlist() or some such method.
2132+
* targetlists were generated by flattening the expressions used in the
2133+
* parent node.
21332134
*
21342135
* If itlist->has_non_vars is true, then we try to match whole subexpressions
21352136
* against elements of the subplan tlist, so that we can avoid recomputing

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include "optimizer/prep.h"
3232
#include "optimizer/subselect.h"
3333
#include "optimizer/tlist.h"
34+
#include "optimizer/var.h"
3435
#include "parser/parse_relation.h"
3536
#include "parser/parsetree.h"
3637
#include "rewrite/rewriteManip.h"

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
#include "nodes/makefuncs.h"
4545
#include "optimizer/prep.h"
4646
#include "optimizer/tlist.h"
47+
#include "optimizer/var.h"
4748
#include "parser/parsetree.h"
4849
#include "parser/parse_coerce.h"
4950
#include "utils/rel.h"
@@ -167,7 +168,7 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
167168
ListCell *l;
168169

169170
vars = pull_var_clause((Node *) parse->returningList,
170-
PVC_RECURSE_AGGREGATES,
171+
PVC_RECURSE_AGGREGATES |
171172
PVC_INCLUDE_PLACEHOLDERS);
172173
foreach(l, vars)
173174
{

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ find_placeholders_in_expr(PlannerInfo *root, Node *expr)
220220
* convenient to use.
221221
*/
222222
vars = pull_var_clause(expr,
223-
PVC_RECURSE_AGGREGATES,
223+
PVC_RECURSE_AGGREGATES |
224224
PVC_INCLUDE_PLACEHOLDERS);
225225
foreach(vl, vars)
226226
{
@@ -354,7 +354,7 @@ fix_placeholder_input_needed_levels(PlannerInfo *root)
354354
{
355355
PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(lc);
356356
List *vars = pull_var_clause((Node *) phinfo->ph_var->phexpr,
357-
PVC_RECURSE_AGGREGATES,
357+
PVC_RECURSE_AGGREGATES |
358358
PVC_INCLUDE_PLACEHOLDERS);
359359

360360
add_vars_to_targetlist(root, vars, phinfo->ph_eval_at, false);

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

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -100,34 +100,6 @@ tlist_member_match_var(Var *var, List *targetlist)
100100
return NULL;
101101
}
102102

103-
/*
104-
* flatten_tlist
105-
* Create a target list that only contains unique variables.
106-
*
107-
* Aggrefs and PlaceHolderVars in the input are treated according to
108-
* aggbehavior and phbehavior, for which see pull_var_clause().
109-
*
110-
* 'tlist' is the current target list
111-
*
112-
* Returns the "flattened" new target list.
113-
*
114-
* The result is entirely new structure sharing no nodes with the original.
115-
* Copying the Var nodes is probably overkill, but be safe for now.
116-
*/
117-
List *
118-
flatten_tlist(List *tlist, PVCAggregateBehavior aggbehavior,
119-
PVCPlaceHolderBehavior phbehavior)
120-
{
121-
List *vlist = pull_var_clause((Node *) tlist,
122-
aggbehavior,
123-
phbehavior);
124-
List *new_tlist;
125-
126-
new_tlist = add_to_flat_tlist(NIL, vlist);
127-
list_free(vlist);
128-
return new_tlist;
129-
}
130-
131103
/*
132104
* add_to_flat_tlist
133105
* Add more items to a flattened tlist (if they're not already in it)

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

Lines changed: 56 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,7 @@ typedef struct
5555
typedef struct
5656
{
5757
List *varlist;
58-
PVCAggregateBehavior aggbehavior;
59-
PVCPlaceHolderBehavior phbehavior;
58+
int flags;
6059
} pull_var_clause_context;
6160

6261
typedef struct
@@ -497,17 +496,22 @@ locate_var_of_level_walker(Node *node,
497496
* pull_var_clause
498497
* Recursively pulls all Var nodes from an expression clause.
499498
*
500-
* Aggrefs are handled according to 'aggbehavior':
501-
* PVC_REJECT_AGGREGATES throw error if Aggref found
499+
* Aggrefs are handled according to these bits in 'flags':
502500
* PVC_INCLUDE_AGGREGATES include Aggrefs in output list
503501
* PVC_RECURSE_AGGREGATES recurse into Aggref arguments
504-
* Vars within an Aggref's expression are included only in the last case.
502+
* neither flag throw error if Aggref found
503+
* Vars within an Aggref's expression are included in the result only
504+
* when PVC_RECURSE_AGGREGATES is specified.
505505
*
506-
* PlaceHolderVars are handled according to 'phbehavior':
507-
* PVC_REJECT_PLACEHOLDERS throw error if PlaceHolderVar found
506+
* PlaceHolderVars are handled according to these bits in 'flags':
508507
* PVC_INCLUDE_PLACEHOLDERS include PlaceHolderVars in output list
509508
* PVC_RECURSE_PLACEHOLDERS recurse into PlaceHolderVar arguments
510-
* Vars within a PHV's expression are included only in the last case.
509+
* neither flag throw error if PlaceHolderVar found
510+
* Vars within a PHV's expression are included in the result only
511+
* when PVC_RECURSE_PLACEHOLDERS is specified.
512+
*
513+
* GroupingFuncs are treated mostly like Aggrefs, and so do not need
514+
* their own flag bits.
511515
*
512516
* CurrentOfExpr nodes are ignored in all cases.
513517
*
@@ -521,14 +525,18 @@ locate_var_of_level_walker(Node *node,
521525
* of sublinks to subplans!
522526
*/
523527
List *
524-
pull_var_clause(Node *node, PVCAggregateBehavior aggbehavior,
525-
PVCPlaceHolderBehavior phbehavior)
528+
pull_var_clause(Node *node, int flags)
526529
{
527530
pull_var_clause_context context;
528531

532+
/* Assert that caller has not specified inconsistent flags */
533+
Assert((flags & (PVC_INCLUDE_AGGREGATES | PVC_RECURSE_AGGREGATES))
534+
!= (PVC_INCLUDE_AGGREGATES | PVC_RECURSE_AGGREGATES));
535+
Assert((flags & (PVC_INCLUDE_PLACEHOLDERS | PVC_RECURSE_PLACEHOLDERS))
536+
!= (PVC_INCLUDE_PLACEHOLDERS | PVC_RECURSE_PLACEHOLDERS));
537+
529538
context.varlist = NIL;
530-
context.aggbehavior = aggbehavior;
531-
context.phbehavior = phbehavior;
539+
context.flags = flags;
532540

533541
pull_var_clause_walker(node, &context);
534542
return context.varlist;
@@ -550,62 +558,58 @@ pull_var_clause_walker(Node *node, pull_var_clause_context *context)
550558
{
551559
if (((Aggref *) node)->agglevelsup != 0)
552560
elog(ERROR, "Upper-level Aggref found where not expected");
553-
switch (context->aggbehavior)
561+
if (context->flags & PVC_INCLUDE_AGGREGATES)
562+
{
563+
context->varlist = lappend(context->varlist, node);
564+
/* we do NOT descend into the contained expression */
565+
return false;
566+
}
567+
else if (context->flags & PVC_RECURSE_AGGREGATES)
554568
{
555-
case PVC_REJECT_AGGREGATES:
556-
elog(ERROR, "Aggref found where not expected");
557-
break;
558-
case PVC_INCLUDE_AGGREGATES:
559-
context->varlist = lappend(context->varlist, node);
560-
/* we do NOT descend into the contained expression */
561-
return false;
562-
case PVC_RECURSE_AGGREGATES:
563-
/* ignore the aggregate, look at its argument instead */
564-
break;
569+
/* fall through to recurse into the aggregate's arguments */
565570
}
571+
else
572+
elog(ERROR, "Aggref found where not expected");
566573
}
567574
else if (IsA(node, GroupingFunc))
568575
{
569576
if (((GroupingFunc *) node)->agglevelsup != 0)
570577
elog(ERROR, "Upper-level GROUPING found where not expected");
571-
switch (context->aggbehavior)
578+
if (context->flags & PVC_INCLUDE_AGGREGATES)
572579
{
573-
case PVC_REJECT_AGGREGATES:
574-
elog(ERROR, "GROUPING found where not expected");
575-
break;
576-
case PVC_INCLUDE_AGGREGATES:
577-
context->varlist = lappend(context->varlist, node);
578-
/* we do NOT descend into the contained expression */
579-
return false;
580-
case PVC_RECURSE_AGGREGATES:
581-
582-
/*
583-
* we do NOT descend into the contained expression, even if
584-
* the caller asked for it, because we never actually evaluate
585-
* it - the result is driven entirely off the associated GROUP
586-
* BY clause, so we never need to extract the actual Vars
587-
* here.
588-
*/
589-
return false;
580+
context->varlist = lappend(context->varlist, node);
581+
/* we do NOT descend into the contained expression */
582+
return false;
583+
}
584+
else if (context->flags & PVC_RECURSE_AGGREGATES)
585+
{
586+
/*
587+
* We do NOT descend into the contained expression, even if the
588+
* caller asked for it, because we never actually evaluate it -
589+
* the result is driven entirely off the associated GROUP BY
590+
* clause, so we never need to extract the actual Vars here.
591+
*/
592+
return false;
590593
}
594+
else
595+
elog(ERROR, "GROUPING found where not expected");
591596
}
592597
else if (IsA(node, PlaceHolderVar))
593598
{
594599
if (((PlaceHolderVar *) node)->phlevelsup != 0)
595600
elog(ERROR, "Upper-level PlaceHolderVar found where not expected");
596-
switch (context->phbehavior)
601+
if (context->flags & PVC_INCLUDE_PLACEHOLDERS)
597602
{
598-
case PVC_REJECT_PLACEHOLDERS:
599-
elog(ERROR, "PlaceHolderVar found where not expected");
600-
break;
601-
case PVC_INCLUDE_PLACEHOLDERS:
602-
context->varlist = lappend(context->varlist, node);
603-
/* we do NOT descend into the contained expression */
604-
return false;
605-
case PVC_RECURSE_PLACEHOLDERS:
606-
/* ignore the placeholder, look at its argument instead */
607-
break;
603+
context->varlist = lappend(context->varlist, node);
604+
/* we do NOT descend into the contained expression */
605+
return false;
608606
}
607+
else if (context->flags & PVC_RECURSE_PLACEHOLDERS)
608+
{
609+
/* fall through to recurse into the placeholder's expression */
610+
}
611+
else
612+
elog(ERROR, "PlaceHolderVar found where not expected");
609613
}
610614
return expression_tree_walker(node, pull_var_clause_walker,
611615
(void *) context);

0 commit comments

Comments
 (0)