Skip to content

Commit eb2f59b

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 ed209db commit eb2f59b

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
@@ -1758,6 +1758,36 @@ select aggfns(distinct a,b,c order by a,c using ~<~,b) filter (where a > 1)
17581758
{"(2,2,bar)","(3,1,baz)"}
17591759
(1 row)
17601760

1761+
-- check handling of bare boolean Var in FILTER
1762+
select max(0) filter (where b1) from bool_test;
1763+
max
1764+
-----
1765+
0
1766+
(1 row)
1767+
1768+
select (select max(0) filter (where b1)) from bool_test;
1769+
max
1770+
-----
1771+
0
1772+
(1 row)
1773+
1774+
-- check for correct detection of nested-aggregate errors in FILTER
1775+
select max(unique1) filter (where sum(ten) > 0) from tenk1;
1776+
ERROR: aggregate functions are not allowed in FILTER
1777+
LINE 1: select max(unique1) filter (where sum(ten) > 0) from tenk1;
1778+
^
1779+
select (select max(unique1) filter (where sum(ten) > 0) from int8_tbl) from tenk1;
1780+
ERROR: aggregate function calls cannot be nested
1781+
LINE 1: select (select max(unique1) filter (where sum(ten) > 0) from...
1782+
^
1783+
select max(unique1) filter (where bool_or(ten > 0)) from tenk1;
1784+
ERROR: aggregate functions are not allowed in FILTER
1785+
LINE 1: select max(unique1) filter (where bool_or(ten > 0)) from ten...
1786+
^
1787+
select (select max(unique1) filter (where bool_or(ten > 0)) from int8_tbl) from tenk1;
1788+
ERROR: aggregate function calls cannot be nested
1789+
LINE 1: select (select max(unique1) filter (where bool_or(ten > 0)) ...
1790+
^
17611791
-- ordered-set aggregates
17621792
select p, percentile_cont(p) within group (order by x::float8)
17631793
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
@@ -645,6 +645,17 @@ select aggfns(distinct a,b,c order by a,c using ~<~,b) filter (where a > 1)
645645
from (values (1,3,'foo'),(0,null,null),(2,2,'bar'),(3,1,'baz')) v(a,b,c),
646646
generate_series(1,2) i;
647647

648+
-- check handling of bare boolean Var in FILTER
649+
select max(0) filter (where b1) from bool_test;
650+
select (select max(0) filter (where b1)) from bool_test;
651+
652+
-- check for correct detection of nested-aggregate errors in FILTER
653+
select max(unique1) filter (where sum(ten) > 0) from tenk1;
654+
select (select max(unique1) filter (where sum(ten) > 0) from int8_tbl) from tenk1;
655+
select max(unique1) filter (where bool_or(ten > 0)) from tenk1;
656+
select (select max(unique1) filter (where bool_or(ten > 0)) from int8_tbl) from tenk1;
657+
658+
648659
-- ordered-set aggregates
649660

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

0 commit comments

Comments
 (0)