Skip to content

Commit ecd4dd9

Browse files
tglsfdctglsfdc
committed
Fix check_agg_arguments' examination of aggregate FILTER clauses.
Recursion into the FILTER clause was mis-implemented, such that a relevant Var or Aggref at the very top of the FILTER clause would be ignored. (Of course, that'd have to be a plain boolean Var or boolean-returning aggregate.) The consequence would be mis-identification of the correct semantic level of the aggregate, which could lead to not-per-spec query behavior. If the FILTER expression is an aggregate, this could also lead to failure to issue an expected "aggregate function calls cannot be nested" error, which would likely result in a core dump later on, since the planner and executor aren't expecting such cases to appear. The root cause is that commit b560ec1 blindly copied some code that assumed it's recursing into a List, and thus didn't examine the top-level node. To forestall questions about why this call doesn't look like the others, as well as possible future copy-and-paste mistakes, let's change all three check_agg_arguments_walker calls in check_agg_arguments, even though only the one for the filter clause is really broken. Per bug #17152 from Zhiyong Wu. This has been wrong since we implemented FILTER, so back-patch to all supported versions. (Testing suggests that pre-v11 branches manage to avoid crashing in the bad-Aggref case, thanks to "redundant" checks in ExecInitAgg. But I'm not sure how thorough that protection is, and anyway the wrong-behavior issue remains, so fix 9.6 and 10 too.) Discussion: https://postgr.es/m/17152-c7f906cc1a88e61b@postgresql.org
1 parent 7b01246 commit ecd4dd9

File tree

3 files changed

+44
-10
lines changed

3 files changed

+44
-10
lines changed

‎src/backend/parser/parse_agg.c

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -616,13 +616,8 @@ check_agg_arguments(ParseState *pstate,
616616
context.min_agglevel = -1;
617617
context.sublevels_up = 0;
618618

619-
(void) expression_tree_walker((Node *) args,
620-
check_agg_arguments_walker,
621-
(void *) &context);
622-
623-
(void) expression_tree_walker((Node *) filter,
624-
check_agg_arguments_walker,
625-
(void *) &context);
619+
(void) check_agg_arguments_walker((Node *) args, &context);
620+
(void) check_agg_arguments_walker((Node *) filter, &context);
626621

627622
/*
628623
* If we found no vars nor aggs at all, it's a level-zero aggregate;
@@ -669,9 +664,7 @@ check_agg_arguments(ParseState *pstate,
669664
{
670665
context.min_varlevel = -1;
671666
context.min_agglevel = -1;
672-
(void) expression_tree_walker((Node *) directargs,
673-
check_agg_arguments_walker,
674-
(void *) &context);
667+
(void) check_agg_arguments_walker((Node *) directargs, &context);
675668
if (context.min_varlevel >= 0 && context.min_varlevel < agglevel)
676669
ereport(ERROR,
677670
(errcode(ERRCODE_GROUPING_ERROR),

‎src/test/regress/expected/aggregates.out

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1784,6 +1784,36 @@ select aggfns(distinct a,b,c order by a,c using ~<~,b) filter (where a > 1)
17841784
{"(2,2,bar)","(3,1,baz)"}
17851785
(1 row)
17861786

1787+
-- check handling of bare boolean Var in FILTER
1788+
select max(0) filter (where b1) from bool_test;
1789+
max
1790+
-----
1791+
0
1792+
(1 row)
1793+
1794+
select (select max(0) filter (where b1)) from bool_test;
1795+
max
1796+
-----
1797+
0
1798+
(1 row)
1799+
1800+
-- check for correct detection of nested-aggregate errors in FILTER
1801+
select max(unique1) filter (where sum(ten) > 0) from tenk1;
1802+
ERROR: aggregate functions are not allowed in FILTER
1803+
LINE 1: select max(unique1) filter (where sum(ten) > 0) from tenk1;
1804+
^
1805+
select (select max(unique1) filter (where sum(ten) > 0) from int8_tbl) from tenk1;
1806+
ERROR: aggregate function calls cannot be nested
1807+
LINE 1: select (select max(unique1) filter (where sum(ten) > 0) from...
1808+
^
1809+
select max(unique1) filter (where bool_or(ten > 0)) from tenk1;
1810+
ERROR: aggregate functions are not allowed in FILTER
1811+
LINE 1: select max(unique1) filter (where bool_or(ten > 0)) from ten...
1812+
^
1813+
select (select max(unique1) filter (where bool_or(ten > 0)) from int8_tbl) from tenk1;
1814+
ERROR: aggregate function calls cannot be nested
1815+
LINE 1: select (select max(unique1) filter (where bool_or(ten > 0)) ...
1816+
^
17871817
-- ordered-set aggregates
17881818
select p, percentile_cont(p) within group (order by x::float8)
17891819
from generate_series(1,5) x,

‎src/test/regress/sql/aggregates.sql

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -660,6 +660,17 @@ select aggfns(distinct a,b,c order by a,c using ~<~,b) filter (where a > 1)
660660
from (values (1,3,'foo'),(0,null,null),(2,2,'bar'),(3,1,'baz')) v(a,b,c),
661661
generate_series(1,2) i;
662662

663+
-- check handling of bare boolean Var in FILTER
664+
select max(0) filter (where b1) from bool_test;
665+
select (select max(0) filter (where b1)) from bool_test;
666+
667+
-- check for correct detection of nested-aggregate errors in FILTER
668+
select max(unique1) filter (where sum(ten) > 0) from tenk1;
669+
select (select max(unique1) filter (where sum(ten) > 0) from int8_tbl) from tenk1;
670+
select max(unique1) filter (where bool_or(ten > 0)) from tenk1;
671+
select (select max(unique1) filter (where bool_or(ten > 0)) from int8_tbl) from tenk1;
672+
673+
663674
-- ordered-set aggregates
664675

665676
select p, percentile_cont(p) within group (order by x::float8)

0 commit comments

Comments
 (0)