From 2d1f94054259d3237e678d89ca9e57305a6a3997 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 4 Feb 2003 00:50:01 +0000 Subject: [PATCH] Minor code cleanup: remove no-longer-useful pull_subplans() function, and convert pull_agg_clause() into count_agg_clause(), which is a more efficient way of doing what it's really being used for. --- src/backend/optimizer/plan/planner.c | 9 +++-- src/backend/optimizer/util/clauses.c | 58 ++++++---------------------- src/include/optimizer/clauses.h | 5 +-- 3 files changed, 20 insertions(+), 52 deletions(-) diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index f01b9feffe..ad68253109 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.143 2003/02/03 15:07:07 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.144 2003/02/04 00:50:00 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -623,11 +623,14 @@ grouping_planner(Query *parse, double tuple_fraction) * Will need actual number of aggregates for estimating costs. * Also, it's possible that optimization has eliminated all * aggregates, and we may as well check for that here. + * + * Note: we do not attempt to detect duplicate aggregates here; + * a somewhat-overestimated count is okay for our present purposes. */ if (parse->hasAggs) { - numAggs = length(pull_agg_clause((Node *) tlist)) + - length(pull_agg_clause(parse->havingQual)); + numAggs = count_agg_clause((Node *) tlist) + + count_agg_clause(parse->havingQual); if (numAggs == 0) parse->hasAggs = false; } diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index f0b8ab1262..a2449cb07a 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.126 2003/02/03 21:15:44 tgl Exp $ + * $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.127 2003/02/04 00:50:00 tgl Exp $ * * HISTORY * AUTHOR DATE MAJOR EVENT @@ -51,10 +51,9 @@ typedef struct static bool contain_agg_clause_walker(Node *node, void *context); static bool contain_distinct_agg_clause_walker(Node *node, void *context); -static bool pull_agg_clause_walker(Node *node, List **listptr); +static bool count_agg_clause_walker(Node *node, int *count); static bool expression_returns_set_walker(Node *node, void *context); static bool contain_subplans_walker(Node *node, void *context); -static bool pull_subplans_walker(Node *node, List **listptr); static bool contain_mutable_functions_walker(Node *node, void *context); static bool contain_volatile_functions_walker(Node *node, void *context); static bool contain_nonstrict_functions_walker(Node *node, void *context); @@ -373,31 +372,28 @@ contain_distinct_agg_clause_walker(Node *node, void *context) } /* - * pull_agg_clause - * Recursively pulls all Aggref nodes from an expression tree. - * - * Returns list of Aggref nodes found. Note the nodes themselves are not - * copied, only referenced. + * count_agg_clause + * Recursively count the Aggref nodes in an expression tree. * * Note: this also checks for nested aggregates, which are an error. */ -List * -pull_agg_clause(Node *clause) +int +count_agg_clause(Node *clause) { - List *result = NIL; + int result = 0; - pull_agg_clause_walker(clause, &result); + count_agg_clause_walker(clause, &result); return result; } static bool -pull_agg_clause_walker(Node *node, List **listptr) +count_agg_clause_walker(Node *node, int *count) { if (node == NULL) return false; if (IsA(node, Aggref)) { - *listptr = lappend(*listptr, node); + (*count)++; /* * Complain if the aggregate's argument contains any aggregates; @@ -411,8 +407,8 @@ pull_agg_clause_walker(Node *node, List **listptr) */ return false; } - return expression_tree_walker(node, pull_agg_clause_walker, - (void *) listptr); + return expression_tree_walker(node, count_agg_clause_walker, + (void *) count); } @@ -511,36 +507,6 @@ contain_subplans_walker(Node *node, void *context) return expression_tree_walker(node, contain_subplans_walker, context); } -/* - * pull_subplans - * Recursively pulls all subplans from an expression tree. - * - * Returns list of SubPlan nodes found. Note the nodes themselves - * are not copied, only referenced. - */ -List * -pull_subplans(Node *clause) -{ - List *result = NIL; - - pull_subplans_walker(clause, &result); - return result; -} - -static bool -pull_subplans_walker(Node *node, List **listptr) -{ - if (node == NULL) - return false; - if (is_subplan(node)) - { - *listptr = lappend(*listptr, node); - /* fall through to check args to subplan */ - } - return expression_tree_walker(node, pull_subplans_walker, - (void *) listptr); -} - /***************************************************************************** * Check clauses for mutable functions diff --git a/src/include/optimizer/clauses.h b/src/include/optimizer/clauses.h index 7ee0f1be2e..5a38d6ef2a 100644 --- a/src/include/optimizer/clauses.h +++ b/src/include/optimizer/clauses.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $Id: clauses.h,v 1.61 2003/01/17 03:25:04 tgl Exp $ + * $Id: clauses.h,v 1.62 2003/02/04 00:50:01 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -46,12 +46,11 @@ extern List *make_ands_implicit(Expr *clause); extern bool contain_agg_clause(Node *clause); extern bool contain_distinct_agg_clause(Node *clause); -extern List *pull_agg_clause(Node *clause); +extern int count_agg_clause(Node *clause); extern bool expression_returns_set(Node *clause); extern bool contain_subplans(Node *clause); -extern List *pull_subplans(Node *clause); extern bool contain_mutable_functions(Node *clause); extern bool contain_volatile_functions(Node *clause);