From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
Cc: xinwen@stu.scu.edu.cn
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17835: Two assertions failed in nodeAgg.c and execExprInterp.c with the same SQL
Date: Mon, 13 Mar 2023 12:13:25 -0400
Message-ID: <1885605.1678724005@sss.pgh.pa.us> (raw)
In-Reply-To: <1772189.1678721174@sss.pgh.pa.us>
References: <17835-4f29f3098b2d0ba4@postgresql.org>
<1772189.1678721174@sss.pgh.pa.us>
I wrote:
> v10 detected this in ExecInitAgg, while later versions are trying to
> do it in the parser, but evidently there's some gap there ...
Looking closer, 69c3936a1 didn't really break this, it just exposed
that it was already broken. check_agg_arguments() has never gotten
this case right, but it was back-stopped by the check in ExecInitAgg,
which 69c3936a1 removed because it was no longer easy to check there.
The problem is simple: check_agg_arguments supposes that an inner
(lower level) aggregate cannot contain aggregates of outer levels,
which is just wrong on its face. So we can't skip descending into
an aggregate's arguments. Somebody (probably Gierth) recognized
that this was true for grouping functions, but didn't see that the
adjacent code was buggy.
regards, tom lane
Attachments:fix-nested-agg-detection.patch (text/x-diff, 1.9K) downloaddiff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.cindex 4fbf80c271..85cd47b7ae 100644--- a/src/backend/parser/parse_agg.c+++ b/src/backend/parser/parse_agg.c@@ -736,8 +736,7 @@ check_agg_arguments_walker(Node *node,
context->min_agglevel > agglevelsup)
context->min_agglevel = agglevelsup;
}
- /* no need to examine args of the inner aggregate */- return false;+ /* Continue and descend into subtree */
}
if (IsA(node, GroupingFunc))
{
diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.outindex e074cb71bf..f0517f95b6 100644--- a/src/test/regress/expected/aggregates.out+++ b/src/test/regress/expected/aggregates.out@@ -1282,6 +1282,12 @@ select (select max(min(unique1)) from int8_tbl) from tenk1;
ERROR: aggregate function calls cannot be nested
LINE 1: select (select max(min(unique1)) from int8_tbl) from tenk1;
^
+select avg((select avg(a1.col1 order by (select avg(a2.col2) from tenk1 a3))+ from tenk1 a1(col1)))+from tenk1 a2(col2);+ERROR: aggregate function calls cannot be nested+LINE 1: select avg((select avg(a1.col1 order by (select avg(a2.col2)...+ ^
--
-- Test removal of redundant GROUP BY columns
--
diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sqlindex 616ef38c25..1783d19bd5 100644--- a/src/test/regress/sql/aggregates.sql+++ b/src/test/regress/sql/aggregates.sql@@ -439,6 +439,9 @@ drop table minmaxtest cascade;
-- check for correct detection of nested-aggregate errors
select max(min(unique1)) from tenk1;
select (select max(min(unique1)) from int8_tbl) from tenk1;
+select avg((select avg(a1.col1 order by (select avg(a2.col2) from tenk1 a3))+ from tenk1 a1(col1)))+from tenk1 a2(col2);
--
-- Test removal of redundant GROUP BY columns
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, andres@anarazel.de, xinwen@stu.scu.edu.cn, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17835: Two assertions failed in nodeAgg.c and execExprInterp.c with the same SQL
In-Reply-To: <1885605.1678724005@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox