diff options
-rw-r--r-- | src/backend/optimizer/plan/planner.c | 51 | ||||
-rw-r--r-- | src/test/regress/expected/aggregates.out | 37 | ||||
-rw-r--r-- | src/test/regress/expected/sqljson.out | 24 | ||||
-rw-r--r-- | src/test/regress/sql/aggregates.sql | 19 |
4 files changed, 115 insertions, 16 deletions
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 566ce5b3cb4..beafac8c0b0 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -3201,10 +3201,53 @@ adjust_group_pathkeys_for_groupagg(PlannerInfo *root) if (AGGKIND_IS_ORDERED_SET(aggref->aggkind)) continue; - /* only add aggregates with a DISTINCT or ORDER BY */ - if (aggref->aggdistinct != NIL || aggref->aggorder != NIL) - unprocessed_aggs = bms_add_member(unprocessed_aggs, - foreach_current_index(lc)); + /* Skip unless there's a DISTINCT or ORDER BY clause */ + if (aggref->aggdistinct == NIL && aggref->aggorder == NIL) + continue; + + /* Additional safety checks are needed if there's a FILTER clause */ + if (aggref->aggfilter != NULL) + { + ListCell *lc2; + bool allow_presort = true; + + /* + * When the Aggref has a FILTER clause, it's possible that the + * filter removes rows that cannot be sorted because the + * expression to sort by results in an error during its + * evaluation. This is a problem for presorting as that happens + * before the FILTER, whereas without presorting, the Aggregate + * node will apply the FILTER *before* sorting. So that we never + * try to sort anything that might error, here we aim to skip over + * any Aggrefs with arguments with expressions which, when + * evaluated, could cause an ERROR. Vars and Consts are ok. There + * may be more cases that should be allowed, but more thought + * needs to be given. Err on the side of caution. + */ + foreach(lc2, aggref->args) + { + TargetEntry *tle = (TargetEntry *) lfirst(lc2); + Expr *expr = tle->expr; + + while (IsA(expr, RelabelType)) + expr = (Expr *) (castNode(RelabelType, expr))->arg; + + /* Common case, Vars and Consts are ok */ + if (IsA(expr, Var) || IsA(expr, Const)) + continue; + + /* Unsupported. Don't try to presort for this Aggref */ + allow_presort = false; + break; + } + + /* Skip unsupported Aggrefs */ + if (!allow_presort) + continue; + } + + unprocessed_aggs = bms_add_member(unprocessed_aggs, + foreach_current_index(lc)); } /* diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out index 6b6371c3e74..1f1ce2380af 100644 --- a/src/test/regress/expected/aggregates.out +++ b/src/test/regress/expected/aggregates.out @@ -1679,6 +1679,43 @@ select sum(two order by two) from tenk1; reset enable_presorted_aggregate; -- +-- Test cases with FILTER clause +-- +-- Ensure we presort when the aggregate contains plain Vars +explain (costs off) +select sum(two order by two) filter (where two > 1) from tenk1; + QUERY PLAN +------------------------------- + Aggregate + -> Sort + Sort Key: two + -> Seq Scan on tenk1 +(4 rows) + +-- Ensure we presort for RelabelType'd Vars +explain (costs off) +select string_agg(distinct f1, ',') filter (where length(f1) > 1) +from varchar_tbl; + QUERY PLAN +------------------------------------- + Aggregate + -> Sort + Sort Key: f1 + -> Seq Scan on varchar_tbl +(4 rows) + +-- Ensure we don't presort when the aggregate's argument contains an +-- explicit cast. +explain (costs off) +select string_agg(distinct f1::varchar(2), ',') filter (where length(f1) > 1) +from varchar_tbl; + QUERY PLAN +------------------------------- + Aggregate + -> Seq Scan on varchar_tbl +(2 rows) + +-- -- Test combinations of DISTINCT and/or ORDER BY -- select array_agg(a order by b) diff --git a/src/test/regress/expected/sqljson.out b/src/test/regress/expected/sqljson.out index bed43aadd8c..625acf3019a 100644 --- a/src/test/regress/expected/sqljson.out +++ b/src/test/regress/expected/sqljson.out @@ -839,22 +839,22 @@ SELECT FROM (VALUES (NULL), (3), (1), (NULL), (NULL), (5), (2), (4), (NULL)) foo(bar); -[ RECORD 1 ]--------------------+------------------------------------------------------------------------------------------------------------------------- -no_options | [1, 2, 3, 4, 5] -returning_jsonb | [1, 2, 3, 4, 5] -absent_on_null | [1, 2, 3, 4, 5] -absentonnull_returning_jsonb | [1, 2, 3, 4, 5] -null_on_null | [1, 2, 3, 4, 5, null, null, null, null] -nullonnull_returning_jsonb | [1, 2, 3, 4, 5, null, null, null, null] -row_no_options | [{"bar":1}, + - | {"bar":2}, + +no_options | [3, 1, 5, 2, 4] +returning_jsonb | [3, 1, 5, 2, 4] +absent_on_null | [3, 1, 5, 2, 4] +absentonnull_returning_jsonb | [3, 1, 5, 2, 4] +null_on_null | [null, 3, 1, null, null, 5, 2, 4, null] +nullonnull_returning_jsonb | [null, 3, 1, null, null, 5, 2, 4, null] +row_no_options | [{"bar":null}, + | {"bar":3}, + - | {"bar":4}, + - | {"bar":5}, + - | {"bar":null}, + + | {"bar":1}, + | {"bar":null}, + | {"bar":null}, + + | {"bar":5}, + + | {"bar":2}, + + | {"bar":4}, + | {"bar":null}] -row_returning_jsonb | [{"bar": 1}, {"bar": 2}, {"bar": 3}, {"bar": 4}, {"bar": 5}, {"bar": null}, {"bar": null}, {"bar": null}, {"bar": null}] +row_returning_jsonb | [{"bar": null}, {"bar": 3}, {"bar": 1}, {"bar": null}, {"bar": null}, {"bar": 5}, {"bar": 2}, {"bar": 4}, {"bar": null}] row_filtered_agg | [{"bar":3}, + | {"bar":4}, + | {"bar":5}] diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql index 2c47a462b7e..277b4b198cc 100644 --- a/src/test/regress/sql/aggregates.sql +++ b/src/test/regress/sql/aggregates.sql @@ -634,6 +634,25 @@ select sum(two order by two) from tenk1; reset enable_presorted_aggregate; -- +-- Test cases with FILTER clause +-- + +-- Ensure we presort when the aggregate contains plain Vars +explain (costs off) +select sum(two order by two) filter (where two > 1) from tenk1; + +-- Ensure we presort for RelabelType'd Vars +explain (costs off) +select string_agg(distinct f1, ',') filter (where length(f1) > 1) +from varchar_tbl; + +-- Ensure we don't presort when the aggregate's argument contains an +-- explicit cast. +explain (costs off) +select string_agg(distinct f1::varchar(2), ',') filter (where length(f1) > 1) +from varchar_tbl; + +-- -- Test combinations of DISTINCT and/or ORDER BY -- |