poc_fix_presorted_aggs_with_filter.patch

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 566ce5b3cb4..682c5107e4b 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -3201,9 +3201,58 @@ 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,
+		/* skip unless there's a DISTINCT or ORDER BY clause */
+		if (aggref->aggdistinct == NIL && aggref->aggorder == NIL)
+			continue;
+
+		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 Agg 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.  Also we can
+			 * ignore any implicit casts as these should also never cause an
+			 * error.  There may be more cases that should be allowed, but
+			 * more thought needs to be given.  We err on the side of caution.
+			 */
+			foreach(lc2, aggref->args)
+			{
+				TargetEntry *tle = (TargetEntry *) lfirst(lc2);
+				Expr *expr = tle->expr;
+
+				/* Common case, vars and Consts are ok */
+				if (IsA(expr, Var) || IsA(expr, Const))
+					continue;
+
+				/*
+				 * implicit casts should be safe, try removing those and
+				 * check for Vars and Consts again.
+				 */
+				expr = (Expr *) strip_implicit_coercions((Node *) expr);
+
+				if (IsA(expr, Var) || IsA(expr, Const))
+					continue;
+
+				/* nope. Can't do it. */
+				allow_presort = false;
+				break;
+			}
+
+			/* did we find something unsupported? */
+			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..3949c42ea58 100644
--- a/src/test/regress/expected/aggregates.out
+++ b/src/test/regress/expected/aggregates.out
@@ -1678,6 +1678,44 @@ select sum(two order by two) from tenk1;
 (2 rows)
 
 reset enable_presorted_aggregate;
+--
+-- Test cases with FILTER clause
+--
+-- Ensure we presort when the aggregate contains plain Vars
+explain (costs off)
+select sum(reltuples order by reltuples) filter (where reltuples >= 0)
+from pg_class;
+            QUERY PLAN            
+----------------------------------
+ Aggregate
+   ->  Sort
+         Sort Key: reltuples
+         ->  Seq Scan on pg_class
+(4 rows)
+
+-- Ensure we presort when there's an implicit cast on a Var
+explain (costs off)
+select string_agg(distinct relname, ',') filter (where relname like 'pg%')
+from pg_class;
+                   QUERY PLAN                    
+-------------------------------------------------
+ Aggregate
+   ->  Sort
+         Sort Key: ((relname)::text) COLLATE "C"
+         ->  Seq Scan on pg_class
+(4 rows)
+
+-- Ensure we don't presort when the aggregate's argument contains an
+-- explicit cast.
+explain (costs off)
+select string_agg(distinct relname::varchar(10), ',') filter (where relname like 'pg%')
+from pg_class;
+         QUERY PLAN         
+----------------------------
+ Aggregate
+   ->  Seq Scan on pg_class
+(2 rows)
+
 --
 -- Test combinations of DISTINCT and/or ORDER BY
 --
diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql
index 2c47a462b7e..af8e8f48cd6 100644
--- a/src/test/regress/sql/aggregates.sql
+++ b/src/test/regress/sql/aggregates.sql
@@ -633,6 +633,26 @@ explain (costs off)
 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(reltuples order by reltuples) filter (where reltuples >= 0)
+from pg_class;
+
+-- Ensure we presort when there's an implicit cast on a Var
+explain (costs off)
+select string_agg(distinct relname, ',') filter (where relname like 'pg%')
+from pg_class;
+
+-- Ensure we don't presort when the aggregate's argument contains an
+-- explicit cast.
+explain (costs off)
+select string_agg(distinct relname::varchar(10), ',') filter (where relname like 'pg%')
+from pg_class;
+
 --
 -- Test combinations of DISTINCT and/or ORDER BY
 --