From 6760c6a2629905de4c09bc2a51f470a74302218f Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Tue, 23 Jun 2026 12:46:43 +0000 Subject: [PATCH 2/2] Catch finer comparison operators outside any opfamily The conflict check skipped any operator not in a btree/hash opfamily, which lets a user-defined comparison finer than the column's default equality push a qual across the grouping boundary and return wrong results. Gate on a boolean result type instead so equality_ops_are_compatible() treats a non-opfamily operator as a conflict. But a non-opfamily operator on an image-faithful grouping column cannot split a group (merged rows are byte-identical), so flag a direct operand only when the column is not image-faithful. This keeps common quals like name LIKE 'a%' or ip << '10/8' pushable while still catching a finer operator on numeric/record. Non-boolean operators (arithmetic) remain ignored, so g + 1 = 5 keeps pushing. --- src/backend/optimizer/util/clauses.c | 17 ++++++-- src/test/regress/expected/aggregates.out | 50 ++++++++++++++++++++++++ src/test/regress/sql/aggregates.sql | 24 ++++++++++++ 3 files changed, 87 insertions(+), 4 deletions(-) diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index 34e1265c5c8..843761c97dd 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -6578,8 +6578,16 @@ wrapped_operand_has_grouping_conflict(Node *node, grouping_walker_ctx *ctx) * equality-compatible with the callback-reported grouping eqop. * * Strips RelabelType wrappers so const-folded CollateExpr leaves don't hide - * the underlying Var. Operators not in any btree/hash opfamily are skipped - * (see the header comment on op_is_safe_index_member). + * the underlying Var. Only boolean operators can split a group, so others are + * ignored. A direct grouping Var operand conflicts only when its equality is + * not image-faithful, so that a finer operator could tell merged rows apart, + * and 'opno' is not opfamily-compatible with it. equality_ops_are_compatible() + * rejects any operator not sharing a btree/hash opfamily with the grouping + * eqop, including one in no opfamily at all, so an operator we cannot prove + * compatible is a conflict. For an image-faithful column such as an integer, + * deterministic text or inet, merged rows are byte-identical, so no operator, + * whether LIKE, network containment, or anything else, can split a group and + * the qual stays pushable. * * A wrapped operand (not a bare Var) goes to * wrapped_operand_has_grouping_conflict(): a grouping column buried in an @@ -6599,7 +6607,7 @@ comparison_has_grouping_eqop_conflict(Oid opno, List *args, { ListCell *lc; - if (!OidIsValid(opno) || !op_is_safe_index_member(opno)) + if (!OidIsValid(opno) || get_op_rettype(opno) != BOOLOID) return false; foreach(lc, args) @@ -6626,7 +6634,8 @@ comparison_has_grouping_eqop_conflict(Oid opno, List *args, if (!OidIsValid(grouping_eqop)) continue; - if (!equality_ops_are_compatible(opno, grouping_eqop)) + if (!grouping_eqop_is_image_faithful(grouping_eqop) && + !equality_ops_are_compatible(opno, grouping_eqop)) return true; } diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out index 6f653446224..ad0e908fb26 100644 --- a/src/test/regress/expected/aggregates.out +++ b/src/test/regress/expected/aggregates.out @@ -1799,6 +1799,56 @@ select x, count(*) from t_xid group by x having x::text = '1'; (4 rows) drop table t_xid; +-- A non-opfamily operator on an image-faithful grouping column cannot split a +-- group (merged rows are byte-identical), so LIKE / regex / network +-- containment stay pushable. +create temp table t_like (s text, ip inet); +insert into t_like values ('apple', '10.0.0.1'), ('apple', '10.0.0.1'), ('pear', '192.168.0.1'); +explain (costs off) +select s, count(*) from t_like group by s having s like 'a%'; + QUERY PLAN +----------------------------------------- + GroupAggregate + Group Key: s + -> Sort + Sort Key: s + -> Seq Scan on t_like + Filter: (s ~~ 'a%'::text) +(6 rows) + +explain (costs off) +select ip, count(*) from t_like group by ip having ip << '10.0.0.0/8'; + QUERY PLAN +-------------------------------------------------- + GroupAggregate + Group Key: ip + -> Sort + Sort Key: ip + -> Seq Scan on t_like + Filter: (ip << '10.0.0.0/8'::inet) +(6 rows) + +drop table t_like; +-- A non-opfamily operator finer than the grouping eqop on a non-image-faithful +-- column must stay in HAVING. (Use plpgsql so the operator is not inlined.) +create function num_image_eq(numeric, numeric) returns bool + language plpgsql immutable as $$ begin return $1::text = $2::text; end $$; +create operator === (leftarg = numeric, rightarg = numeric, function = num_image_eq); +create temp table t_op (n numeric); +insert into t_op values (1.0), (1.00), (2); +explain (costs off) +select n, count(*) from t_op group by n having n === 1.0; + QUERY PLAN +------------------------ + HashAggregate + Group Key: n + Filter: (n === 1.0) + -> Seq Scan on t_op +(4 rows) + +drop table t_op; +drop operator === (numeric, numeric); +drop function num_image_eq(numeric, numeric); -- -- Test GROUP BY ALL -- diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql index 251a6d5e4ba..8a69bf2ae57 100644 --- a/src/test/regress/sql/aggregates.sql +++ b/src/test/regress/sql/aggregates.sql @@ -654,6 +654,30 @@ explain (costs off) select x, count(*) from t_xid group by x having x::text = '1'; drop table t_xid; +-- A non-opfamily operator on an image-faithful grouping column cannot split a +-- group (merged rows are byte-identical), so LIKE / regex / network +-- containment stay pushable. +create temp table t_like (s text, ip inet); +insert into t_like values ('apple', '10.0.0.1'), ('apple', '10.0.0.1'), ('pear', '192.168.0.1'); +explain (costs off) +select s, count(*) from t_like group by s having s like 'a%'; +explain (costs off) +select ip, count(*) from t_like group by ip having ip << '10.0.0.0/8'; +drop table t_like; + +-- A non-opfamily operator finer than the grouping eqop on a non-image-faithful +-- column must stay in HAVING. (Use plpgsql so the operator is not inlined.) +create function num_image_eq(numeric, numeric) returns bool + language plpgsql immutable as $$ begin return $1::text = $2::text; end $$; +create operator === (leftarg = numeric, rightarg = numeric, function = num_image_eq); +create temp table t_op (n numeric); +insert into t_op values (1.0), (1.00), (2); +explain (costs off) +select n, count(*) from t_op group by n having n === 1.0; +drop table t_op; +drop operator === (numeric, numeric); +drop function num_image_eq(numeric, numeric); + -- -- Test GROUP BY ALL -- -- 2.43.0