v2-0001-Fix-HAVING-to-WHERE-pushdown-with-nondeterministi.patch

From 06717b1ee9aa7304d70e51bd4ed0f949f0cd220d Mon Sep 17 00:00:00 2001
From: Richard Guo <guofenglinux@gmail.com>
Date: Mon, 30 Mar 2026 19:43:56 +0900
Subject: [PATCH v2] Fix HAVING-to-WHERE pushdown with nondeterministic
 collations

When GROUP BY uses a nondeterministic collation, the planner's
optimization of moving HAVING clauses to WHERE can produce incorrect
query results.  The HAVING clause may apply a stricter collation that
distinguishes values the GROUP BY considers equal.  Pushing such a
clause to WHERE causes it to filter individual rows before grouping,
potentially eliminating group members and changing aggregate results.

Fix this by detecting collation conflicts before flatten_group_exprs,
while the HAVING clause still contains GROUP Vars (Vars referencing
RTE_GROUP).  At that point, each GROUP Var directly carries the GROUP
BY collation as its varcollid, making it straightforward to compare
against the operator's inputcollid.  A mismatch where the GROUP BY
collation is nondeterministic means the clause is unsafe to push down.

The conflicting clause indices are recorded in a Bitmapset and
consulted during the existing HAVING-to-WHERE loop, so that only
affected clauses are kept in HAVING; other safe clauses in the same
query are still pushed.

Back-patch to v18 only.  The fix relies on the RTE_GROUP mechanism
introduced in v18 (commit 247dea89f), which is what lets us identify
grouping expressions and their resolved collations via GROUP Vars on
pre-flatten havingQual.  Pre-v18 branches lack that machinery, so a
back-patch there would need a different approach.  Given the absence
of field reports of this bug on back branches, the risk of carrying a
different fix on stable branches is not justified.
---
 src/backend/optimizer/plan/planner.c          | 124 +++++++++++
 .../regress/expected/collate.icu.utf8.out     | 194 ++++++++++++++++++
 src/test/regress/sql/collate.icu.utf8.sql     |  61 ++++++
 3 files changed, 379 insertions(+)

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 4ec76ce31a9..10491cfd7f6 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -137,6 +137,9 @@ typedef struct
 /* Local functions */
 static Node *preprocess_expression(PlannerInfo *root, Node *expr, int kind);
 static void preprocess_qual_conditions(PlannerInfo *root, Node *jtnode);
+static Bitmapset *find_having_collation_conflicts(Query *parse,
+												  Index group_rtindex);
+static bool having_collation_conflict_walker(Node *node, Index *group_rtindex);
 static void grouping_planner(PlannerInfo *root, double tuple_fraction,
 							 SetOperationStmt *setops);
 static grouping_sets_data *preprocess_grouping_sets(PlannerInfo *root);
@@ -762,6 +765,8 @@ subquery_planner(PlannerGlobal *glob, Query *parse, char *plan_name,
 	PlannerInfo *root;
 	List	   *newWithCheckOptions;
 	List	   *newHaving;
+	Bitmapset  *havingCollationConflicts;
+	int			havingIdx;
 	bool		hasOuterJoins;
 	bool		hasResultRTEs;
 	RelOptInfo *final_rel;
@@ -1175,6 +1180,27 @@ subquery_planner(PlannerGlobal *glob, Query *parse, char *plan_name,
 		}
 	}
 
+	/*
+	 * Before we flatten GROUP Vars, check which HAVING clauses have collation
+	 * conflicts.  When GROUP BY uses a nondeterministic collation, values
+	 * that are "equal" for grouping may be distinguishable under a different
+	 * collation.  If such a HAVING clause were moved to WHERE, it would
+	 * filter individual rows before grouping, potentially eliminating some
+	 * members of a group and thereby changing aggregate results.
+	 *
+	 * We do this check before flatten_group_exprs because we can easily
+	 * identify grouping expressions by checking whether a Var references
+	 * RTE_GROUP, and such Vars directly carry the GROUP BY collation as their
+	 * varcollid.  After flattening, these Vars are replaced by the underlying
+	 * expressions, and we would have to match expressions in the HAVING
+	 * clause back to grouping expressions, which is much more complex.
+	 */
+	if (parse->hasGroupRTE)
+		havingCollationConflicts =
+			find_having_collation_conflicts(parse, root->group_rtindex);
+	else
+		havingCollationConflicts = NULL;
+
 	/*
 	 * Replace any Vars in the subquery's targetlist and havingQual that
 	 * reference GROUP outputs with the underlying grouping expressions.
@@ -1219,6 +1245,14 @@ subquery_planner(PlannerGlobal *glob, Query *parse, char *plan_name,
 	 * but it's okay: it's just an optimization to avoid running pull_varnos
 	 * when there cannot be any Vars in the HAVING clause.)
 	 *
+	 * We also cannot do this if the HAVING clause uses a different collation
+	 * than the GROUP BY for any grouping expression whose GROUP BY collation
+	 * is nondeterministic.  This is detected before flatten_group_exprs (see
+	 * find_having_collation_conflicts above) and recorded in the
+	 * havingCollationConflicts bitmapset.  The bitmapset indexes remain valid
+	 * here because flatten_group_exprs uses expression_tree_mutator, which
+	 * preserves the list length and ordering of havingQual.
+	 *
 	 * Also, it may be that the clause is so expensive to execute that we're
 	 * better off doing it only once per group, despite the loss of
 	 * selectivity.  This is hard to estimate short of doing the entire
@@ -1251,6 +1285,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse, char *plan_name,
 	 * as Node *.
 	 */
 	newHaving = NIL;
