diff options
Diffstat (limited to 'src')
| -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 58553df3140..632295197a9 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 55a5ed7b4c6..658a4abc315 100644 --- a/src/backend/optimizer/plan/planagg.c +++ b/src/backend/optimizer/plan/planagg.c @@ -257,7 +257,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 @@ -265,7 +268,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 165856de0bc..c7d59d28da3 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 7286f1aa446..4c5b98a6126 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: minmaxtest.f1 + -> Index Only Scan using minmaxtesti on minmaxtest + Index Cond: (f1 IS NOT NULL) + -> Index Only Scan using minmaxtest1i on minmaxtest1 + Index Cond: (f1 IS NOT NULL) + -> Index Only Scan Backward using minmaxtest2i on minmaxtest2 + Index Cond: (f1 IS NOT NULL) + -> Index Only Scan using minmaxtest3i on minmaxtest3 + Index Cond: (f1 IS NOT NULL) + InitPlan 2 (returns $1) + -> Limit + -> Merge Append + Sort Key: minmaxtest_1.f1 + -> Index Only Scan Backward using minmaxtesti on minmaxtest minmaxtest_1 + Index Cond: (f1 IS NOT NULL) + -> Index Only Scan Backward using minmaxtest1i on minmaxtest1 minmaxtest1_1 + Index Cond: (f1 IS NOT NULL) + -> Index Only Scan using minmaxtest2i on minmaxtest2 minmaxtest2_1 + Index Cond: (f1 IS NOT NULL) + -> Index Only Scan Backward using minmaxtest3i on minmaxtest3 minmaxtest3_1 + 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 53a2183b3dd..38d4757df3f 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; -- check for correct detection of nested-aggregate errors |