diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c index 03edb8026a8..40df317d681 100644 --- a/src/backend/optimizer/prep/prepjointree.c +++ b/src/backend/optimizer/prep/prepjointree.c @@ -83,6 +83,7 @@ static void make_setop_translation_list(Query *query, Index newvarno, List **translated_vars); static bool is_simple_subquery(Query *subquery, RangeTblEntry *rte, JoinExpr *lowest_outer_join, + bool will_need_phvs, bool deletion_ok); static Node *pull_up_simple_values(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte); @@ -691,7 +692,11 @@ pull_up_subqueries_recurse(PlannerInfo *root, Node *jtnode, */ if (rte->rtekind == RTE_SUBQUERY && is_simple_subquery(rte->subquery, rte, - lowest_outer_join, deletion_ok) && + lowest_outer_join, + (lowest_nulling_outer_join != NULL || + containing_appendrel != NULL || + root->parse->groupingSets), + deletion_ok) && (containing_appendrel == NULL || is_safe_append_member(rte->subquery))) return pull_up_simple_subquery(root, jtnode, rte, @@ -955,7 +960,11 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte, * pull_up_subqueries_recurse. */ if (is_simple_subquery(subquery, rte, - lowest_outer_join, deletion_ok) && + lowest_outer_join, + (lowest_nulling_outer_join != NULL || + containing_appendrel != NULL || + parse->groupingSets), + deletion_ok) && (containing_appendrel == NULL || is_safe_append_member(subquery))) { /* good to go */ @@ -1023,6 +1032,14 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte, rvcontext.rv_cache = palloc0((list_length(subquery->targetList) + 1) * sizeof(Node *)); + /* + * The next few stanzas identify cases where we might need to insert + * PlaceHolderVars. These same conditions must be checked by callers of + * is_simple_subquery(): pass will_need_phvs = true if anything below + * would set rvcontext.need_phvs = true. Else we can find ourselves + * trying to generate a PHV with empty phrels. + */ + /* * If we are under an outer join then non-nullable items and lateral * references may have to be turned into PlaceHolderVars. @@ -1433,11 +1450,14 @@ make_setop_translation_list(Query *query, Index newvarno, * (Note subquery is not necessarily equal to rte->subquery; it could be a * processed copy of that.) * lowest_outer_join is the lowest outer join above the subquery, or NULL. + * will_need_phvs is true if we might need to wrap the subquery outputs + * with PlaceHolderVars; see comments within. * deletion_ok is TRUE if it'd be okay to delete the subquery entirely. */ static bool is_simple_subquery(Query *subquery, RangeTblEntry *rte, JoinExpr *lowest_outer_join, + bool will_need_phvs, bool deletion_ok) { /* @@ -1489,7 +1509,7 @@ is_simple_subquery(Query *subquery, RangeTblEntry *rte, /* * Don't pull up a subquery with an empty jointree, unless it has no quals - * and deletion_ok is TRUE and we're not underneath an outer join. + * and deletion_ok is true and will_need_phvs is false. * * query_planner() will correctly generate a Result plan for a jointree * that's totally empty, but we can't cope with an empty FromExpr @@ -1504,9 +1524,8 @@ is_simple_subquery(Query *subquery, RangeTblEntry *rte, * But even in a safe context, we must keep the subquery if it has any * quals, because it's unclear where to put them in the upper query. * - * Also, we must forbid pullup if such a subquery is underneath an outer - * join, because then we might need to wrap its output columns with - * PlaceHolderVars, and the PHVs would then have empty relid sets meaning + * Also, we must forbid pullup if the subquery outputs would need + * PlaceHolderVar wrappers; the PHVs would then have empty relids meaning * we couldn't tell where to evaluate them. (This test is separate from * the deletion_ok flag for possible future expansion: deletion_ok tells * whether the immediate parent site in the jointree could cope, not @@ -1514,6 +1533,9 @@ is_simple_subquery(Query *subquery, RangeTblEntry *rte, * fixed by letting the PHVs use the relids of the parent jointree item, * but that complication is for another day.) * + * The caller must pass will_need_phvs = true under the same conditions + * that would cause pull_up_simple_subquery to set need_phvs = true. + * * Note that deletion of a subquery is also dependent on the check below * that its targetlist contains no set-returning functions. Deletion from * a FROM list or inner JOIN is okay only if the subquery must return @@ -1522,7 +1544,7 @@ is_simple_subquery(Query *subquery, RangeTblEntry *rte, if (subquery->jointree->fromlist == NIL && (subquery->jointree->quals != NULL || !deletion_ok || - lowest_outer_join != NULL)) + will_need_phvs)) return false; /* diff --git a/src/test/regress/expected/groupingsets.out b/src/test/regress/expected/groupingsets.out index d052f365c70..036a1bce5b9 100644 --- a/src/test/regress/expected/groupingsets.out +++ b/src/test/regress/expected/groupingsets.out @@ -1677,27 +1677,25 @@ select v||'a', case when grouping(v||'a') = 1 then 1 else 0 end, count(*) -- test handling of outer GroupingFunc within subqueries explain (costs off) select (select grouping(v1)) from (values ((select 1))) v(v1) group by cube(v1); - QUERY PLAN ---------------------------- - GroupAggregate - Group Key: $2 + QUERY PLAN +--------------------------------- + MixedAggregate + Hash Key: $1 Group Key: () - InitPlan 1 (returns $1) - -> Result - InitPlan 3 (returns $2) - -> Result - InitPlan 4 (returns $3) - -> Result -> Result - SubPlan 2 + InitPlan 2 (returns $1) + -> Result + InitPlan 3 (returns $2) + -> Result + SubPlan 1 -> Result -(12 rows) +(10 rows) select (select grouping(v1)) from (values ((select 1))) v(v1) group by cube(v1); grouping ---------- - 0 1 + 0 (2 rows) explain (costs off) @@ -1723,4 +1721,37 @@ select (select grouping(v1)) from (values ((select 1))) v(v1) group by v1; 0 (1 row) +-- check that we don't pull up when a phrel-less PHV would result +explain (verbose, costs off) +select ss.f from int4_tbl as i4 cross join lateral (select i4.f1 as f) as ss +group by cube(ss.f) order by 1; + QUERY PLAN +-------------------------------------------------- + Sort + Output: (i4.f1) + Sort Key: (i4.f1) + -> MixedAggregate + Output: (i4.f1) + Hash Key: (i4.f1) + Group Key: () + -> Nested Loop + Output: (i4.f1) + -> Seq Scan on public.int4_tbl i4 + Output: i4.f1 + -> Result + Output: i4.f1 +(13 rows) + +select ss.f from int4_tbl as i4 cross join lateral (select i4.f1 as f) as ss +group by cube(ss.f) order by 1; + f +------------- + -2147483647 + -123456 + 0 + 123456 + 2147483647 + +(6 rows) + -- end diff --git a/src/test/regress/sql/groupingsets.sql b/src/test/regress/sql/groupingsets.sql index dd41687faa6..b056a30873e 100644 --- a/src/test/regress/sql/groupingsets.sql +++ b/src/test/regress/sql/groupingsets.sql @@ -477,4 +477,11 @@ explain (costs off) select (select grouping(v1)) from (values ((select 1))) v(v1) group by v1; select (select grouping(v1)) from (values ((select 1))) v(v1) group by v1; +-- check that we don't pull up when a phrel-less PHV would result +explain (verbose, costs off) +select ss.f from int4_tbl as i4 cross join lateral (select i4.f1 as f) as ss +group by cube(ss.f) order by 1; +select ss.f from int4_tbl as i4 cross join lateral (select i4.f1 as f) as ss +group by cube(ss.f) order by 1; + -- end