+	havingIdx = 0;
 	foreach(l, (List *) parse->havingQual)
 	{
 		Node	   *havingclause = (Node *) lfirst(l);
@@ -1258,6 +1293,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse, char *plan_name,
 		if (contain_agg_clause(havingclause) ||
 			contain_volatile_functions(havingclause) ||
 			contain_subplans(havingclause) ||
+			bms_is_member(havingIdx, havingCollationConflicts) ||
 			(parse->groupClause && parse->groupingSets &&
 			 bms_is_member(root->group_rtindex, pull_varnos(root, havingclause))))
 		{
@@ -1294,6 +1330,8 @@ subquery_planner(PlannerGlobal *glob, Query *parse, char *plan_name,
 			/* ... and also keep it in HAVING */
 			newHaving = lappend(newHaving, havingclause);
 		}
+
+		havingIdx++;
 	}
 	parse->havingQual = (Node *) newHaving;
 
@@ -1485,6 +1523,92 @@ preprocess_qual_conditions(PlannerInfo *root, Node *jtnode)
 			 (int) nodeTag(jtnode));
 }
 
+/*
+ * find_having_collation_conflicts
+ *	  Identify HAVING clauses that must not be moved to WHERE due to collation
+ *	  mismatches with GROUP BY.
+ *
+ * This must be called before flatten_group_exprs, while the HAVING clause
+ * still contains GROUP Vars (Vars referencing RTE_GROUP).  These GROUP Vars
+ * carry the GROUP BY collation as their varcollid, so checking for conflicts
+ * is straightforward: for each collation-sensitive operator in a HAVING
+ * clause, we check if any GROUP Var in its argument subtree has a
+ * nondeterministic collation that differs from the operator's inputcollid.
+ *
+ * Returns a Bitmapset of zero-based indexes into the havingQual list for
+ * clauses that have collation conflicts and must stay in HAVING.
+ */
+static Bitmapset *
+find_having_collation_conflicts(Query *parse, Index group_rtindex)
+{
+	Bitmapset  *result = NULL;
+	int			idx = 0;
+
+	if (parse->havingQual == NULL)
+		return NULL;
+
+	foreach_ptr(Node, clause, (List *) parse->havingQual)
+	{
+		if (having_collation_conflict_walker(clause, &group_rtindex))
+			result = bms_add_member(result, idx);
+		idx++;
+	}
+
+	return result;
+}
+
+/*
+ * Walker function for find_having_collation_conflicts.
+ *
+ * At each node, use exprInputCollation() to get its inputcollid (if any).
+ * If valid, check whether any GROUP Var in the node's subtree has a
+ * nondeterministic varcollid that differs from the inputcollid.  Such a
+ * mismatch means the node would distinguish values that the GROUP BY
+ * considers equal, making it unsafe to push the clause to WHERE.
+ */
+static bool
+having_collation_conflict_walker(Node *node, Index *group_rtindex)
+{
+	Oid			inputcollid;
+
+	if (node == NULL)
+		return false;
+
+	inputcollid = exprInputCollation(node);
+	if (OidIsValid(inputcollid))
+	{
+		List	   *vars;
+
+		/*
+		 * PlaceHolderVars may have been introduced by pull_up_subqueries, and
+		 * we need to look through them to find the underlying Vars.  Aggrefs
+		 * can be present here, and we need to look through them to reach any
+		 * GROUP Vars in their direct arguments.  WindowFuncs are ignored
+		 * since they cannot appear in a HAVING clause.
+		 */
+		vars = pull_var_clause(node,
+							   PVC_RECURSE_PLACEHOLDERS |
+							   PVC_RECURSE_AGGREGATES);
+
+		foreach_node(Var, var, vars)
+		{
+			if (var->varno == *group_rtindex &&
+				OidIsValid(var->varcollid) &&
+				var->varcollid != inputcollid &&
+				!get_collation_isdeterministic(var->varcollid))
+			{
+				list_free(vars);
+				return true;
+			}
+		}
+
+		list_free(vars);
+	}
+
+	return expression_tree_walker(node, having_collation_conflict_walker,
+								  group_rtindex);
+}
+
 /*
  * preprocess_phv_expression
  *	  Do preprocessing on a PlaceHolderVar expression that's been pulled up.
diff --git a/src/test/regress/expected/collate.icu.utf8.out b/src/test/regress/expected/collate.icu.utf8.out
index fce726029a2..2eb4c8eb94f 100644
--- a/src/test/regress/expected/collate.icu.utf8.out
+++ b/src/test/regress/expected/collate.icu.utf8.out
@@ -1780,6 +1780,200 @@ SELECT string_to_array('ABCDEFGHI' COLLATE case_insensitive, NULL, 'b');
  {A,NULL,C,D,E,F,G,H,I}
 (1 row)
 
+-- Test HAVING-to-WHERE pushdown with nondeterministic collations.
+-- When a HAVING clause uses a different collation than the GROUP BY's
+-- nondeterministic collation, it must not be pushed to WHERE, otherwise
+-- aggregate results can change because the stricter filter eliminates rows
+-- before grouping.
+-- Negative: collation conflict, HAVING must not be pushed to WHERE
+EXPLAIN (COSTS OFF)
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING x = 'abc' COLLATE case_sensitive;
+                     QUERY PLAN                     
+----------------------------------------------------
+ HashAggregate
+   Group Key: x
+   Filter: (x = 'abc'::text COLLATE case_sensitive)
+   ->  Seq Scan on test3ci
+(4 rows)
+
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING x = 'abc' COLLATE case_sensitive;
+  x  | count 
+-----+-------
+ abc |     2
+(1 row)
+
+-- Positive: same collation, safe to push HAVING to WHERE
+EXPLAIN (COSTS OFF)
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING x = 'abc' COLLATE case_insensitive;
+                         QUERY PLAN                         
+------------------------------------------------------------
+ GroupAggregate
+   ->  Seq Scan on test3ci
+         Filter: (x = 'abc'::text COLLATE case_insensitive)
+(3 rows)
+
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING x = 'abc' COLLATE case_insensitive;
+  x  | count 
+-----+-------
+ abc |     2
+(1 row)
+
+-- Negative: function applied to grouped column with conflicting collation
+EXPLAIN (COSTS OFF)
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING upper(x) = 'ABC' COLLATE case_sensitive;
+                        QUERY PLAN                         
+-----------------------------------------------------------
+ HashAggregate
+   Group Key: x
+   Filter: (upper(x) = 'ABC'::text COLLATE case_sensitive)
+   ->  Seq Scan on test3ci
+(4 rows)
+
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING upper(x) = 'ABC' COLLATE case_sensitive;
+  x  | count 
+-----+-------
+ abc |     2
+(1 row)
+
+-- Positive: function with same collation as GROUP BY
+EXPLAIN (COSTS OFF)
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING upper(x) = 'ABC' COLLATE case_insensitive;
+                               QUERY PLAN                                
+-------------------------------------------------------------------------
+ GroupAggregate
+   Group Key: x
+   ->  Sort
+         Sort Key: x COLLATE case_insensitive
+         ->  Seq Scan on test3ci
+               Filter: (upper(x) = 'ABC'::text COLLATE case_insensitive)
+(6 rows)
+
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING upper(x) = 'ABC' COLLATE case_insensitive;
+  x  | count 
+-----+-------
+ abc |     2
+(1 row)
+
+-- Negative: inner function has conflicting collation, even though outer
+-- operator's collation matches GROUP BY due to a COLLATE override
+EXPLAIN (COSTS OFF)
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING upper(x COLLATE case_sensitive) COLLATE case_insensitive = 'ABC';
+                     QUERY PLAN                     
+----------------------------------------------------
+ HashAggregate
+   Group Key: x
+   Filter: ((upper((x)::text))::text = 'ABC'::text)
+   ->  Seq Scan on test3ci
+(4 rows)
+
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING upper(x COLLATE case_sensitive) COLLATE case_insensitive = 'ABC';
+  x  | count 
+-----+-------
+ abc |     2
+(1 row)
+
+-- Mixed AND: conflicting clause stays in HAVING, safe clause pushed to WHERE
+EXPLAIN (COSTS OFF)
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING x = 'abc' COLLATE case_sensitive AND length(x) > 1;
+                     QUERY PLAN                     
+----------------------------------------------------
+ HashAggregate
+   Group Key: x
+   Filter: (x = 'abc'::text COLLATE case_sensitive)
+   ->  Seq Scan on test3ci
+         Filter: (length(x) > 1)
+(5 rows)
+
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING x = 'abc' COLLATE case_sensitive AND length(x) > 1;
+  x  | count 
+-----+-------
+ abc |     2
+(1 row)
+
+-- Positive: AND of two safe clauses, both can be pushed
+EXPLAIN (COSTS OFF)
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING x = 'abc' COLLATE case_insensitive AND length(x) > 1;
+                                    QUERY PLAN                                    
+----------------------------------------------------------------------------------
+ GroupAggregate
+   ->  Seq Scan on test3ci
+         Filter: ((x = 'abc'::text COLLATE case_insensitive) AND (length(x) > 1))
+(3 rows)
+
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING x = 'abc' COLLATE case_insensitive AND length(x) > 1;
+  x  | count 
+-----+-------
+ abc |     2
+(1 row)
+
+-- Negative: OR with a conflicting clause: must stay in HAVING
+EXPLAIN (COSTS OFF)
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING x = 'abc' COLLATE case_sensitive OR x = 'def' COLLATE case_sensitive ORDER BY 1;
+                                               QUERY PLAN                                               
+--------------------------------------------------------------------------------------------------------
+ Sort
+   Sort Key: x COLLATE case_insensitive
+   ->  HashAggregate
+         Group Key: x
+         Filter: ((x = 'abc'::text COLLATE case_sensitive) OR (x = 'def'::text COLLATE case_sensitive))
+         ->  Seq Scan on test3ci
+(6 rows)
+
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING x = 'abc' COLLATE case_sensitive OR x = 'def' COLLATE case_sensitive ORDER BY 1;
+  x  | count 
+-----+-------
+ abc |     2
+ def |     1
+(2 rows)
+
+-- Positive: conflicting collation but no grouping expression reference
+EXPLAIN (COSTS OFF)
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING current_setting('server_version') = 'abc' COLLATE case_sensitive;
+                                               QUERY PLAN                                                
+---------------------------------------------------------------------------------------------------------
+ HashAggregate
+   Group Key: x
+   ->  Result
+         One-Time Filter: (current_setting('server_version'::text) = 'abc'::text COLLATE case_sensitive)
+         ->  Seq Scan on test3ci
+(5 rows)
+
+-- Positive: deterministic collation in GROUP BY: always safe to push, even if
+-- HAVING uses a nondeterministic collation
+EXPLAIN (COSTS OFF)
+SELECT x, count(*) FROM test3cs GROUP BY x HAVING x = 'abc' COLLATE case_sensitive;
+                        QUERY PLAN                        
+----------------------------------------------------------
+ GroupAggregate
+   ->  Seq Scan on test3cs
+         Filter: (x = 'abc'::text COLLATE case_sensitive)
+(3 rows)
+
+SELECT x, count(*) FROM test3cs GROUP BY x HAVING x = 'abc' COLLATE case_sensitive;
+  x  | count 
+-----+-------
+ abc |     1
+(1 row)
+
+EXPLAIN (COSTS OFF)
+SELECT x, count(*) FROM test3cs GROUP BY x HAVING x = 'abc' COLLATE case_insensitive ORDER BY 1;
+                            QUERY PLAN                            
+------------------------------------------------------------------
+ GroupAggregate
+   Group Key: x
+   ->  Sort
+         Sort Key: x COLLATE case_sensitive
+         ->  Seq Scan on test3cs
+               Filter: (x = 'abc'::text COLLATE case_insensitive)
+(6 rows)
+
+SELECT x, count(*) FROM test3cs GROUP BY x HAVING x = 'abc' COLLATE case_insensitive ORDER BY 1;
+  x  | count 
+-----+-------
+ abc |     1
+ ABC |     1
+(2 rows)
+
 -- bpchar
 CREATE TABLE test1bpci (x char(3) COLLATE case_insensitive);
 CREATE TABLE test2bpci (x char(3) COLLATE case_insensitive);
diff --git a/src/test/regress/sql/collate.icu.utf8.sql b/src/test/regress/sql/collate.icu.utf8.sql
index 0bf65a63535..2fe3d5466d6 100644
--- a/src/test/regress/sql/collate.icu.utf8.sql
+++ b/src/test/regress/sql/collate.icu.utf8.sql
@@ -642,6 +642,67 @@ CREATE UNIQUE INDEX ON test3ci (x);  -- error
 SELECT string_to_array('ABC,DEF,GHI' COLLATE case_insensitive, ',', 'abc');
 SELECT string_to_array('ABCDEFGHI' COLLATE case_insensitive, NULL, 'b');
 
+-- Test HAVING-to-WHERE pushdown with nondeterministic collations.
+-- When a HAVING clause uses a different collation than the GROUP BY's
+-- nondeterministic collation, it must not be pushed to WHERE, otherwise
+-- aggregate results can change because the stricter filter eliminates rows
+-- before grouping.
+
+-- Negative: collation conflict, HAVING must not be pushed to WHERE
+EXPLAIN (COSTS OFF)
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING x = 'abc' COLLATE case_sensitive;
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING x = 'abc' COLLATE case_sensitive;
+
+-- Positive: same collation, safe to push HAVING to WHERE
+EXPLAIN (COSTS OFF)
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING x = 'abc' COLLATE case_insensitive;
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING x = 'abc' COLLATE case_insensitive;
+
+-- Negative: function applied to grouped column with conflicting collation
+EXPLAIN (COSTS OFF)
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING upper(x) = 'ABC' COLLATE case_sensitive;
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING upper(x) = 'ABC' COLLATE case_sensitive;
+
+-- Positive: function with same collation as GROUP BY
+EXPLAIN (COSTS OFF)
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING upper(x) = 'ABC' COLLATE case_insensitive;
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING upper(x) = 'ABC' COLLATE case_insensitive;
+
+-- Negative: inner function has conflicting collation, even though outer
+-- operator's collation matches GROUP BY due to a COLLATE override
+EXPLAIN (COSTS OFF)
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING upper(x COLLATE case_sensitive) COLLATE case_insensitive = 'ABC';
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING upper(x COLLATE case_sensitive) COLLATE case_insensitive = 'ABC';
+
+-- Mixed AND: conflicting clause stays in HAVING, safe clause pushed to WHERE
+EXPLAIN (COSTS OFF)
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING x = 'abc' COLLATE case_sensitive AND length(x) > 1;
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING x = 'abc' COLLATE case_sensitive AND length(x) > 1;
+
+-- Positive: AND of two safe clauses, both can be pushed
+EXPLAIN (COSTS OFF)
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING x = 'abc' COLLATE case_insensitive AND length(x) > 1;
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING x = 'abc' COLLATE case_insensitive AND length(x) > 1;
+
+-- Negative: OR with a conflicting clause: must stay in HAVING
+EXPLAIN (COSTS OFF)
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING x = 'abc' COLLATE case_sensitive OR x = 'def' COLLATE case_sensitive ORDER BY 1;
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING x = 'abc' COLLATE case_sensitive OR x = 'def' COLLATE case_sensitive ORDER BY 1;
+
+-- Positive: conflicting collation but no grouping expression reference
+EXPLAIN (COSTS OFF)
+SELECT x, count(*) FROM test3ci GROUP BY x HAVING current_setting('server_version') = 'abc' COLLATE case_sensitive;
+
+-- Positive: deterministic collation in GROUP BY: always safe to push, even if
+-- HAVING uses a nondeterministic collation
+EXPLAIN (COSTS OFF)
+SELECT x, count(*) FROM test3cs GROUP BY x HAVING x = 'abc' COLLATE case_sensitive;
+SELECT x, count(*) FROM test3cs GROUP BY x HAVING x = 'abc' COLLATE case_sensitive;
+
+EXPLAIN (COSTS OFF)
+SELECT x, count(*) FROM test3cs GROUP BY x HAVING x = 'abc' COLLATE case_insensitive ORDER BY 1;
+SELECT x, count(*) FROM test3cs GROUP BY x HAVING x = 'abc' COLLATE case_insensitive ORDER BY 1;
+
 -- bpchar
 CREATE TABLE test1bpci (x char(3) COLLATE case_insensitive);
 CREATE TABLE test2bpci (x char(3) COLLATE case_insensitive);
-- 
2.39.5 (Apple Git-154)