diff --git a/src/backend/optimizer/plan/planagg.c b/src/backend/optimizer/plan/planagg.c index f7ce9b12a92..88a6c99ea50 100644 --- a/src/backend/optimizer/plan/planagg.c +++ b/src/backend/optimizer/plan/planagg.c @@ -37,6 +37,7 @@ #include "optimizer/paths.h" #include "optimizer/planmain.h" #include "optimizer/subselect.h" +#include "optimizer/tlist.h" #include "parser/parsetree.h" #include "parser/parse_clause.h" #include "utils/lsyscache.h" @@ -521,7 +522,27 @@ make_agg_subplan(PlannerInfo *root, MinMaxAggInfo *mminfo) */ plan = create_plan(subroot, mminfo->path); - plan->targetlist = subparse->targetList; + /* + * If the top-level plan node is one that cannot do expression evaluation + * and its existing target list isn't already what we need, we must insert + * a Result node to project the desired tlist. + */ + if (!is_projection_capable_plan(plan) && + !tlist_same_exprs(subparse->targetList, plan->targetlist)) + { + plan = (Plan *) make_result(subroot, + subparse->targetList, + NULL, + plan); + } + else + { + /* + * Otherwise, just replace the subplan's flat tlist with the desired + * tlist. + */ + plan->targetlist = subparse->targetList; + } plan = (Plan *) make_limit(plan, subparse->limitOffset, diff --git a/src/backend/optimizer/util/tlist.c b/src/backend/optimizer/util/tlist.c index b8149f7b461..c98c767f310 100644 --- a/src/backend/optimizer/util/tlist.c +++ b/src/backend/optimizer/util/tlist.c @@ -188,6 +188,43 @@ get_tlist_exprs(List *tlist, bool includeJunk) } +/* + * tlist_same_exprs + * Check whether two target lists contain the same expressions + * + * Note: this function is used to decide whether it's safe to jam a new tlist + * into a non-projection-capable plan node. Obviously we can't do that unless + * the node's tlist shows it already returns the column values we want. + * However, we can ignore the TargetEntry attributes resname, ressortgroupref, + * resorigtbl, resorigcol, and resjunk, because those are only labelings that + * don't affect the row values computed by the node. (Moreover, if we didn't + * ignore them, we'd frequently fail to make the desired optimization, since + * the planner tends to not bother to make resname etc. valid in intermediate + * plan nodes.) Note that on success, the caller must still jam the desired + * tlist into the plan node, else it won't have the desired labeling fields. + */ +bool +tlist_same_exprs(List *tlist1, List *tlist2) +{ + ListCell *lc1, + *lc2; + + if (list_length(tlist1) != list_length(tlist2)) + return false; /* not same length, so can't match */ + + forboth(lc1, tlist1, lc2, tlist2) + { + TargetEntry *tle1 = (TargetEntry *) lfirst(lc1); + TargetEntry *tle2 = (TargetEntry *) lfirst(lc2); + + if (!equal(tle1->expr, tle2->expr)) + return false; + } + + return true; +} + + /* * Does tlist have same output datatypes as listed in colTypes? * diff --git a/src/include/optimizer/tlist.h b/src/include/optimizer/tlist.h index 0edc8835fbd..d172549a332 100644 --- a/src/include/optimizer/tlist.h +++ b/src/include/optimizer/tlist.h @@ -26,6 +26,9 @@ extern List *flatten_tlist(List *tlist, PVCAggregateBehavior aggbehavior, extern List *add_to_flat_tlist(List *tlist, List *exprs); extern List *get_tlist_exprs(List *tlist, bool includeJunk); + +extern bool tlist_same_exprs(List *tlist1, List *tlist2); + extern bool tlist_same_datatypes(List *tlist, List *colTypes, bool junkOK); extern bool tlist_same_collations(List *tlist, List *colCollations, bool junkOK); diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out index 36d90f5bba1..c79b8f950f1 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -1203,6 +1203,28 @@ select * from matest0 order by 1-id; 1 | Test 1 (6 rows) +explain (verbose, costs off) select min(1-id) from matest0; + QUERY PLAN +------------------------------------------------ + Aggregate + Output: min((1 - public.matest0.id)) + -> Append + -> Seq Scan on public.matest0 + Output: public.matest0.id + -> Seq Scan on public.matest1 matest0 + Output: public.matest0.id + -> Seq Scan on public.matest2 matest0 + Output: public.matest0.id + -> Seq Scan on public.matest3 matest0 + Output: public.matest0.id +(11 rows) + +select min(1-id) from matest0; + min +----- + -5 +(1 row) + reset enable_indexscan; set enable_seqscan = off; -- plan with fewest seqscans should be merge explain (verbose, costs off) select * from matest0 order by 1-id; @@ -1236,6 +1258,41 @@ select * from matest0 order by 1-id; 1 | Test 1 (6 rows) +explain (verbose, costs off) select min(1-id) from matest0; + QUERY PLAN +-------------------------------------------------------------------------------------- + Result + Output: $0 + InitPlan 1 (returns $0) + -> Limit + Output: ((1 - public.matest0.id)) + -> Result + Output: ((1 - public.matest0.id)) + -> Merge Append + Sort Key: ((1 - public.matest0.id)) + -> Index Scan using matest0i on public.matest0 + Output: public.matest0.id, (1 - public.matest0.id) + Index Cond: ((1 - public.matest0.id) IS NOT NULL) + -> Index Scan using matest1i on public.matest1 matest0 + Output: public.matest0.id, (1 - public.matest0.id) + Index Cond: ((1 - public.matest0.id) IS NOT NULL) + -> Sort + Output: public.matest0.id, ((1 - public.matest0.id)) + Sort Key: ((1 - public.matest0.id)) + -> Seq Scan on public.matest2 matest0 + Output: public.matest0.id, (1 - public.matest0.id) + Filter: ((1 - public.matest0.id) IS NOT NULL) + -> Index Scan using matest3i on public.matest3 matest0 + Output: public.matest0.id, (1 - public.matest0.id) + Index Cond: ((1 - public.matest0.id) IS NOT NULL) +(24 rows) + +select min(1-id) from matest0; + min +----- + -5 +(1 row) + reset enable_seqscan; drop table matest0 cascade; NOTICE: drop cascades to 3 other objects diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql index 6c7d809fb60..3b3d71f040c 100644 --- a/src/test/regress/sql/inherit.sql +++ b/src/test/regress/sql/inherit.sql @@ -398,11 +398,15 @@ insert into matest3 (name) values ('Test 6'); set enable_indexscan = off; -- force use of seqscan/sort, so no merge explain (verbose, costs off) select * from matest0 order by 1-id; select * from matest0 order by 1-id; +explain (verbose, costs off) select min(1-id) from matest0; +select min(1-id) from matest0; reset enable_indexscan; set enable_seqscan = off; -- plan with fewest seqscans should be merge explain (verbose, costs off) select * from matest0 order by 1-id; select * from matest0 order by 1-id; +explain (verbose, costs off) select min(1-id) from matest0; +select min(1-id) from matest0; reset enable_seqscan; drop table matest0 cascade;