From 2344edb152ac9bd6a83e8d308ebb7606154adfb8 Mon Sep 17 00:00:00 2001 From: Richard Guo Date: Thu, 27 Aug 2026 17:41:13 +0900 Subject: [PATCH v2] Fix qual pushdown past grouping through simple CASE Commit 44fb59fc6 taught the grouping-conflict walker to treat the arg of a simple CASE as a direct operand of each WHEN comparison, but it only checked the collation, on the assumption that the WHEN operator is always the type-default "=" and thus matches the grouping eqop. That assumption fails once the arg is relabeled to another type: the WHEN then compares under that type's "=", which need not agree with the grouping equality. For instance, with a DISTINCT over a citext column, a qual such as "CASE t::text WHEN 'A' THEN ..." was pushed below the Unique, although the equivalent "t::text = 'A'" is correctly kept above it. Instead of special-casing the arg, have the walker bind a Var arg while walking the WHEN conditions and resolve each CaseTestExpr to it, so that the arg is checked exactly as each WHEN uses it: with the opfamily and collation checks of a direct operand when the WHEN is a comparison, and as a non-operand reference otherwise. A non-Var arg is walked once as a non-operand, as before. The CaseTestExpr in an ArrayCoerceExpr's elemexpr is left alone, since it does not refer to any CASE arg. --- src/backend/optimizer/util/clauses.c | 109 +++++++++++++++--------- src/test/regress/expected/subselect.out | 27 ++++++ src/test/regress/sql/subselect.sql | 11 +++ 3 files changed, 107 insertions(+), 40 deletions(-) diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index 8da4ed617b5..2778169bbd2 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -103,12 +103,15 @@ typedef struct /* * Walker context for expression_has_grouping_conflict. get_eqop is a callback * that returns the equality operator used for grouping. cb_context is opaque - * to the walker and is forwarded to get_eqop unchanged. + * to the walker and is forwarded to get_eqop unchanged. case_var is the Var + * that the CaseTestExprs of the simple CASE being walked stand for, or NULL if + * there is none. */ typedef struct { grouping_eqop_callback get_eqop; void *cb_context; + Var *case_var; } grouping_walker_ctx; static bool contain_agg_clause_walker(Node *node, void *context); @@ -6376,6 +6379,7 @@ expression_has_grouping_conflict(Node *expr, ctx.get_eqop = get_eqop; ctx.cb_context = context; + ctx.case_var = NULL; return grouping_conflict_walker(expr, &ctx); } @@ -6394,9 +6398,13 @@ expression_has_grouping_conflict(Node *expr, * member, and RowCompareExpr (one operator and collation per column). A * simple CASE (CaseExpr with a non-NULL arg) is a comparison in disguise: * parse analysis builds each WHEN as "OpExpr(CaseTestExpr op val)", with the - * CaseTestExpr standing in for the arg, so the arg is effectively an operand - * of each WHEN's comparison. Those WHEN operators are always the type-default - * "=", matching the grouping eqop, so only a collation conflict is possible + * CaseTestExpr standing in for the arg. If the arg is a Var (after looking + * through RelabelType), it is bound in ctx->case_var while the WHEN + * conditions are walked and each CaseTestExpr is resolved to it, so the Var + * is checked exactly as each WHEN uses it. Any other arg is walked once as + * a non-operand and its CaseTestExprs are ignored, as is the one in an + * ArrayCoerceExpr's elemexpr. A CaseTestExpr from any other source is + * resolved to case_var too, which can at worst report a conflict that is not * there. */ static bool @@ -6464,54 +6472,71 @@ grouping_conflict_walker(Node *node, grouping_walker_ctx *ctx) } return false; } + else if (IsA(node, CaseTestExpr)) + { + /* + * A direct operand of a comparison is handled by + * grouping_check_operand; any other use is a non-operand reference to + * the Var it stands for, if any. + */ + return grouping_conflict_walker((Node *) ctx->case_var, ctx); + } + else if (IsA(node, ArrayCoerceExpr)) + { + ArrayCoerceExpr *acexpr = (ArrayCoerceExpr *) node; + Var *save_case_var = ctx->case_var; + bool result; + + if (grouping_conflict_walker((Node *) acexpr->arg, ctx)) + return true; + + /* The CaseTestExpr in elemexpr is an array element, not case_var. */ + ctx->case_var = NULL; + result = grouping_conflict_walker((Node *) acexpr->elemexpr, ctx); + ctx->case_var = save_case_var; + return result; + } else if (IsA(node, CaseExpr) && ((CaseExpr *) node)->arg != NULL) { CaseExpr *cexpr = (CaseExpr *) node; Node *arg = (Node *) cexpr->arg; + Var *save_case_var = ctx->case_var; + bool result = false; /* Look through RelabelType to find a direct Var arg. */ while (arg && IsA(arg, RelabelType)) arg = (Node *) ((RelabelType *) arg)->arg; + /* + * A Var arg needs no walk of its own: each WHEN condition refers to + * it through a CaseTestExpr, which is resolved to the Var and checked + * as the WHEN uses it. Any other arg is a non-operand reference in + * its own right: walk it once here and ignore its CaseTestExprs. + */ if (arg && IsA(arg, Var)) + ctx->case_var = (Var *) arg; + else { - Var *var = (Var *) arg; - - /* - * The arg is a grouping column compared by every WHEN. For a - * nondeterministic collation, reject if any WHEN applies a - * different collation. - */ - if (OidIsValid(ctx->get_eqop(var, ctx->cb_context)) && - OidIsValid(var->varcollid) && - !get_collation_isdeterministic(var->varcollid)) + if (grouping_conflict_walker(arg, ctx)) + return true; + ctx->case_var = NULL; + } + foreach_node(CaseWhen, cw, cexpr->args) + { + if (grouping_conflict_walker((Node *) cw->expr, ctx)) { - foreach_node(CaseWhen, cw, cexpr->args) - { - Oid collid = exprInputCollation((Node *) cw->expr); - - if (OidIsValid(collid) && collid != var->varcollid) - return true; - } + result = true; + break; } } - else if (grouping_conflict_walker((Node *) cexpr->arg, ctx)) - { - /* arg is a complex expression; walked as a non-operand */ + ctx->case_var = save_case_var; + if (result) return true; - } - /* - * Walk the WHEN conditions, their results, and the default result as - * non-operands. The WHEN conditions hold a CaseTestExpr in place of - * the arg, so they contribute no grouping operand of their own, but - * the condition expression or the substitution result may reference - * another grouping column. - */ + /* The results and the default result contain no CaseTestExpr. */ foreach_node(CaseWhen, cw, cexpr->args) { - if (grouping_conflict_walker((Node *) cw->expr, ctx) || - grouping_conflict_walker((Node *) cw->result, ctx)) + if (grouping_conflict_walker((Node *) cw->result, ctx)) return true; } return grouping_conflict_walker((Node *) cexpr->defresult, ctx); @@ -6544,12 +6569,13 @@ grouping_check_operands(Oid opno, Oid inputcollid, List *args, * Handle one operand 'arg' of a comparison with operator 'opno' and * collation 'inputcollid'. * - * If 'arg' is a grouping column (after looking through RelabelType), verify - * that comparison's operator has equality semantics compatible with the - * grouping eqop and, for a nondeterministic collation, that it uses the same - * collation; such a direct operand is then fully handled and is not recursed - * into. Any other operand is walked normally, so a grouping column buried - * inside it is seen as a non-operand reference. + * If 'arg' is a grouping column (after looking through RelabelType, or through + * a CaseTestExpr to the Var it stands for), verify that comparison's operator + * has equality semantics compatible with the grouping eqop and, for a + * nondeterministic collation, that it uses the same collation; such a direct + * operand is then fully handled and is not recursed into. Any other operand + * is walked normally, so a grouping column buried inside it is seen as a + * non-operand reference. */ static bool grouping_check_operand(Node *arg, Oid opno, Oid inputcollid, @@ -6560,6 +6586,9 @@ grouping_check_operand(Node *arg, Oid opno, Oid inputcollid, while (node && IsA(node, RelabelType)) node = (Node *) ((RelabelType *) node)->arg; + if (node && IsA(node, CaseTestExpr)) + node = (Node *) ctx->case_var; + if (node && IsA(node, Var)) { Var *var = (Var *) node; diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out index ce0ff764417..cf295d56507 100644 --- a/src/test/regress/expected/subselect.out +++ b/src/test/regress/expected/subselect.out @@ -2124,6 +2124,33 @@ WHERE a = ROW(1.0)::t_rec; 1 | (1.00) (1 row) +-- Simple CASE: the arg is compared by each WHEN, so the same rules apply. +-- The relabeled arg is compared by oid's "=", not the grouping eqop. +EXPLAIN (COSTS OFF) +SELECT * FROM (SELECT DISTINCT id FROM pdt) s +WHERE (CASE id::oid WHEN 1 THEN 1 ELSE 0 END) = 1; + QUERY PLAN +------------------------------------------------------------------ + Subquery Scan on s + Filter: (CASE (s.id)::oid WHEN '1'::oid THEN 1 ELSE 0 END = 1) + -> HashAggregate + Group Key: pdt.id + -> Seq Scan on pdt +(5 rows) + +-- Positive: compatible opfamily, safe to push past the grouping +EXPLAIN (COSTS OFF) +SELECT * FROM (SELECT DISTINCT id FROM pdt) s +WHERE (CASE id WHEN 1 THEN 1 ELSE 0 END) = 1; + QUERY PLAN +-------------------------------------------------------------- + Unique + -> Sort + Sort Key: pdt.id + -> Seq Scan on pdt + Filter: (CASE id WHEN 1 THEN 1 ELSE 0 END = 1) +(5 rows) + -- Set operations: any operation other than UNION ALL groups rows by equality, -- so the same opfamily-mismatch rules apply. CREATE TEMP TABLE u1 (a t_rec); diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql index 0b18e0132aa..07438694f6e 100644 --- a/src/test/regress/sql/subselect.sql +++ b/src/test/regress/sql/subselect.sql @@ -1041,6 +1041,17 @@ WHERE a = ROW(1.0)::t_rec; SELECT * FROM (SELECT DISTINCT ON (a) id, a FROM pdt ORDER BY a, id) s WHERE a = ROW(1.0)::t_rec; +-- Simple CASE: the arg is compared by each WHEN, so the same rules apply. +-- The relabeled arg is compared by oid's "=", not the grouping eqop. +EXPLAIN (COSTS OFF) +SELECT * FROM (SELECT DISTINCT id FROM pdt) s +WHERE (CASE id::oid WHEN 1 THEN 1 ELSE 0 END) = 1; + +-- Positive: compatible opfamily, safe to push past the grouping +EXPLAIN (COSTS OFF) +SELECT * FROM (SELECT DISTINCT id FROM pdt) s +WHERE (CASE id WHEN 1 THEN 1 ELSE 0 END) = 1; + -- Set operations: any operation other than UNION ALL groups rows by equality, -- so the same opfamily-mismatch rules apply. CREATE TEMP TABLE u1 (a t_rec); -- 2.37.1 (Apple Git-137.1)