Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit065ce49

Browse files
committed
Fix issue with ORDER BY / DISTINCT aggregates and FILTER
1349d27 added support so that aggregate functions with an ORDER BY orDISTINCT clause could make use of presorted inputs to avoid an implicitsort within nodeAgg.c. That commit failed to consider that a FILTERclause may exist that filters rows before the aggregate functionarguments are evaluated. That can be problematic if an aggregateargument contains an expression which could error out during evaluation.It's perfectly valid to want to have a FILTER clause which eliminatessuch values, and with the pre-sorted path added in1349d27, it waspossible that the planner would produce a plan with a Sort node abovethe Aggregate to perform the sort on the aggregate's arguments long beforethe Aggregate node would filter out the non-matching values.Here we fix this by inspecting ORDER BY / DISTINCT aggregate functionswhich have a FILTER clause to see if the aggregate's arguments areanything more complex than a Var or a Const. Evaluating these isn'tgoing to cause an error. If we find any non-Var, non-Const parametersthen the planner will now opt to perform the sort in the Aggregate nodefor these aggregates, i.e. disable the presorted aggregate optimization.An alternative fix would have been to completely disallow the presortedoptimization for Aggrefs with any FILTER clause, but that wasn't done asthat could cause large performance regressions for queries that seesignificant gains from1349d27 due to presorted results coming in froman Index Scan.Backpatch to 16, where1349d27 was introducedAuthor: David Rowley <dgrowleyml@gmail.com>Reported-by: Kaimeh <kkaimeh@gmail.com>Diagnosed-by: Tom Lane <tgl@sss.pgh.pa.us>Discussion:https://postgr.es/m/CAK-%2BJz9J%3DQ06-M7cDJoPNeYbz5EZDqkjQbJnmRyQyzkbRGsYkA%40mail.gmail.comBackpatch-through: 16
1 parentecb8e56 commit065ce49

File tree

4 files changed

+115
-16
lines changed

4 files changed

+115
-16
lines changed

‎src/backend/optimizer/plan/planner.c

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3280,10 +3280,53 @@ adjust_group_pathkeys_for_groupagg(PlannerInfo *root)
32803280
if (AGGKIND_IS_ORDERED_SET(aggref->aggkind))
32813281
continue;
32823282

3283-
/* only add aggregates with a DISTINCT or ORDER BY */
3284-
if (aggref->aggdistinct!=NIL||aggref->aggorder!=NIL)
3285-
unprocessed_aggs=bms_add_member(unprocessed_aggs,
3286-
foreach_current_index(lc));
3283+
/* Skip unless there's a DISTINCT or ORDER BY clause */
3284+
if (aggref->aggdistinct==NIL&&aggref->aggorder==NIL)
3285+
continue;
3286+
3287+
/* Additional safety checks are needed if there's a FILTER clause */
3288+
if (aggref->aggfilter!=NULL)
3289+
{
3290+
ListCell*lc2;
3291+
boolallow_presort= true;
3292+
3293+
/*
3294+
* When the Aggref has a FILTER clause, it's possible that the
3295+
* filter removes rows that cannot be sorted because the
3296+
* expression to sort by results in an error during its
3297+
* evaluation. This is a problem for presorting as that happens
3298+
* before the FILTER, whereas without presorting, the Aggregate
3299+
* node will apply the FILTER *before* sorting. So that we never
3300+
* try to sort anything that might error, here we aim to skip over
3301+
* any Aggrefs with arguments with expressions which, when
3302+
* evaluated, could cause an ERROR. Vars and Consts are ok. There
3303+
* may be more cases that should be allowed, but more thought
3304+
* needs to be given. Err on the side of caution.
3305+
*/
3306+
foreach(lc2,aggref->args)
3307+
{
3308+
TargetEntry*tle= (TargetEntry*)lfirst(lc2);
3309+
Expr*expr=tle->expr;
3310+
3311+
while (IsA(expr,RelabelType))
3312+
expr= (Expr*) (castNode(RelabelType,expr))->arg;
3313+
3314+
/* Common case, Vars and Consts are ok */
3315+
if (IsA(expr,Var)||IsA(expr,Const))
3316+
continue;
3317+
3318+
/* Unsupported. Don't try to presort for this Aggref */
3319+
allow_presort= false;
3320+
break;
3321+
}
3322+
3323+
/* Skip unsupported Aggrefs */
3324+
if (!allow_presort)
3325+
continue;
3326+
}
3327+
3328+
unprocessed_aggs=bms_add_member(unprocessed_aggs,
3329+
foreach_current_index(lc));
32873330
}
32883331

