diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c index 349bd537e5e..70e048c6e9c 100644 --- a/src/backend/optimizer/path/equivclass.c +++ b/src/backend/optimizer/path/equivclass.c @@ -1973,12 +1973,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 @@ -1988,7 +1988,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; @@ -2001,6 +2002,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 0bbca071fb0..cf2ab9ea840 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 b6fb8ee5ce9..ab04d5e6047 100644 --- a/src/include/optimizer/paths.h +++ b/src/include/optimizer/paths.h @@ -124,7 +124,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 *generate_implied_equalities_for_indexcol(PlannerInfo *root, IndexOptInfo *index, int indexcol); diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out index 5678f066cbf..b6d79ca387f 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 Only Scan using minmaxtesti on minmaxtest + Index Cond: (f1 IS NOT NULL) + -> Index Only Scan using minmaxtest1i on minmaxtest1 minmaxtest + Index Cond: (f1 IS NOT NULL) + -> Index Only Scan Backward using minmaxtest2i on minmaxtest2 minmaxtest + Index Cond: (f1 IS NOT NULL) + -> Index Only 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 Only Scan Backward using minmaxtesti on minmaxtest + Index Cond: (f1 IS NOT NULL) + -> Index Only Scan Backward using minmaxtest1i on minmaxtest1 minmaxtest + Index Cond: (f1 IS NOT NULL) + -> Index Only Scan using minmaxtest2i on minmaxtest2 minmaxtest + Index Cond: (f1 IS NOT NULL) + -> Index Only 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 d1c74720d37..e33b36bad11 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; --