Skip to content

Commit 2313dda

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 76987ba commit 2313dda

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
@@ -627,13 +627,8 @@ check_agg_arguments(ParseState *pstate,
627627
context.min_agglevel = -1;
628628
context.sublevels_up = 0;
629629

630-
(void) expression_tree_walker((Node *) args,
631-
check_agg_arguments_walker,
632-
(void *) &context);
633-
634-
(void) expression_tree_walker((Node *) filter,
635-
check_agg_arguments_walker,
636-
(void *) &context);
630+
(void) check_agg_arguments_walker((Node *) args, &context);
631+
(void) check_agg_arguments_walker((Node *) filter, &context);
637632

638633
/*
639634
* If we found no vars nor aggs at all, it's a level-zero aggregate;
@@ -680,9 +675,7 @@ check_agg_arguments(ParseState *pstate,
680675
{
681676
context.min_varlevel = -1;
682677
context.min_agglevel = -1;
683-
(void) expression_tree_walker((Node *) directargs,
684-
check_agg_arguments_walker,
685-
(void *) &context);
678+
(void) check_agg_arguments_walker((Node *) directargs, &context);
686679
if (context.min_varlevel >= 0 && context.min_varlevel < agglevel)
687680
ereport(ERROR,
688681
(errcode(ERRCODE_GROUPING_ERROR),

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

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

1848+
-- check handling of bare boolean Var in FILTER
1849+
select max(0) filter (where b1) from bool_test;
1850+
max
1851+
-----
1852+
0
1853+
(1 row)
1854+
1855+
select (select max(0) filter (where b1)) from bool_test;
1856+
max
1857+
-----
1858+
0
1859+
(1 row)
1860+
1861+
-- check for correct detection of nested-aggregate errors in FILTER
1862+
select max(unique1) filter (where sum(ten) > 0) from tenk1;
1863+
ERROR: aggregate functions are not allowed in FILTER
1864+
LINE 1: select max(unique1) filter (where sum(ten) > 0) from tenk1;
1865+
^
1866+
select (select max(unique1) filter (where sum(ten) > 0) from int8_tbl) from tenk1;
1867+
ERROR: aggregate function calls cannot be nested
1868+
LINE 1: select (select max(unique1) filter (where sum(ten) > 0) from...
1869+
^
1870+
select max(unique1) filter (where bool_or(ten > 0)) from tenk1;
1871+
ERROR: aggregate functions are not allowed in FILTER
1872+
LINE 1: select max(unique1) filter (where bool_or(ten > 0)) from ten...
1873+
^
1874+
select (select max(unique1) filter (where bool_or(ten > 0)) from int8_tbl) from tenk1;
1875+
ERROR: aggregate function calls cannot be nested
1876+
LINE 1: select (select max(unique1) filter (where bool_or(ten > 0)) ...
1877+
^
18481878
-- ordered-set aggregates
18491879
select p, percentile_cont(p) within group (order by x::float8)
18501880
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
@@ -682,6 +682,17 @@ select aggfns(distinct a,b,c order by a,c using ~<~,b) filter (where a > 1)
682682
from (values (1,3,'foo'),(0,null,null),(2,2,'bar'),(3,1,'baz')) v(a,b,c),
683683
generate_series(1,2) i;
684684

685+
-- check handling of bare boolean Var in FILTER
686+
select max(0) filter (where b1) from bool_test;
687+
select (select max(0) filter (where b1)) from bool_test;
688+
689+
-- check for correct detection of nested-aggregate errors in FILTER
690+
select max(unique1) filter (where sum(ten) > 0) from tenk1;
691+
select (select max(unique1) filter (where sum(ten) > 0) from int8_tbl) from tenk1;
692+
select max(unique1) filter (where bool_or(ten > 0)) from tenk1;
693+
select (select max(unique1) filter (where bool_or(ten > 0)) from int8_tbl) from tenk1;
694+
695+
685696
-- ordered-set aggregates
686697

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

0 commit comments

Comments
 (0)