32893332
/*

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1586,6 +1586,43 @@ select sum(two order by two) from tenk1;
15861586
(2 rows)
15871587

15881588
reset enable_presorted_aggregate;
1589+
--
1590+
-- Test cases with FILTER clause
1591+
--
1592+
-- Ensure we presort when the aggregate contains plain Vars
1593+
explain (costs off)
1594+
select sum(two order by two) filter (where two > 1) from tenk1;
1595+
QUERY PLAN
1596+
-------------------------------
1597+
Aggregate
1598+
-> Sort
1599+
Sort Key: two
1600+
-> Seq Scan on tenk1
1601+
(4 rows)
1602+
1603+
-- Ensure we presort for RelabelType'd Vars
1604+
explain (costs off)
1605+
select string_agg(distinct f1, ',') filter (where length(f1) > 1)
1606+
from varchar_tbl;
1607+
QUERY PLAN
1608+
-------------------------------------
1609+
Aggregate
1610+
-> Sort
1611+
Sort Key: f1
1612+
-> Seq Scan on varchar_tbl
1613+
(4 rows)
1614+
1615+
-- Ensure we don't presort when the aggregate's argument contains an
1616+
-- explicit cast.
1617+
explain (costs off)
1618+
select string_agg(distinct f1::varchar(2), ',') filter (where length(f1) > 1)
1619+
from varchar_tbl;
1620+
QUERY PLAN
1621+
-------------------------------
1622+
Aggregate
1623+
-> Seq Scan on varchar_tbl
1624+
(2 rows)
1625+
15891626
--
15901627
-- Test combinations of DISTINCT and/or ORDER BY
15911628
--

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

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -835,22 +835,22 @@ SELECT
835835
FROM
836836
(VALUES (NULL), (3), (1), (NULL), (NULL), (5), (2), (4), (NULL)) foo(bar);
837837
-[ RECORD 1 ]--------------------+-------------------------------------------------------------------------------------------------------------------------
838-
no_options | [1, 2, 3, 4, 5]
839-
returning_jsonb | [1, 2, 3, 4, 5]
840-
absent_on_null | [1, 2, 3, 4, 5]
841-
absentonnull_returning_jsonb | [1, 2, 3, 4, 5]
842-
null_on_null | [1, 2, 3, 4, 5, null, null, null, null]
843-
nullonnull_returning_jsonb | [1, 2, 3, 4, 5, null, null, null, null]
844-
row_no_options | [{"bar":1}, +
845-
| {"bar":2}, +
838+
no_options | [3, 1, 5, 2, 4]
839+
returning_jsonb | [3, 1, 5, 2, 4]
840+
absent_on_null | [3, 1, 5, 2, 4]
841+
absentonnull_returning_jsonb | [3, 1, 5, 2, 4]
842+
null_on_null | [null, 3, 1, null, null, 5, 2, 4, null]
843+
nullonnull_returning_jsonb | [null, 3, 1, null, null, 5, 2, 4, null]
844+
row_no_options | [{"bar":null}, +
846845
| {"bar":3}, +
847-
| {"bar":4}, +
848-
| {"bar":5}, +
849-
| {"bar":null}, +
846+
| {"bar":1}, +
850847
| {"bar":null}, +
851848
| {"bar":null}, +
849+
| {"bar":5}, +
850+
| {"bar":2}, +
851+
| {"bar":4}, +
852852
| {"bar":null}]
853-
row_returning_jsonb | [{"bar":1}, {"bar":2}, {"bar":3}, {"bar":4}, {"bar":5}, {"bar":null}, {"bar":null}, {"bar":null}, {"bar": null}]
853+
row_returning_jsonb | [{"bar":null}, {"bar":3}, {"bar":1}, {"bar":null}, {"bar":null}, {"bar":5}, {"bar":2}, {"bar":4}, {"bar": null}]
854854
row_filtered_agg | [{"bar":3}, +
855855
| {"bar":4}, +
856856
| {"bar":5}]

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -595,6 +595,25 @@ explain (costs off)
595595
selectsum(twoorder by two)from tenk1;
596596
reset enable_presorted_aggregate;
597597

598+
--
599+
-- Test cases with FILTER clause
600+
--
601+
602+
-- Ensure we presort when the aggregate contains plain Vars
603+
explain (costs off)
604+
selectsum(twoorder by two) filter (where two>1)from tenk1;
605+
606+
-- Ensure we presort for RelabelType'd Vars
607+
explain (costs off)
608+
select string_agg(distinct f1,',') filter (where length(f1)>1)
609+
from varchar_tbl;
610+
611+
-- Ensure we don't presort when the aggregate's argument contains an
612+
-- explicit cast.
613+
explain (costs off)
614+
select string_agg(distinct f1::varchar(2),',') filter (where length(f1)>1)
615+
from varchar_tbl;
616+
598617
--
599618
-- Test combinations of DISTINCT and/or ORDER BY
600619
--

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp