diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c index 3254c83cc6c..b8340557b34 100644 --- a/src/backend/parser/parse_agg.c +++ b/src/backend/parser/parse_agg.c @@ -38,6 +38,8 @@ typedef struct ParseState *pstate; int min_varlevel; int min_agglevel; + int min_ctelevel; + RangeTblEntry *min_cte; int sublevels_up; } check_agg_arguments_context; @@ -58,7 +60,8 @@ typedef struct static int check_agg_arguments(ParseState *pstate, List *directargs, List *args, - Expr *filter); + Expr *filter, + int agglocation); static bool check_agg_arguments_walker(Node *node, check_agg_arguments_context *context); static Node *substitute_grouped_columns(Node *node, ParseState *pstate, Query *qry, @@ -339,7 +342,8 @@ check_agglevels_and_constraints(ParseState *pstate, Node *expr) min_varlevel = check_agg_arguments(pstate, directargs, args, - filter); + filter, + location); *p_levelsup = min_varlevel; @@ -641,7 +645,8 @@ static int check_agg_arguments(ParseState *pstate, List *directargs, List *args, - Expr *filter) + Expr *filter, + int agglocation) { int agglevel; check_agg_arguments_context context; @@ -649,6 +654,8 @@ check_agg_arguments(ParseState *pstate, context.pstate = pstate; context.min_varlevel = -1; /* signifies nothing found yet */ context.min_agglevel = -1; + context.min_ctelevel = -1; + context.min_cte = NULL; context.sublevels_up = 0; (void) check_agg_arguments_walker((Node *) args, &context); @@ -686,6 +693,20 @@ check_agg_arguments(ParseState *pstate, parser_errposition(pstate, aggloc))); } + /* + * If there's a non-local CTE that's below the aggregate's semantic level, + * complain. It's not quite clear what we should do to fix up such a case + * (treating the CTE reference like a Var seems wrong), and it's also + * unclear whether there is a real-world use for such cases. + */ + if (context.min_ctelevel >= 0 && context.min_ctelevel < agglevel) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("outer-level aggregate cannot use a nested CTE"), + errdetail("CTE \"%s\" is below the aggregate's semantic level.", + context.min_cte->eref->aliasname), + parser_errposition(pstate, agglocation))); + /* * Now check for vars/aggs in the direct arguments, and throw error if * needed. Note that we allow a Var of the agg's semantic level, but not @@ -699,6 +720,7 @@ check_agg_arguments(ParseState *pstate, { context.min_varlevel = -1; context.min_agglevel = -1; + context.min_ctelevel = -1; (void) check_agg_arguments_walker((Node *) directargs, &context); if (context.min_varlevel >= 0 && context.min_varlevel < agglevel) ereport(ERROR, @@ -714,6 +736,13 @@ check_agg_arguments(ParseState *pstate, parser_errposition(pstate, locate_agg_of_level((Node *) directargs, context.min_agglevel)))); + if (context.min_ctelevel >= 0 && context.min_ctelevel < agglevel) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("outer-level aggregate cannot use a nested CTE"), + errdetail("CTE \"%s\" is below the aggregate's semantic level.", + context.min_cte->eref->aliasname), + parser_errposition(pstate, agglocation))); } return agglevel; } @@ -794,11 +823,6 @@ check_agg_arguments_walker(Node *node, if (IsA(node, RangeTblEntry)) { - /* - * CTE references act similarly to Vars of the CTE's level. Without - * this we might conclude that the Agg can be evaluated above the CTE, - * leading to trouble. - */ RangeTblEntry *rte = (RangeTblEntry *) node; if (rte->rtekind == RTE_CTE) @@ -810,9 +834,12 @@ check_agg_arguments_walker(Node *node, /* ignore local CTEs of subqueries */ if (ctelevelsup >= 0) { - if (context->min_varlevel < 0 || - context->min_varlevel > ctelevelsup) - context->min_varlevel = ctelevelsup; + if (context->min_ctelevel < 0 || + context->min_ctelevel > ctelevelsup) + { + context->min_ctelevel = ctelevelsup; + context->min_cte = rte; + } } } return false; /* allow range_table_walker to continue */ diff --git a/src/test/regress/expected/with.out b/src/test/regress/expected/with.out index 86fdb85c6c5..f4caedf272f 100644 --- a/src/test/regress/expected/with.out +++ b/src/test/regress/expected/with.out @@ -2300,36 +2300,44 @@ from int4_tbl; -- -- test for bug #19055: interaction of WITH with aggregates -- --- The reference to cte1 must determine the aggregate's level, --- even though it contains no Vars referencing cte1 -explain (verbose, costs off) +-- For now, we just throw an error if there's a use of a CTE below the +-- semantic level that the SQL standard assigns to the aggregate. +-- It's not entirely clear what we could do instead that doesn't risk +-- breaking more things than it fixes. select f1, (with cte1(x,y) as (select 1,2) select count((select i4.f1 from cte1))) as ss from int4_tbl i4; - QUERY PLAN -------------------------------------------------- - Seq Scan on public.int4_tbl i4 - Output: i4.f1, (SubPlan expr_1) - SubPlan expr_1 +ERROR: outer-level aggregate cannot use a nested CTE +LINE 2: select count((select i4.f1 from cte1))) as ss + ^ +DETAIL: CTE "cte1" is below the aggregate's semantic level. +-- +-- test for bug #19106: interaction of WITH with aggregates +-- +-- the initial fix for #19055 was too aggressive and broke this case +explain (verbose, costs off) +with a as ( select id from (values (1), (2)) as v(id) ), + b as ( select max((select sum(id) from a)) as agg ) +select agg from b; + QUERY PLAN +-------------------------------------------- + Aggregate + Output: max((InitPlan expr_1).col1) + InitPlan expr_1 -> Aggregate - Output: count((InitPlan expr_2).col1) - InitPlan expr_2 - -> Result - Output: i4.f1 - -> Result -(9 rows) + Output: sum("*VALUES*".column1) + -> Values Scan on "*VALUES*" + Output: "*VALUES*".column1 + -> Result +(8 rows) -select f1, (with cte1(x,y) as (select 1,2) - select count((select i4.f1 from cte1))) as ss -from int4_tbl i4; - f1 | ss --------------+---- - 0 | 1 - 123456 | 1 - -123456 | 1 - 2147483647 | 1 - -2147483647 | 1 -(5 rows) +with a as ( select id from (values (1), (2)) as v(id) ), + b as ( select max((select sum(id) from a)) as agg ) +select agg from b; + agg +----- + 3 +(1 row) -- -- test for nested-recursive-WITH bug diff --git a/src/test/regress/sql/with.sql b/src/test/regress/sql/with.sql index d88d5abb91a..cd25a5e7154 100644 --- a/src/test/regress/sql/with.sql +++ b/src/test/regress/sql/with.sql @@ -1100,16 +1100,26 @@ from int4_tbl; -- -- test for bug #19055: interaction of WITH with aggregates -- --- The reference to cte1 must determine the aggregate's level, --- even though it contains no Vars referencing cte1 -explain (verbose, costs off) +-- For now, we just throw an error if there's a use of a CTE below the +-- semantic level that the SQL standard assigns to the aggregate. +-- It's not entirely clear what we could do instead that doesn't risk +-- breaking more things than it fixes. select f1, (with cte1(x,y) as (select 1,2) select count((select i4.f1 from cte1))) as ss from int4_tbl i4; -select f1, (with cte1(x,y) as (select 1,2) - select count((select i4.f1 from cte1))) as ss -from int4_tbl i4; +-- +-- test for bug #19106: interaction of WITH with aggregates +-- +-- the initial fix for #19055 was too aggressive and broke this case +explain (verbose, costs off) +with a as ( select id from (values (1), (2)) as v(id) ), + b as ( select max((select sum(id) from a)) as agg ) +select agg from b; + +with a as ( select id from (values (1), (2)) as v(id) ), + b as ( select max((select sum(id) from a)) as agg ) +select agg from b; -- -- test for nested-recursive-WITH bug