From b0129b7e1c24d67b0609139f5416c31e03545b25 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Thu, 12 Oct 2017 14:56:21 +0300 Subject: [PATCH 1/1] Fix subquery pullup into a query containing GROUPING SETS and HAVING. --- src/backend/optimizer/prep/prepjointree.c | 44 +++++++++++++++++++++++++------ src/test/regress/expected/subselect.out | 14 ++++++++++ src/test/regress/sql/subselect.sql | 10 +++++++ 3 files changed, 60 insertions(+), 8 deletions(-) diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c index f3bb73a664..7fa65696c3 100644 --- a/src/backend/optimizer/prep/prepjointree.c +++ b/src/backend/optimizer/prep/prepjointree.c @@ -1003,11 +1003,8 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte, /* * The subquery's targetlist items are now in the appropriate form to - * insert into the top query, but if we are under an outer join then - * non-nullable items and lateral references may have to be turned into - * PlaceHolderVars. If we are dealing with an appendrel member then - * anything that's not a simple Var has to be turned into a - * PlaceHolderVar. Set up required context data for pullup_replace_vars. + * insert into the top query. Set up required context data for + * pullup_replace_vars. */ rvcontext.root = root; rvcontext.targetlist = subquery->targetList; @@ -1019,14 +1016,45 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte, rvcontext.relids = NULL; rvcontext.outer_hasSubLinks = &parse->hasSubLinks; rvcontext.varno = varno; - rvcontext.need_phvs = (lowest_nulling_outer_join != NULL || - containing_appendrel != NULL); - rvcontext.wrap_non_vars = (containing_appendrel != NULL); + /* these flags will be set below, if needed */ + rvcontext.need_phvs = false; + rvcontext.wrap_non_vars = false; /* initialize cache array with indexes 0 .. length(tlist) */ rvcontext.rv_cache = palloc0((list_length(subquery->targetList) + 1) * sizeof(Node *)); /* + * If we are under an outer join then non-nullable items and lateral + * references may have to be turned into PlaceHolderVars. + */ + if (lowest_nulling_outer_join != NULL) + rvcontext.need_phvs = true; + + /* + * If we are dealing with an appendrel member then anything that's not + * a simple Var has to be turned into a PlaceHolderVar. + */ + if (containing_appendrel != NULL) + { + rvcontext.need_phvs = true; + rvcontext.wrap_non_vars = true; + } + + /* + * If the parent query uses GROUPING SETS and HAVING, we need + * PlaceHolderVars for anything that's not a simple Var. Because + * for the "rolled up" columns, the HAVING should consider them as + * NULL. XXX: We would really only need to use PHVs in the havingQual + * itself, but grouping sets planning gets confused if there are + * references to both the underlying expression, and placeholders. + */ + if (parse->groupingSets && parse->havingQual) + { + rvcontext.need_phvs = true; + rvcontext.wrap_non_vars = true; + } + + /* * Replace all of the top query's references to the subquery's outputs * with copies of the adjusted subtlist items, being careful not to * replace any of the jointree structure. (This'd be a lot cleaner if we diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out index f3ebad5857..c8be7854be 100644 --- a/src/test/regress/expected/subselect.out +++ b/src/test/regress/expected/subselect.out @@ -1093,3 +1093,17 @@ select * from (select pk,c2 from sq_limit order by c1,pk) as x limit 3; drop function explain_sq_limit(); drop table sq_limit; +-- +-- Check that when a subquery is pulled up into a query with grouping sets, +-- HAVING refers to the after-grouping, possibly NULLed value. I.e. the +-- below HAVING x = 'foo' condition is not always true, even though x is +-- a constant 'foo' in the subquery. +-- +select four, x + from (select four, ten, 'foo' as x from tenk1 ) as t + group by grouping sets(four, x) having x = 'foo' order by four; + four | x +------+----- + | foo +(1 row) + diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql index 5ac8badabe..aac3286ea1 100644 --- a/src/test/regress/sql/subselect.sql +++ b/src/test/regress/sql/subselect.sql @@ -581,3 +581,13 @@ select * from (select pk,c2 from sq_limit order by c1,pk) as x limit 3; drop function explain_sq_limit(); drop table sq_limit; + +-- +-- Check that when a subquery is pulled up into a query with grouping sets, +-- HAVING refers to the after-grouping, possibly NULLed value. I.e. the +-- below HAVING x = 'foo' condition is not always true, even though x is +-- a constant 'foo' in the subquery. +-- +select four, x + from (select four, ten, 'foo' as x from tenk1 ) as t + group by grouping sets(four, x) having x = 'foo' order by four; -- 2.11.0