diff options
| -rw-r--r-- | src/backend/optimizer/path/equivclass.c | 10 | ||||
| -rw-r--r-- | src/backend/optimizer/plan/planagg.c | 8 | ||||
| -rw-r--r-- | src/include/optimizer/paths.h | 3 | ||||
| -rw-r--r-- | src/test/regress/expected/aggregates.out | 39 | ||||
| -rw-r--r-- | src/test/regress/sql/aggregates.sql | 5 |
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; -- |