aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--src/backend/optimizer/path/equivclass.c10
-rw-r--r--src/backend/optimizer/plan/planagg.c8
-rw-r--r--src/include/optimizer/paths.h3
-rw-r--r--src/test/regress/expected/aggregates.out39
-rw-r--r--src/test/regress/sql/aggregates.sql5
5 files changed, 59 insertions, 6 deletions
diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c
index f84dcdfd1be..57214a33a24 100644
--- a/src/backend/optimizer/path/equivclass.c
+++ b/src/backend/optimizer/path/equivclass.c
@@ -1897,12 +1897,12 @@ add_child_rel_equivalences(PlannerInfo *root,
/*
* mutate_eclass_expressions
* Apply an expression tree mutator to all expressions stored in
- * equivalence classes.
+ * equivalence classes (but ignore child exprs unless include_child_exprs).
*
* This is a bit of a hack ... it's currently needed only by planagg.c,
* which needs to do a global search-and-replace of MIN/MAX Aggrefs
* after eclasses are already set up. Without changing the eclasses too,
- * subsequent matching of ORDER BY clauses would fail.
+ * subsequent matching of ORDER BY and DISTINCT clauses would fail.
*
* Note that we assume the mutation won't affect relation membership or any
* other properties we keep track of (which is a bit bogus, but by the time
@@ -1912,7 +1912,8 @@ add_child_rel_equivalences(PlannerInfo *root,
void
mutate_eclass_expressions(PlannerInfo *root,
Node *(*mutator) (),
- void *context)
+ void *context,
+ bool include_child_exprs)
{
ListCell *lc1;
@@ -1925,6 +1926,9 @@ mutate_eclass_expressions(PlannerInfo *root,
{
EquivalenceMember *cur_em = (EquivalenceMember *) lfirst(lc2);
+ if (cur_em->em_is_child && !include_child_exprs)
+ continue; /* ignore children unless requested */
+
cur_em->em_expr = (Expr *)
mutator((Node *) cur_em->em_expr, context);
}
diff --git a/src/backend/optimizer/plan/planagg.c b/src/backend/optimizer/plan/planagg.c
index 3797b3a58c9..f7ce9b12a92 100644
--- a/src/backend/optimizer/plan/planagg.c
+++ b/src/backend/optimizer/plan/planagg.c
@@ -256,7 +256,10 @@ optimize_minmax_aggregates(PlannerInfo *root, List *tlist,
/*
* We have to replace Aggrefs with Params in equivalence classes too, else
- * ORDER BY or DISTINCT on an optimized aggregate will fail.
+ * ORDER BY or DISTINCT on an optimized aggregate will fail. We don't
+ * need to process child eclass members though, since they aren't of
+ * interest anymore --- and replace_aggs_with_params_mutator isn't able
+ * to handle Aggrefs containing translated child Vars, anyway.
*
* Note: at some point it might become necessary to mutate other data
* structures too, such as the query's sortClause or distinctClause. Right
@@ -264,7 +267,8 @@ optimize_minmax_aggregates(PlannerInfo *root, List *tlist,
*/
mutate_eclass_expressions(root,
replace_aggs_with_params_mutator,
- (void *) root);
+ (void *) root,
+ false);
/*
* Generate the output plan --- basically just a Result
diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h
index 32cd47d9847..dcc79992334 100644
--- a/src/include/optimizer/paths.h
+++ b/src/include/optimizer/paths.h
@@ -121,7 +121,8 @@ extern void add_child_rel_equivalences(PlannerInfo *root,
RelOptInfo *child_rel);
extern void mutate_eclass_expressions(PlannerInfo *root,
Node *(*mutator) (),
- void *context);
+ void *context,
+ bool include_child_exprs);
extern List *find_eclass_clauses_for_index_join(PlannerInfo *root,
RelOptInfo *rel,
Relids outer_relids);
diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out
index 48610066bbd..abedc30a7d6 100644
--- a/src/test/regress/expected/aggregates.out
+++ b/src/test/regress/expected/aggregates.out
@@ -740,6 +740,45 @@ select min(f1), max(f1) from minmaxtest;
11 | 18
(1 row)
+-- DISTINCT doesn't do anything useful here, but it shouldn't fail
+explain (costs off)
+ select distinct min(f1), max(f1) from minmaxtest;
+ QUERY PLAN
+--------------------------------------------------------------------------------------
+ HashAggregate
+ InitPlan 1 (returns $0)
+ -> Limit
+ -> Merge Append
+ Sort Key: public.minmaxtest.f1
+ -> Index Scan using minmaxtesti on minmaxtest
+ Index Cond: (f1 IS NOT NULL)
+ -> Index Scan using minmaxtest1i on minmaxtest1 minmaxtest
+ Index Cond: (f1 IS NOT NULL)
+ -> Index Scan Backward using minmaxtest2i on minmaxtest2 minmaxtest
+ Index Cond: (f1 IS NOT NULL)
+ -> Index Scan using minmaxtest3i on minmaxtest3 minmaxtest
+ Index Cond: (f1 IS NOT NULL)
+ InitPlan 2 (returns $1)
+ -> Limit
+ -> Merge Append
+ Sort Key: public.minmaxtest.f1
+ -> Index Scan Backward using minmaxtesti on minmaxtest
+ Index Cond: (f1 IS NOT NULL)
+ -> Index Scan Backward using minmaxtest1i on minmaxtest1 minmaxtest
+ Index Cond: (f1 IS NOT NULL)
+ -> Index Scan using minmaxtest2i on minmaxtest2 minmaxtest
+ Index Cond: (f1 IS NOT NULL)
+ -> Index Scan Backward using minmaxtest3i on minmaxtest3 minmaxtest
+ Index Cond: (f1 IS NOT NULL)
+ -> Result
+(26 rows)
+
+select distinct min(f1), max(f1) from minmaxtest;
+ min | max
+-----+-----
+ 11 | 18
+(1 row)
+
drop table minmaxtest cascade;
NOTICE: drop cascades to 3 other objects
DETAIL: drop cascades to table minmaxtest1
diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql
index 04ec67b33af..8bc74d28837 100644
--- a/src/test/regress/sql/aggregates.sql
+++ b/src/test/regress/sql/aggregates.sql
@@ -278,6 +278,11 @@ explain (costs off)
select min(f1), max(f1) from minmaxtest;
select min(f1), max(f1) from minmaxtest;
+-- DISTINCT doesn't do anything useful here, but it shouldn't fail
+explain (costs off)
+ select distinct min(f1), max(f1) from minmaxtest;
+select distinct min(f1), max(f1) from minmaxtest;
+
drop table minmaxtest cascade;
--