agora inbox for pgsql-bugs@postgresql.org
help / color / mirror / Atom feedBUG #19649: Qual pushdown into GROUP BY subqueries ignores non-equivalence-preserving references to grouping col
9+ messages / 3 participants
[nested] [flat]
* BUG #19649: Qual pushdown into GROUP BY subqueries ignores non-equivalence-preserving references to grouping col
@ 2026-09-03 06:41 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 9+ messages in thread
From: PG Bug reporting form @ 2026-09-03 06:41 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: 303677365@qq.com
The following bug has been logged on the website:
Bug reference: 19649
Logged by: chunling qin
Email address: 303677365@qq.com
PostgreSQL version: 18.6
Operating system: 86_64
Description:
When an outer WHERE/HAVING clause references a grouping column of a GROUP BY
(or DISTINCT) subquery through a type coercion (::text, CoerceViaIO) or a
function/operator wrapper (j->>0), the qual is pushed down below the
grouping node even though the reference applies a different equivalence
relation than the grouping does. Values that the grouping considers equal —
but whose text representations differ — get separated by the pushed-down
qual, splitting one group into two halves. This produces silently wrong
results: count(*) values change, a group can emit different group keys
depending on the WHERE, and rows are lost.
The simplest proof that something is wrong: the same subquery group answers
with two different group keys under two different outer WHERE clauses —
impossible under SQL semantics, since WHERE may only select subquery output
rows, never alter them.
CREATE TABLE t(id int primary key, j jsonb);
INSERT INTO t VALUES (1,'1'),(2,'1.0');
-- jsonb 1 = 1.0, so the table has exactly ONE jsonb group with count = 2
SELECT j, c FROM (SELECT j, count(*) c FROM t GROUP BY j) s;
-- 1 | 2 (baseline: one group)
SELECT j, c FROM (SELECT j, count(*) c FROM t GROUP BY j) s WHERE j::text =
'1';
-- 1 | 1 (WRONG: count changed by WHERE)
SELECT j, c FROM (SELECT j, count(*) c FROM t GROUP BY j) s WHERE j::text =
'1.0';
-- 1.0 | 1 (WRONG: the same group, different key)
SELECT j, c FROM (SELECT j, count(*) c FROM t GROUP BY j) s WHERE j =
'1'::jsonb;
-- 1 | 2 (control: same-eqop comparison is
correct)
```
hunt@(null)=# CREATE TABLE t(id int primary key, j jsonb);
INSERT INTO t VALUES (1,'1'),(2,'1.0');
-- jsonb 1 = 1.0, so the table has exactly ONE jsonb group with count = 2
SELECT j, c FROM (SELECT j, count(*) c FROM t GROUP BY j) s;
-- 1 | 2 (baseline: one group)
SELECT j, c FROM (SELECT j, count(*) c FROM t GROUP BY j) s WHERE j::text =
'1';
-- 1 | 1 (WRONG: count changed by WHERE)
SELECT j, c FROM (SELECT j, count(*) c FROM t GROUP BY j) s WHERE j::text =
'1.0';
-- 1.0 | 1 (WRONG: the same group, different key)
SELECT j, c FROM (SELECT j, count(*) c FROM t GROUP BY j) s WHERE j =
'1'::jsonb;
-- 1 | 2 (control: same-eqop comparison is
correct)
CREATE TABLE
INSERT 0 2
j | c
---+---
1 | 2
(1 row)
j | c
---+---
1 | 1
(1 row)
j | c
-----+---
1.0 | 1
(1 row)
j | c
---+---
1 | 2
(1 row)
hunt@(null)=# select version();
version
---------------------------------------------------------------------------------------------
------------------------------------------
PostgreSQL 20devel on x86_64-pc-linux-gnu, compiled by gcc (Tencent
Compiler 12.3.1.8) 12.3.
1 20230912 (TencentOS 12.3.1.8-6), 64-bit
(1 row)
```
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: BUG #19649: Qual pushdown into GROUP BY subqueries ignores non-equivalence-preserving references to grouping col
@ 2026-09-03 21:17 Andrey Rachitskiy <pl0h0yp1@gmail.com>
parent: PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 9+ messages in thread
From: Andrey Rachitskiy @ 2026-09-03 21:17 UTC (permalink / raw)
To: 303677365@qq.com; pgsql-bugs@lists.postgresql.org; +Cc: Tender Wang <tndrwang@gmail.com>; Richard Guo <guofenglinux@gmail.com>
чт, 3 сент. 2026 г. в 18:02, PG Bug reporting form <noreply@postgresql.org>:
> The following bug has been logged on the website:
>
> Bug reference: 19649
> Logged by: chunling qin
> Email address: 303677365@qq.com
> PostgreSQL version: 18.6
> Operating system: 86_64
> Description:
>
> When an outer WHERE/HAVING clause references a grouping column of a GROUP
> BY
> (or DISTINCT) subquery through a type coercion (::text, CoerceViaIO) or a
> function/operator wrapper (j->>0), the qual is pushed down below the
> grouping node even though the reference applies a different equivalence
> relation than the grouping does. Values that the grouping considers equal —
> but whose text representations differ — get separated by the pushed-down
> qual, splitting one group into two halves. This produces silently wrong
> results: count(*) values change, a group can emit different group keys
> depending on the WHERE, and rows are lost.
>
> The simplest proof that something is wrong: the same subquery group answers
> with two different group keys under two different outer WHERE clauses —
> impossible under SQL semantics, since WHERE may only select subquery output
> rows, never alter them.
>
> CREATE TABLE t(id int primary key, j jsonb);
> INSERT INTO t VALUES (1,'1'),(2,'1.0');
> -- jsonb 1 = 1.0, so the table has exactly ONE jsonb group with count = 2
>
> SELECT j, c FROM (SELECT j, count(*) c FROM t GROUP BY j) s;
> -- 1 | 2 (baseline: one group)
>
> SELECT j, c FROM (SELECT j, count(*) c FROM t GROUP BY j) s WHERE j::text =
> '1';
> -- 1 | 1 (WRONG: count changed by WHERE)
>
> SELECT j, c FROM (SELECT j, count(*) c FROM t GROUP BY j) s WHERE j::text =
> '1.0';
> -- 1.0 | 1 (WRONG: the same group, different
> key)
>
> SELECT j, c FROM (SELECT j, count(*) c FROM t GROUP BY j) s WHERE j =
> '1'::jsonb;
> -- 1 | 2 (control: same-eqop comparison is
> correct)
>
>
> ```
> hunt@(null)=# CREATE TABLE t(id int primary key, j jsonb);
> INSERT INTO t VALUES (1,'1'),(2,'1.0');
> -- jsonb 1 = 1.0, so the table has exactly ONE jsonb group with count = 2
>
> SELECT j, c FROM (SELECT j, count(*) c FROM t GROUP BY j) s;
> -- 1 | 2 (baseline: one group)
>
> SELECT j, c FROM (SELECT j, count(*) c FROM t GROUP BY j) s WHERE j::text =
> '1';
> -- 1 | 1 (WRONG: count changed by WHERE)
>
> SELECT j, c FROM (SELECT j, count(*) c FROM t GROUP BY j) s WHERE j::text =
> '1.0';
> -- 1.0 | 1 (WRONG: the same group, different
> key)
>
> SELECT j, c FROM (SELECT j, count(*) c FROM t GROUP BY j) s WHERE j =
> '1'::jsonb;
> -- 1 | 2 (control: same-eqop comparison is
> correct)
> CREATE TABLE
> INSERT 0 2
> j | c
> ---+---
> 1 | 2
> (1 row)
>
> j | c
> ---+---
> 1 | 1
> (1 row)
>
> j | c
> -----+---
> 1.0 | 1
> (1 row)
>
> j | c
> ---+---
> 1 | 2
> (1 row)
>
> hunt@(null)=# select version();
> version
>
>
> ---------------------------------------------------------------------------------------------
> ------------------------------------------
> PostgreSQL 20devel on x86_64-pc-linux-gnu, compiled by gcc (Tencent
> Compiler 12.3.1.8) 12.3.
> 1 20230912 (TencentOS 12.3.1.8-6), 64-bit
> (1 row)
>
> ```
>
>
Hi!
Thanks for the report!
Commit 44fb59fc605 added grouping conflict checks.
That check focused on direct grouping-Var operands.
A wrapper inside a comparison operand, such as CoerceViaIO or
jsonb text extraction, was still treated as pushdown-safe for
deterministic collations.
Proposal fix
In grouping_check_operand(), keep existing direct-operand compatibility
checks.
For comparison operands that are not direct Vars, recurse into the
operand tree and apply the same opfamily/collation compatibility checks
to grouping Vars found inside wrappers.
The implementation keeps direct-Var checks in one helper
(grouping_var_has_comparison_conflict) and reuses it from the wrapper
walker (grouping_operand_has_comparison_conflict_walker).
This blocks wrapper-based finer equivalence at pushdown boundaries while
preserving existing behavior for truly direct operands.
--
Regards,
Rachitskiy Andrey
Attachments:
[text/x-patch] 0001-Fix-qual-pushdown-for-wrapped-grouping-comparisons.patch (12.4K, ../../CAB8bMiss5S2SZonboQfbTMJsNCd+k_J8AbeS-vXheVUrR_TUhQ@mail.gmail.com/3-0001-Fix-qual-pushdown-for-wrapped-grouping-comparisons.patch)
download | inline diff:
From d2ca4509dc508e2e27cb382f12761050550620e9 Mon Sep 17 00:00:00 2001
From: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Date: Thu, 3 Sep 2026 23:58:51 +0500
Subject: [PATCH] Fix qual pushdown for wrapped grouping comparisons
A grouping column used through a wrapper inside a comparison operand can
apply a finer equivalence relation than the grouping boundary uses.
For example, jsonb grouping merges 1 and 1.0, but wrappers such as
j::text or j #>> '{}' can separate them when pushed below DISTINCT or
GROUP BY.
Teach grouping_check_operand() to recurse into non-direct comparison
operands and apply the same opfamily/collation compatibility checks to
grouping Vars found inside wrappers.
Add DISTINCT ON regression cases for j::text and j #>> '{}' over jsonb
and verify that these quals stay above Unique.
Bug: #19649
Author: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Reported-by: chunling qin <303677365@qq.com>
---
src/backend/optimizer/util/clauses.c | 137 ++++++++++++++++++++----
src/test/regress/expected/subselect.out | 88 +++++++++++++++
src/test/regress/sql/subselect.sql | 38 +++++++
3 files changed, 243 insertions(+), 20 deletions(-)
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 55cebe4a74b..94198597081 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -112,6 +112,9 @@ typedef struct
grouping_eqop_callback get_eqop;
void *cb_context;
Var *case_var;
+ Oid cmp_opno;
+ Oid cmp_inputcollid;
+ bool cmp_context_valid;
} grouping_walker_ctx;
static bool contain_agg_clause_walker(Node *node, void *context);
@@ -133,6 +136,14 @@ static List *find_nonnullable_vars_walker(Node *node, bool top_level);
static bool is_strict_saop(ScalarArrayOpExpr *expr, bool falseOK);
static bool convert_saop_to_hashed_saop_walker(Node *node, void *context);
static bool grouping_conflict_walker(Node *node, grouping_walker_ctx *ctx);
+static bool grouping_var_has_comparison_conflict(Var *var, Oid opno,
+ Oid inputcollid,
+ grouping_walker_ctx *ctx);
+static bool grouping_operand_has_comparison_conflict(Node *node, Oid opno,
+ Oid inputcollid,
+ grouping_walker_ctx *ctx);
+static bool grouping_operand_has_comparison_conflict_walker(Node *node,
+ void *context);
static bool grouping_check_operands(Oid opno, Oid inputcollid,
List *args, grouping_walker_ctx *ctx);
static bool grouping_check_operand(Node *arg, Oid opno, Oid inputcollid,
@@ -6417,15 +6428,13 @@ pull_paramids_walker(Node *node, Bitmapset **context)
* For a nondeterministic collation, every other reference is rejected: a
* comparison under a different collation, and any function or operator over
* the column, because we cannot tell whether the function yields the same
- * result for values the grouping treats as equal, and many do not. A column
- * with a deterministic collation is not restricted this way.
+ * result for values the grouping treats as equal, and many do not.
*
- * This leaves one case uncaught: with a deterministic collation, a function
- * over the column can still feed a finer comparison than the direct-operand
- * check sees, for example record_image_ops over a rebuilt record, or scale()
- * over numeric where two equal values differ in scale. Catching it would
- * require knowing that a type's equality is bitwise, which we do not test
- * here.
+ * In addition, within a comparison, if an operand is not a direct grouping
+ * Var, we recurse into it and apply the same opfamily/collation checks to any
+ * grouping Vars found there. This catches wrappers such as CoerceViaIO and
+ * text-extraction operators that can feed a comparison using a different
+ * equality relation than grouping does.
*
* Returns true if any such conflict exists.
*/
@@ -6642,6 +6651,100 @@ grouping_check_operands(Oid opno, Oid inputcollid, List *args,
return false;
}
+/*
+ * grouping_var_has_comparison_conflict
+ * Apply direct-operand grouping checks to one Var.
+ *
+ * Returns true when this Var is a grouping column and the surrounding
+ * comparison would apply a conflicting equivalence relation, either because
+ * the comparison operator is from an incompatible equality family or because
+ * a nondeterministic-collation grouping Var is compared under a different
+ * collation.
+ */
+static bool
+grouping_var_has_comparison_conflict(Var *var, Oid opno, Oid inputcollid,
+ grouping_walker_ctx *ctx)
+{
+ Oid grouping_eqop = ctx->get_eqop(var, ctx->cb_context);
+
+ if (!OidIsValid(grouping_eqop))
+ return false;
+
+ if (!equality_ops_are_compatible(opno, grouping_eqop))
+ return true;
+
+ if (OidIsValid(var->varcollid) &&
+ !get_collation_isdeterministic(var->varcollid) &&
+ inputcollid != var->varcollid)
+ return true;
+
+ return false;
+}
+
+/*
+ * grouping_operand_has_comparison_conflict
+ * Recursively inspect a non-direct comparison operand.
+ *
+ * grouping_check_operand calls this only after determining that the operand is
+ * not itself a direct Var reference.
+ */
+static bool
+grouping_operand_has_comparison_conflict(Node *node, Oid opno,
+ Oid inputcollid,
+ grouping_walker_ctx *ctx)
+{
+ grouping_walker_ctx check_ctx = *ctx;
+
+ check_ctx.cmp_opno = opno;
+ check_ctx.cmp_inputcollid = inputcollid;
+ check_ctx.cmp_context_valid = true;
+
+ return grouping_operand_has_comparison_conflict_walker(node, &check_ctx);
+}
+
+/*
+ * grouping_operand_has_comparison_conflict_walker
+ * Walker for grouping_operand_has_comparison_conflict.
+ *
+ * 'context' is grouping_walker_ctx with cmp_* fields set for the surrounding
+ * comparison. We descend through wrapper structure and apply
+ * grouping_var_has_comparison_conflict to every grouping Var found in the
+ * operand subtree. CaseTestExpr is resolved through ctx->case_var,
+ * matching the CASE handling used by grouping_conflict_walker.
+ *
+ * Returns true if any grouping Var inside this operand would fail the same
+ * operator/collation checks that we use for direct operands.
+ */
+static bool
+grouping_operand_has_comparison_conflict_walker(Node *node, void *context)
+{
+ grouping_walker_ctx *ctx = (grouping_walker_ctx *) context;
+
+ if (node == NULL)
+ return false;
+
+ if (IsA(node, RelabelType))
+ return grouping_operand_has_comparison_conflict_walker(
+ (Node *) ((RelabelType *) node)->arg, context);
+
+ if (IsA(node, CaseTestExpr))
+ return grouping_operand_has_comparison_conflict_walker(
+ (Node *) ctx->case_var, context);
+
+ if (IsA(node, Var))
+ {
+ Var *var = (Var *) node;
+
+ Assert(ctx->cmp_context_valid);
+ return grouping_var_has_comparison_conflict(var, ctx->cmp_opno,
+ ctx->cmp_inputcollid, ctx);
+ }
+
+ return expression_tree_walker(node,
+ grouping_operand_has_comparison_conflict_walker,
+ context);
+}
+
/*
* grouping_check_operand
* Handle one operand 'arg' of a comparison with operator 'opno' and
@@ -6670,22 +6773,16 @@ grouping_check_operand(Node *arg, Oid opno, Oid inputcollid,
if (node && IsA(node, Var))
{
Var *var = (Var *) node;
- Oid grouping_eqop = ctx->get_eqop(var, ctx->cb_context);
- if (OidIsValid(grouping_eqop))
- {
- /* incompatible equality semantics */
- if (!equality_ops_are_compatible(opno, grouping_eqop))
- return true;
- /* nondeterministic collation compared under a different collation */
- if (OidIsValid(var->varcollid) &&
- !get_collation_isdeterministic(var->varcollid) &&
- inputcollid != var->varcollid)
- return true;
- }
+ if (grouping_var_has_comparison_conflict(var, opno, inputcollid, ctx))
+ return true;
return false; /* direct operand handled; do not recurse */
}
+ /* Recurse into non-direct operands and reuse direct checks. */
+ if (grouping_operand_has_comparison_conflict(arg, opno, inputcollid, ctx))
+ return true;
+
return grouping_conflict_walker(arg, ctx);
}
diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out
index cf295d56507..cc660286529 100644
--- a/src/test/regress/expected/subselect.out
+++ b/src/test/regress/expected/subselect.out
@@ -2151,6 +2151,94 @@ WHERE (CASE id WHEN 1 THEN 1 ELSE 0 END) = 1;
Filter: (CASE id WHEN 1 THEN 1 ELSE 0 END = 1)
(5 rows)
+-- Wrapped jsonb comparisons over grouped subqueries (DISTINCT ON and
+-- GROUP BY): wrapped quals must stay above grouping.
+CREATE TEMP TABLE pdt_json (id int, j jsonb);
+INSERT INTO pdt_json VALUES
+ (1, '1'),
+ (2, '1.0');
+EXPLAIN (COSTS OFF)
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j::text = '1.0';
+ QUERY PLAN
+-------------------------------------------------
+ Subquery Scan on s
+ Filter: ((s.j)::text = '1.0'::text)
+ -> Unique
+ -> Sort
+ Sort Key: pdt_json.j, pdt_json.id
+ -> Seq Scan on pdt_json
+(6 rows)
+
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j::text = '1.0';
+ id | j
+----+---
+(0 rows)
+
+EXPLAIN (COSTS OFF)
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j #>> '{}' = '1.0';
+ QUERY PLAN
+--------------------------------------------------
+ Subquery Scan on s
+ Filter: ((s.j #>> '{}'::text[]) = '1.0'::text)
+ -> Unique
+ -> Sort
+ Sort Key: pdt_json.j, pdt_json.id
+ -> Seq Scan on pdt_json
+(6 rows)
+
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j #>> '{}' = '1.0';
+ id | j
+----+---
+(0 rows)
+
+EXPLAIN (COSTS OFF)
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1';
+ QUERY PLAN
+--------------------------------------------------
+ Subquery Scan on s
+ -> HashAggregate
+ Group Key: pdt_json.j
+ Filter: ((pdt_json.j)::text = '1'::text)
+ -> Seq Scan on pdt_json
+(5 rows)
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1';
+ c
+---
+ 2
+(1 row)
+
+EXPLAIN (COSTS OFF)
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1.0';
+ QUERY PLAN
+----------------------------------------------------
+ Subquery Scan on s
+ -> HashAggregate
+ Group Key: pdt_json.j
+ Filter: ((pdt_json.j)::text = '1.0'::text)
+ -> Seq Scan on pdt_json
+(5 rows)
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1.0';
+ c
+---
+(0 rows)
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j = '1'::jsonb;
+ c
+---
+ 2
+(1 row)
+
-- 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 07438694f6e..ba60d9f4cc1 100644
--- a/src/test/regress/sql/subselect.sql
+++ b/src/test/regress/sql/subselect.sql
@@ -1052,6 +1052,44 @@ EXPLAIN (COSTS OFF)
SELECT * FROM (SELECT DISTINCT id FROM pdt) s
WHERE (CASE id WHEN 1 THEN 1 ELSE 0 END) = 1;
+-- Wrapped jsonb comparisons over grouped subqueries (DISTINCT ON and
+-- GROUP BY): wrapped quals must stay above grouping.
+CREATE TEMP TABLE pdt_json (id int, j jsonb);
+INSERT INTO pdt_json VALUES
+ (1, '1'),
+ (2, '1.0');
+
+EXPLAIN (COSTS OFF)
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j::text = '1.0';
+
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j::text = '1.0';
+
+EXPLAIN (COSTS OFF)
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j #>> '{}' = '1.0';
+
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j #>> '{}' = '1.0';
+
+EXPLAIN (COSTS OFF)
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1';
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1';
+
+EXPLAIN (COSTS OFF)
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1.0';
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1.0';
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j = '1'::jsonb;
+
-- 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.53.0
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: BUG #19649: Qual pushdown into GROUP BY subqueries ignores non-equivalence-preserving references to grouping col
@ 2026-09-04 15:12 Andrei Lepikhov <lepihov@gmail.com>
parent: Andrey Rachitskiy <pl0h0yp1@gmail.com>
0 siblings, 1 reply; 9+ messages in thread
From: Andrei Lepikhov @ 2026-09-04 15:12 UTC (permalink / raw)
To: Andrey Rachitskiy <pl0h0yp1@gmail.com>; 303677365@qq.com; pgsql-bugs@lists.postgresql.org; +Cc: Tender Wang <tndrwang@gmail.com>
On 03/09/2026 23:17, Andrey Rachitskiy wrote:
> This blocks wrapper-based finer equivalence at pushdown boundaries while
> preserving existing behavior for truly direct operands.
I don't like this fix. It causes regressions where we haven't had it before.
Let's see:
CREATE TABLE r(i int, s text, ts timestamptz);
EXPLAIN (COSTS OFF)
SELECT * FROM (SELECT i, count(*) c FROM r GROUP BY i) s WHERE i::text = '5';
Before:
GroupAggregate
Group Key: r.i
-> Sort
Sort Key: r.i
-> Seq Scan on r
Filter: ((i)::text = '5'::text)
With your fix:
HashAggregate
Group Key: r.i
Filter: ((r.i)::text = '5'::text)
-> Seq Scan on r
I think, filter should be pushed down to the scan.
--
regards, Andrei Lepikhov,
pgEdge
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: BUG #19649: Qual pushdown into GROUP BY subqueries ignores non-equivalence-preserving references to grouping col
@ 2026-09-04 16:10 Andrey Rachitskiy <pl0h0yp1@gmail.com>
parent: Andrei Lepikhov <lepihov@gmail.com>
0 siblings, 1 reply; 9+ messages in thread
From: Andrey Rachitskiy @ 2026-09-04 16:10 UTC (permalink / raw)
To: Andrei Lepikhov <lepihov@gmail.com>; +Cc: 303677365@qq.com, pgsql-bugs@lists.postgresql.org, Tender Wang <tndrwang@gmail.com>
пт, 4 сент. 2026 г. в 20:12, Andrei Lepikhov <lepihov@gmail.com>:
> On 03/09/2026 23:17, Andrey Rachitskiy wrote:
> > This blocks wrapper-based finer equivalence at pushdown boundaries while
> > preserving existing behavior for truly direct operands.
> I don't like this fix. It causes regressions where we haven't had it
> before.
> Let's see:
>
> CREATE TABLE r(i int, s text, ts timestamptz);
> EXPLAIN (COSTS OFF)
> SELECT * FROM (SELECT i, count(*) c FROM r GROUP BY i) s WHERE i::text =
> '5';
>
> Before:
>
> GroupAggregate
> Group Key: r.i
> -> Sort
> Sort Key: r.i
> -> Seq Scan on r
> Filter: ((i)::text = '5'::text)
>
> With your fix:
>
> HashAggregate
> Group Key: r.i
> Filter: ((r.i)::text = '5'::text)
> -> Seq Scan on r
>
> I think, filter should be pushed down to the scan.
>
> --
> regards, Andrei Lepikhov,
> pgEdge
>
Dear Andrei,
Thanks for the review.
v2 with a correction in the attachment.
I also removed the duplicate logic.
--
Regards,
Rachitskiy Andrey
Attachments:
[text/x-patch] v2-0001-Fix-qual-pushdown-for-wrapped-grouping-comparisons.patch (14.6K, ../../CAB8bMivStAVVSPonHGeP0-Uxb5bd2mgLNyBunYxHG1s+UxnKOQ@mail.gmail.com/3-v2-0001-Fix-qual-pushdown-for-wrapped-grouping-comparisons.patch)
download | inline diff:
From e1f42b1f8b1bc7e9128220f5517235004f510a87 Mon Sep 17 00:00:00 2001
From: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Date: Fri, 4 Sep 2026 20:41:04 +0500
Subject: [PATCH v2] Fix qual pushdown for wrapped grouping comparisons
A grouping column used through a wrapper inside a comparison operand can
apply a finer equivalence relation than the grouping boundary uses.
For example, jsonb grouping merges 1 and 1.0, but wrappers such as
j::text or j #>> '{}' can separate them when pushed below DISTINCT or
GROUP BY.
Keep the direct-operand checks unchanged, and recurse into non-direct
comparison operands only for wrapped jsonb grouping Vars.
Add regression cases for wrapped jsonb comparisons, and a guard case
that confirms a safe wrapper qual (i::text = '5') is still pushed down
to Seq Scan.
Bug: #19649
Author: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Reviewed-by: Andrei Lepikhov <lepihov@gmail.com>
Reported-by: chunling qin <303677365@qq.com>
---
src/backend/optimizer/util/clauses.c | 156 ++++++++++++++++++++----
src/test/regress/expected/subselect.out | 105 ++++++++++++++++
src/test/regress/sql/subselect.sql | 46 +++++++
3 files changed, 285 insertions(+), 22 deletions(-)
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 55cebe4a74b..73738679d59 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -27,6 +27,7 @@
#include "catalog/pg_operator.h"
#include "catalog/pg_proc.h"
#include "catalog/pg_type.h"
+#include "catalog/pg_type_d.h"
#include "executor/executor.h"
#include "executor/functions.h"
#include "funcapi.h"
@@ -112,6 +113,9 @@ typedef struct
grouping_eqop_callback get_eqop;
void *cb_context;
Var *case_var;
+ Oid cmp_opno;
+ Oid cmp_inputcollid;
+ bool cmp_context_valid;
} grouping_walker_ctx;
static bool contain_agg_clause_walker(Node *node, void *context);
@@ -133,6 +137,15 @@ static List *find_nonnullable_vars_walker(Node *node, bool top_level);
static bool is_strict_saop(ScalarArrayOpExpr *expr, bool falseOK);
static bool convert_saop_to_hashed_saop_walker(Node *node, void *context);
static bool grouping_conflict_walker(Node *node, grouping_walker_ctx *ctx);
+static bool grouping_var_has_nondeterministic_collation(Var *var);
+static bool grouping_var_has_comparison_conflict(Var *var, Oid opno,
+ Oid inputcollid,
+ grouping_walker_ctx *ctx);
+static bool grouping_operand_has_comparison_conflict(Node *node, Oid opno,
+ Oid inputcollid,
+ grouping_walker_ctx *ctx);
+static bool grouping_operand_has_comparison_conflict_walker(Node *node,
+ void *context);
static bool grouping_check_operands(Oid opno, Oid inputcollid,
List *args, grouping_walker_ctx *ctx);
static bool grouping_check_operand(Node *arg, Oid opno, Oid inputcollid,
@@ -6417,15 +6430,13 @@ pull_paramids_walker(Node *node, Bitmapset **context)
* For a nondeterministic collation, every other reference is rejected: a
* comparison under a different collation, and any function or operator over
* the column, because we cannot tell whether the function yields the same
- * result for values the grouping treats as equal, and many do not. A column
- * with a deterministic collation is not restricted this way.
+ * result for values the grouping treats as equal, and many do not.
*
- * This leaves one case uncaught: with a deterministic collation, a function
- * over the column can still feed a finer comparison than the direct-operand
- * check sees, for example record_image_ops over a rebuilt record, or scale()
- * over numeric where two equal values differ in scale. Catching it would
- * require knowing that a type's equality is bitwise, which we do not test
- * here.
+ * In addition, within a comparison, if an operand is not a direct grouping
+ * Var, we recurse into it and apply the same opfamily/collation checks to
+ * wrapped jsonb grouping Vars found there. This catches wrappers such as
+ * CoerceViaIO and text-extraction operators over jsonb that can feed a
+ * comparison using a different equality relation than grouping does.
*
* Returns true if any such conflict exists.
*/
@@ -6468,6 +6479,13 @@ expression_has_grouping_conflict(Node *expr,
* ArrayCoerceExpr's elemexpr and a JsonConstructorExpr's coercion, which
* stand for something else.
*/
+static bool
+grouping_var_has_nondeterministic_collation(Var *var)
+{
+ return OidIsValid(var->varcollid) &&
+ !get_collation_isdeterministic(var->varcollid);
+}
+
static bool
grouping_conflict_walker(Node *node, grouping_walker_ctx *ctx)
{
@@ -6487,8 +6505,7 @@ grouping_conflict_walker(Node *node, grouping_walker_ctx *ctx)
* boolean is not collatable, so it takes the deterministic path here.
*/
if (OidIsValid(ctx->get_eqop(var, ctx->cb_context)) &&
- OidIsValid(var->varcollid) &&
- !get_collation_isdeterministic(var->varcollid))
+ grouping_var_has_nondeterministic_collation(var))
return true;
return false;
}
@@ -6642,6 +6659,107 @@ grouping_check_operands(Oid opno, Oid inputcollid, List *args,
return false;
}
+/*
+ * grouping_var_has_comparison_conflict
+ * Apply direct-operand grouping checks to one Var.
+ *
+ * Returns true when this Var is a grouping column and the surrounding
+ * comparison would apply a conflicting equivalence relation, either because
+ * the comparison operator is from an incompatible equality family or because
+ * a nondeterministic-collation grouping Var is compared under a different
+ * collation.
+ */
+static bool
+grouping_var_has_comparison_conflict(Var *var, Oid opno, Oid inputcollid,
+ grouping_walker_ctx *ctx)
+{
+ Oid grouping_eqop = ctx->get_eqop(var, ctx->cb_context);
+
+ if (!OidIsValid(grouping_eqop))
+ return false;
+
+ if (!equality_ops_are_compatible(opno, grouping_eqop))
+ return true;
+
+ if (grouping_var_has_nondeterministic_collation(var) &&
+ inputcollid != var->varcollid)
+ return true;
+
+ return false;
+}
+
+/*
+ * grouping_operand_has_comparison_conflict
+ * Recursively inspect a non-direct comparison operand.
+ *
+ * grouping_check_operand calls this only after determining that the operand is
+ * not itself a direct Var reference.
+ */
+static bool
+grouping_operand_has_comparison_conflict(Node *node, Oid opno,
+ Oid inputcollid,
+ grouping_walker_ctx *ctx)
+{
+ grouping_walker_ctx check_ctx = *ctx;
+
+ check_ctx.cmp_opno = opno;
+ check_ctx.cmp_inputcollid = inputcollid;
+ check_ctx.cmp_context_valid = true;
+
+ return grouping_operand_has_comparison_conflict_walker(node, &check_ctx);
+}
+
+/*
+ * grouping_operand_has_comparison_conflict_walker
+ * Walker for grouping_operand_has_comparison_conflict.
+ *
+ * 'context' is grouping_walker_ctx with cmp_* fields set for the surrounding
+ * comparison. We descend through wrapper structure and apply
+ * grouping_var_has_comparison_conflict to every grouping Var found in the
+ * operand subtree. CaseTestExpr is resolved through ctx->case_var,
+ * matching the CASE handling used by grouping_conflict_walker.
+ *
+ * Returns true if any wrapped jsonb grouping Var inside this operand would
+ * fail the same operator/collation checks that we use for direct operands.
+ */
+static bool
+grouping_operand_has_comparison_conflict_walker(Node *node, void *context)
+{
+ grouping_walker_ctx *ctx = (grouping_walker_ctx *) context;
+
+ if (node == NULL)
+ return false;
+
+ if (IsA(node, RelabelType))
+ return grouping_operand_has_comparison_conflict_walker(
+ (Node *) ((RelabelType *) node)->arg, context);
+
+ if (IsA(node, CaseTestExpr))
+ return grouping_operand_has_comparison_conflict_walker(
+ (Node *) ctx->case_var, context);
+
+ if (IsA(node, Var))
+ {
+ Var *var = (Var *) node;
+
+ /*
+ * Keep this check narrow: v1's all-type recursion blocked safe
+ * pushdown such as int4->text wrappers. The known broken class here
+ * is wrappers over jsonb grouping keys.
+ */
+ if (getBaseType(var->vartype) != JSONBOID)
+ return false;
+
+ Assert(ctx->cmp_context_valid);
+ return grouping_var_has_comparison_conflict(var, ctx->cmp_opno,
+ ctx->cmp_inputcollid, ctx);
+ }
+
+ return expression_tree_walker(node,
+ grouping_operand_has_comparison_conflict_walker,
+ context);
+}
+
/*
* grouping_check_operand
* Handle one operand 'arg' of a comparison with operator 'opno' and
@@ -6670,22 +6788,16 @@ grouping_check_operand(Node *arg, Oid opno, Oid inputcollid,
if (node && IsA(node, Var))
{
Var *var = (Var *) node;
- Oid grouping_eqop = ctx->get_eqop(var, ctx->cb_context);
- if (OidIsValid(grouping_eqop))
- {
- /* incompatible equality semantics */
- if (!equality_ops_are_compatible(opno, grouping_eqop))
- return true;
- /* nondeterministic collation compared under a different collation */
- if (OidIsValid(var->varcollid) &&
- !get_collation_isdeterministic(var->varcollid) &&
- inputcollid != var->varcollid)
- return true;
- }
+ if (grouping_var_has_comparison_conflict(var, opno, inputcollid, ctx))
+ return true;
return false; /* direct operand handled; do not recurse */
}
+ /* Recurse into non-direct operands and reuse direct checks. */
+ if (grouping_operand_has_comparison_conflict(arg, opno, inputcollid, ctx))
+ return true;
+
return grouping_conflict_walker(arg, ctx);
}
diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out
index cf295d56507..ae3e76b856f 100644
--- a/src/test/regress/expected/subselect.out
+++ b/src/test/regress/expected/subselect.out
@@ -2151,6 +2151,111 @@ WHERE (CASE id WHEN 1 THEN 1 ELSE 0 END) = 1;
Filter: (CASE id WHEN 1 THEN 1 ELSE 0 END = 1)
(5 rows)
+-- Wrapped jsonb comparisons over grouped subqueries (DISTINCT ON and
+-- GROUP BY): wrapped quals must stay above grouping.
+CREATE TEMP TABLE pdt_json (id int, j jsonb);
+INSERT INTO pdt_json VALUES
+ (1, '1'),
+ (2, '1.0');
+EXPLAIN (COSTS OFF)
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j::text = '1.0';
+ QUERY PLAN
+-------------------------------------------------
+ Subquery Scan on s
+ Filter: ((s.j)::text = '1.0'::text)
+ -> Unique
+ -> Sort
+ Sort Key: pdt_json.j, pdt_json.id
+ -> Seq Scan on pdt_json
+(6 rows)
+
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j::text = '1.0';
+ id | j
+----+---
+(0 rows)
+
+EXPLAIN (COSTS OFF)
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j #>> '{}' = '1.0';
+ QUERY PLAN
+--------------------------------------------------
+ Subquery Scan on s
+ Filter: ((s.j #>> '{}'::text[]) = '1.0'::text)
+ -> Unique
+ -> Sort
+ Sort Key: pdt_json.j, pdt_json.id
+ -> Seq Scan on pdt_json
+(6 rows)
+
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j #>> '{}' = '1.0';
+ id | j
+----+---
+(0 rows)
+
+EXPLAIN (COSTS OFF)
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1';
+ QUERY PLAN
+--------------------------------------------------
+ Subquery Scan on s
+ -> HashAggregate
+ Group Key: pdt_json.j
+ Filter: ((pdt_json.j)::text = '1'::text)
+ -> Seq Scan on pdt_json
+(5 rows)
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1';
+ c
+---
+ 2
+(1 row)
+
+EXPLAIN (COSTS OFF)
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1.0';
+ QUERY PLAN
+----------------------------------------------------
+ Subquery Scan on s
+ -> HashAggregate
+ Group Key: pdt_json.j
+ Filter: ((pdt_json.j)::text = '1.0'::text)
+ -> Seq Scan on pdt_json
+(5 rows)
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1.0';
+ c
+---
+(0 rows)
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j = '1'::jsonb;
+ c
+---
+ 2
+(1 row)
+
+-- But safe wrappers such as int4->text must still push below grouping.
+CREATE TEMP TABLE pdt_int (i int, s text, ts timestamptz);
+SET enable_hashagg TO off;
+EXPLAIN (COSTS OFF)
+SELECT * FROM (SELECT i, count(*) c FROM pdt_int GROUP BY i) s
+WHERE i::text = '5';
+ QUERY PLAN
+-----------------------------------------------
+ GroupAggregate
+ Group Key: pdt_int.i
+ -> Sort
+ Sort Key: pdt_int.i
+ -> Seq Scan on pdt_int
+ Filter: ((i)::text = '5'::text)
+(6 rows)
+
+RESET enable_hashagg;
-- 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 07438694f6e..01937ae249c 100644
--- a/src/test/regress/sql/subselect.sql
+++ b/src/test/regress/sql/subselect.sql
@@ -1052,6 +1052,52 @@ EXPLAIN (COSTS OFF)
SELECT * FROM (SELECT DISTINCT id FROM pdt) s
WHERE (CASE id WHEN 1 THEN 1 ELSE 0 END) = 1;
+-- Wrapped jsonb comparisons over grouped subqueries (DISTINCT ON and
+-- GROUP BY): wrapped quals must stay above grouping.
+CREATE TEMP TABLE pdt_json (id int, j jsonb);
+INSERT INTO pdt_json VALUES
+ (1, '1'),
+ (2, '1.0');
+
+EXPLAIN (COSTS OFF)
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j::text = '1.0';
+
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j::text = '1.0';
+
+EXPLAIN (COSTS OFF)
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j #>> '{}' = '1.0';
+
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j #>> '{}' = '1.0';
+
+EXPLAIN (COSTS OFF)
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1';
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1';
+
+EXPLAIN (COSTS OFF)
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1.0';
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1.0';
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j = '1'::jsonb;
+
+-- But safe wrappers such as int4->text must still push below grouping.
+CREATE TEMP TABLE pdt_int (i int, s text, ts timestamptz);
+SET enable_hashagg TO off;
+EXPLAIN (COSTS OFF)
+SELECT * FROM (SELECT i, count(*) c FROM pdt_int GROUP BY i) s
+WHERE i::text = '5';
+RESET enable_hashagg;
+
-- 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.53.0
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: BUG #19649: Qual pushdown into GROUP BY subqueries ignores non-equivalence-preserving references to grouping col
@ 2026-09-05 07:52 Andrey Rachitskiy <pl0h0yp1@gmail.com>
parent: Andrey Rachitskiy <pl0h0yp1@gmail.com>
0 siblings, 1 reply; 9+ messages in thread
From: Andrey Rachitskiy @ 2026-09-05 07:52 UTC (permalink / raw)
To: Andrei Lepikhov <lepihov@gmail.com>; +Cc: 303677365@qq.com, pgsql-bugs@lists.postgresql.org, Tender Wang <tndrwang@gmail.com>
пт, 4 сент. 2026 г. в 21:10, Andrey Rachitskiy <pl0h0yp1@gmail.com>:
>
> пт, 4 сент. 2026 г. в 20:12, Andrei Lepikhov <lepihov@gmail.com>:
>
>> On 03/09/2026 23:17, Andrey Rachitskiy wrote:
>> > This blocks wrapper-based finer equivalence at pushdown boundaries while
>> > preserving existing behavior for truly direct operands.
>> I don't like this fix. It causes regressions where we haven't had it
>> before.
>> Let's see:
>>
>> CREATE TABLE r(i int, s text, ts timestamptz);
>> EXPLAIN (COSTS OFF)
>> SELECT * FROM (SELECT i, count(*) c FROM r GROUP BY i) s WHERE i::text =
>> '5';
>>
>> Before:
>>
>> GroupAggregate
>> Group Key: r.i
>> -> Sort
>> Sort Key: r.i
>> -> Seq Scan on r
>> Filter: ((i)::text = '5'::text)
>>
>> With your fix:
>>
>> HashAggregate
>> Group Key: r.i
>> Filter: ((r.i)::text = '5'::text)
>> -> Seq Scan on r
>>
>> I think, filter should be pushed down to the scan.
>>
>> --
>> regards, Andrei Lepikhov,
>> pgEdge
>>
>
> Dear Andrei,
>
> Thanks for the review.
>
> v2 with a correction in the attachment.
> I also removed the duplicate logic.
>
>
In v2, I did not account for the GROUP BY case.
44fb59fc605 checks it later in
find_having_conflicts, after the qual is pushed into HAVING.
ReplaceVarsFromTargetList copies a GROUP Var from the tlist, so the
walker can keep a wrapped jsonb qual on the Agg node.
Without the wrapper check the qual moves to WHERE. The reporter's
j::text filters then turn count(*) from 2 into 1.
This update also looks at groupClause in qual_is_pushdown_safe, so the
qual is not pushed. The filter stays on Subquery Scan, as for
DISTINCT.
v3 in attachment.
P.S. I'm still learning plans and could be mistaken, so please don't judge
too harshly.
Attachments:
[text/x-patch] v3-0001-Fix-qual-pushdown-for-wrapped-grouping-comparisons.patch (21.1K, ../../CAB8bMivO2qD9obEMdPxDpQ+Zanq_e_fHA2c1tKe=YeRxjnbXWA@mail.gmail.com/3-v3-0001-Fix-qual-pushdown-for-wrapped-grouping-comparisons.patch)
download | inline diff:
From e1f42b1f8b1bc7e9128220f5517235004f510a87 Mon Sep 17 00:00:00 2001
From: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Date: Fri, 4 Sep 2026 20:41:04 +0500
Subject: [PATCH v3] Fix qual pushdown for wrapped grouping comparisons
A grouping column used through a wrapper inside a comparison operand can
apply a finer equivalence relation than the grouping boundary uses.
For example, jsonb grouping merges 1 and 1.0, but wrappers such as
j::text or j #>> '{}' can separate them when pushed below DISTINCT or
GROUP BY.
Keep the direct-operand checks unchanged, and recurse into non-direct
comparison operands only for wrapped jsonb grouping Vars.
Also check GROUP BY in qual_is_pushdown_safe, so a wrapped jsonb qual
is not pushed below aggregation.
Bug: #19649
Author: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Reviewed-by: Andrei Lepikhov <lepihov@gmail.com>
Reported-by: chunling qin <303677365@qq.com>
---
src/backend/optimizer/path/allpaths.c | 52 ++++++-----
src/backend/optimizer/util/clauses.c | 152 +++++++++++++++++++++++++-----
src/test/regress/expected/subselect.out | 160 ++++++++++++++++++++++++++++++++
src/test/regress/sql/subselect.sql | 70 ++++++++++++++
4 files changed, 387 insertions(+), 47 deletions(-)
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 24a6a8d11dd..29556ca857e 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -4445,15 +4445,15 @@ targetIsInAllPartitionLists(TargetEntry *tle, Query *query)
* 5. rinfo's clause must not refer to any subquery output columns that were
* found to be unsafe to reference by subquery_is_pushdown_safe().
*
- * 6. If the subquery has a grouping layer (DISTINCT, DISTINCT ON, window
- * PARTITION BY, or a set operation that groups rows by equality), rinfo's
- * clause must not apply a different equivalence relation to a grouping column
- * than the grouping uses; otherwise it would distinguish rows the grouping
- * considers equal, and pushing such a clause past the grouping would drop
- * members of a group and change which row becomes the group's representative
- * (or, for window functions, change per-partition values such as ranks and
- * counts). See expression_has_grouping_conflict for the kinds of conflict
- * detected.
+ * 6. If the subquery has a grouping layer (GROUP BY, DISTINCT, DISTINCT ON,
+ * window PARTITION BY, or a set operation that groups rows by equality),
+ * rinfo's clause must not apply a different equivalence relation to a
+ * grouping column than the grouping uses. Otherwise it would distinguish
+ * rows the grouping considers equal, and pushing such a clause past the
+ * grouping would drop members of a group and change which row becomes the
+ * group's representative (or, for window functions, change per-partition
+ * values such as ranks and counts). See expression_has_grouping_conflict
+ * for the kinds of conflict detected.
*/
static pushdown_safe_type
qual_is_pushdown_safe(Query *subquery, Index rti, RestrictInfo *rinfo,
@@ -4552,7 +4552,8 @@ qual_is_pushdown_safe(Query *subquery, Index rti, RestrictInfo *rinfo,
/* Check point 6 */
if (safe == PUSHDOWN_SAFE &&
- (subquery->hasWindowFuncs ||
+ (subquery->groupClause != NIL ||
+ subquery->hasWindowFuncs ||
subquery->distinctClause != NIL ||
(subquery->setOperations != NULL &&
setop_has_grouping(subquery->setOperations))))
@@ -4579,20 +4580,11 @@ static Oid
pushdown_var_grouping_eqop(Var *var, void *context)
{
Query *subquery = (Query *) context;
- Oid eqop;
if (var->varlevelsup != 0)
return InvalidOid;
- eqop = subquery_column_grouping_eqop(subquery, var->varattno);
-
- /*
- * qual_is_pushdown_safe ensures any level-0 subquery Var that reaches us
- * references a grouping column.
- */
- Assert(OidIsValid(eqop));
-
- return eqop;
+ return subquery_column_grouping_eqop(subquery, var->varattno);
}
/*
@@ -4602,11 +4594,12 @@ pushdown_var_grouping_eqop(Var *var, void *context)
* participate in any grouping mechanism.
*
* A subquery output column is grouping-relevant if it appears in
- * subquery->distinctClause (covering both DISTINCT and DISTINCT ON), in every
- * window's PARTITION BY clause, or is grouped by some node in a set-operation
- * tree. In all of these cases the parser builds the SortGroupClause with the
- * column's type-default equality operator via get_sort_group_operators, so any
- * matching SortGroupClause carries the correct eqop.
+ * subquery->groupClause, subquery->distinctClause (covering both DISTINCT and
+ * DISTINCT ON), in every window's PARTITION BY clause, or is grouped by some
+ * node in a set-operation tree. In all of these cases the parser builds the
+ * SortGroupClause with the column's type-default equality operator via
+ * get_sort_group_operators, so any matching SortGroupClause carries the
+ * correct eqop. Aggregate output columns are not grouping-relevant.
*/
static Oid
subquery_column_grouping_eqop(Query *subquery, AttrNumber attno)
@@ -4619,6 +4612,15 @@ subquery_column_grouping_eqop(Query *subquery, AttrNumber attno)
tle = list_nth_node(TargetEntry, subquery->targetList, attno - 1);
+ /* GROUP BY */
+ foreach(lc, subquery->groupClause)
+ {
+ SortGroupClause *sgc = lfirst_node(SortGroupClause, lc);
+
+ if (sgc->tleSortGroupRef == tle->ressortgroupref)
+ return sgc->eqop;
+ }
+
/* DISTINCT or DISTINCT ON */
foreach(lc, subquery->distinctClause)
{
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 55cebe4a74b..f161b38e702 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -27,6 +27,7 @@
#include "catalog/pg_operator.h"
#include "catalog/pg_proc.h"
#include "catalog/pg_type.h"
+#include "catalog/pg_type_d.h"
#include "executor/executor.h"
#include "executor/functions.h"
#include "funcapi.h"
@@ -112,6 +113,9 @@ typedef struct
grouping_eqop_callback get_eqop;
void *cb_context;
Var *case_var;
+ Oid cmp_opno;
+ Oid cmp_inputcollid;
+ bool cmp_context_valid;
} grouping_walker_ctx;
static bool contain_agg_clause_walker(Node *node, void *context);
@@ -133,6 +137,15 @@ static List *find_nonnullable_vars_walker(Node *node, bool top_level);
static bool is_strict_saop(ScalarArrayOpExpr *expr, bool falseOK);
static bool convert_saop_to_hashed_saop_walker(Node *node, void *context);
static bool grouping_conflict_walker(Node *node, grouping_walker_ctx *ctx);
+static bool grouping_var_has_nondeterministic_collation(Var *var);
+static bool grouping_var_has_comparison_conflict(Var *var, Oid opno,
+ Oid inputcollid,
+ grouping_walker_ctx *ctx);
+static bool grouping_operand_has_comparison_conflict(Node *node, Oid opno,
+ Oid inputcollid,
+ grouping_walker_ctx *ctx);
+static bool grouping_operand_has_comparison_conflict_walker(Node *node,
+ void *context);
static bool grouping_check_operands(Oid opno, Oid inputcollid,
List *args, grouping_walker_ctx *ctx);
static bool grouping_check_operand(Node *arg, Oid opno, Oid inputcollid,
@@ -6417,15 +6430,13 @@ pull_paramids_walker(Node *node, Bitmapset **context)
* For a nondeterministic collation, every other reference is rejected: a
* comparison under a different collation, and any function or operator over
* the column, because we cannot tell whether the function yields the same
- * result for values the grouping treats as equal, and many do not. A column
- * with a deterministic collation is not restricted this way.
+ * result for values the grouping treats as equal, and many do not.
*
- * This leaves one case uncaught: with a deterministic collation, a function
- * over the column can still feed a finer comparison than the direct-operand
- * check sees, for example record_image_ops over a rebuilt record, or scale()
- * over numeric where two equal values differ in scale. Catching it would
- * require knowing that a type's equality is bitwise, which we do not test
- * here.
+ * In addition, within a comparison, if an operand is not a direct grouping
+ * Var, we recurse into it and apply the same opfamily/collation checks to
+ * wrapped jsonb grouping Vars found there. This catches wrappers such as
+ * CoerceViaIO and text-extraction operators over jsonb that can feed a
+ * comparison using a different equality relation than grouping does.
*
* Returns true if any such conflict exists.
*/
@@ -6468,6 +6479,13 @@ expression_has_grouping_conflict(Node *expr,
* ArrayCoerceExpr's elemexpr and a JsonConstructorExpr's coercion, which
* stand for something else.
*/
+static bool
+grouping_var_has_nondeterministic_collation(Var *var)
+{
+ return OidIsValid(var->varcollid) &&
+ !get_collation_isdeterministic(var->varcollid);
+}
+
static bool
grouping_conflict_walker(Node *node, grouping_walker_ctx *ctx)
{
@@ -6487,8 +6505,7 @@ grouping_conflict_walker(Node *node, grouping_walker_ctx *ctx)
* boolean is not collatable, so it takes the deterministic path here.
*/
if (OidIsValid(ctx->get_eqop(var, ctx->cb_context)) &&
- OidIsValid(var->varcollid) &&
- !get_collation_isdeterministic(var->varcollid))
+ grouping_var_has_nondeterministic_collation(var))
return true;
return false;
}
@@ -6642,6 +6659,103 @@ grouping_check_operands(Oid opno, Oid inputcollid, List *args,
return false;
}
+/*
+ * grouping_var_has_comparison_conflict
+ * Apply direct-operand grouping checks to one Var.
+ *
+ * Returns true when this Var is a grouping column and the surrounding
+ * comparison would apply a conflicting equivalence relation, either because
+ * the comparison operator is from an incompatible equality family or because
+ * a nondeterministic-collation grouping Var is compared under a different
+ * collation.
+ */
+static bool
+grouping_var_has_comparison_conflict(Var *var, Oid opno, Oid inputcollid,
+ grouping_walker_ctx *ctx)
+{
+ Oid grouping_eqop = ctx->get_eqop(var, ctx->cb_context);
+
+ if (!OidIsValid(grouping_eqop))
+ return false;
+
+ if (!equality_ops_are_compatible(opno, grouping_eqop))
+ return true;
+
+ if (grouping_var_has_nondeterministic_collation(var) &&
+ inputcollid != var->varcollid)
+ return true;
+
+ return false;
+}
+
+/*
+ * grouping_operand_has_comparison_conflict
+ * Recursively inspect a non-direct comparison operand.
+ *
+ * grouping_check_operand calls this only after determining that the operand is
+ * not itself a direct Var reference.
+ */
+static bool
+grouping_operand_has_comparison_conflict(Node *node, Oid opno,
+ Oid inputcollid,
+ grouping_walker_ctx *ctx)
+{
+ grouping_walker_ctx check_ctx = *ctx;
+
+ check_ctx.cmp_opno = opno;
+ check_ctx.cmp_inputcollid = inputcollid;
+ check_ctx.cmp_context_valid = true;
+
+ return grouping_operand_has_comparison_conflict_walker(node, &check_ctx);
+}
+
+/*
+ * grouping_operand_has_comparison_conflict_walker
+ * Walker for grouping_operand_has_comparison_conflict.
+ *
+ * 'context' is grouping_walker_ctx with cmp_* fields set for the surrounding
+ * comparison. We descend through wrapper structure and apply
+ * grouping_var_has_comparison_conflict to every grouping Var found in the
+ * operand subtree. CaseTestExpr is resolved through ctx->case_var,
+ * matching the CASE handling used by grouping_conflict_walker.
+ *
+ * Returns true if any wrapped jsonb grouping Var inside this operand would
+ * fail the same operator/collation checks that we use for direct operands.
+ */
+static bool
+grouping_operand_has_comparison_conflict_walker(Node *node, void *context)
+{
+ grouping_walker_ctx *ctx = (grouping_walker_ctx *) context;
+
+ if (node == NULL)
+ return false;
+
+ if (IsA(node, RelabelType))
+ return grouping_operand_has_comparison_conflict_walker(
+ (Node *) ((RelabelType *) node)->arg, context);
+
+ if (IsA(node, CaseTestExpr))
+ return grouping_operand_has_comparison_conflict_walker(
+ (Node *) ctx->case_var, context);
+
+ if (IsA(node, Var))
+ {
+ Var *var = (Var *) node;
+
+ /* jsonb only. The same check on every type would block safe wrappers. */
+ if (getBaseType(var->vartype) != JSONBOID)
+ return false;
+
+ Assert(ctx->cmp_context_valid);
+ return grouping_var_has_comparison_conflict(var, ctx->cmp_opno,
+ ctx->cmp_inputcollid, ctx);
+ }
+
+ return expression_tree_walker(node,
+ grouping_operand_has_comparison_conflict_walker,
+ context);
+}
+
/*
* grouping_check_operand
* Handle one operand 'arg' of a comparison with operator 'opno' and
@@ -6670,22 +6784,16 @@ grouping_check_operand(Node *arg, Oid opno, Oid inputcollid,
if (node && IsA(node, Var))
{
Var *var = (Var *) node;
- Oid grouping_eqop = ctx->get_eqop(var, ctx->cb_context);
- if (OidIsValid(grouping_eqop))
- {
- /* incompatible equality semantics */
- if (!equality_ops_are_compatible(opno, grouping_eqop))
- return true;
- /* nondeterministic collation compared under a different collation */
- if (OidIsValid(var->varcollid) &&
- !get_collation_isdeterministic(var->varcollid) &&
- inputcollid != var->varcollid)
- return true;
- }
+ if (grouping_var_has_comparison_conflict(var, opno, inputcollid, ctx))
+ return true;
return false; /* direct operand handled; do not recurse */
}
+ /* Recurse into non-direct operands and reuse direct checks. */
+ if (grouping_operand_has_comparison_conflict(arg, opno, inputcollid, ctx))
+ return true;
+
return grouping_conflict_walker(arg, ctx);
}
diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out
index cf295d56507..01228fe574d 100644
--- a/src/test/regress/expected/subselect.out
+++ b/src/test/regress/expected/subselect.out
@@ -2151,6 +2151,166 @@ WHERE (CASE id WHEN 1 THEN 1 ELSE 0 END) = 1;
Filter: (CASE id WHEN 1 THEN 1 ELSE 0 END = 1)
(5 rows)
+-- Wrapped jsonb comparisons over grouped subqueries (DISTINCT ON and
+-- GROUP BY): wrapped quals must stay above grouping.
+CREATE TEMP TABLE pdt_json (id int, j jsonb);
+INSERT INTO pdt_json VALUES
+ (1, '1'),
+ (2, '1.0');
+EXPLAIN (COSTS OFF)
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j::text = '1.0';
+ QUERY PLAN
+-------------------------------------------------
+ Subquery Scan on s
+ Filter: ((s.j)::text = '1.0'::text)
+ -> Unique
+ -> Sort
+ Sort Key: pdt_json.j, pdt_json.id
+ -> Seq Scan on pdt_json
+(6 rows)
+
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j::text = '1.0';
+ id | j
+----+---
+(0 rows)
+
+EXPLAIN (COSTS OFF)
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j #>> '{}' = '1.0';
+ QUERY PLAN
+--------------------------------------------------
+ Subquery Scan on s
+ Filter: ((s.j #>> '{}'::text[]) = '1.0'::text)
+ -> Unique
+ -> Sort
+ Sort Key: pdt_json.j, pdt_json.id
+ -> Seq Scan on pdt_json
+(6 rows)
+
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j #>> '{}' = '1.0';
+ id | j
+----+---
+(0 rows)
+
+EXPLAIN (COSTS OFF)
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1';
+ QUERY PLAN
+-------------------------------------
+ Subquery Scan on s
+ Filter: ((s.j)::text = '1'::text)
+ -> HashAggregate
+ Group Key: pdt_json.j
+ -> Seq Scan on pdt_json
+(5 rows)
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1';
+ c
+---
+ 2
+(1 row)
+
+EXPLAIN (COSTS OFF)
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1.0';
+ QUERY PLAN
+---------------------------------------
+ Subquery Scan on s
+ Filter: ((s.j)::text = '1.0'::text)
+ -> HashAggregate
+ Group Key: pdt_json.j
+ -> Seq Scan on pdt_json
+(5 rows)
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1.0';
+ c
+---
+(0 rows)
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j = '1'::jsonb;
+ c
+---
+ 2
+(1 row)
+
+-- Same GROUP BY shape with hash aggregation disabled. That is the
+-- GroupAggregate plan that still pushed the wrapper onto Seq Scan
+-- without the GROUP BY check in qual_is_pushdown_safe.
+CREATE TEMP TABLE pdt_json_pk (id int primary key, j jsonb);
+INSERT INTO pdt_json_pk VALUES (1, '1'), (2, '1.0');
+SET enable_hashagg TO off;
+EXPLAIN (COSTS OFF)
+SELECT j, c FROM (SELECT j, count(*) c FROM pdt_json_pk GROUP BY j) s
+WHERE j::text = '1';
+ QUERY PLAN
+-------------------------------------------
+ Subquery Scan on s
+ Filter: ((s.j)::text = '1'::text)
+ -> GroupAggregate
+ Group Key: pdt_json_pk.j
+ -> Sort
+ Sort Key: pdt_json_pk.j
+ -> Seq Scan on pdt_json_pk
+(7 rows)
+
+SELECT j, c FROM (SELECT j, count(*) c FROM pdt_json_pk GROUP BY j) s
+WHERE j::text = '1';
+ j | c
+---+---
+ 1 | 2
+(1 row)
+
+EXPLAIN (COSTS OFF)
+SELECT j, c FROM (SELECT j, count(*) c FROM pdt_json_pk GROUP BY j) s
+WHERE j::text = '1.0';
+ QUERY PLAN
+-------------------------------------------
+ Subquery Scan on s
+ Filter: ((s.j)::text = '1.0'::text)
+ -> GroupAggregate
+ Group Key: pdt_json_pk.j
+ -> Sort
+ Sort Key: pdt_json_pk.j
+ -> Seq Scan on pdt_json_pk
+(7 rows)
+
+SELECT j, c FROM (SELECT j, count(*) c FROM pdt_json_pk GROUP BY j) s
+WHERE j::text = '1.0';
+ j | c
+---+---
+(0 rows)
+
+SELECT j, c FROM (SELECT j, count(*) c FROM pdt_json_pk GROUP BY j) s
+WHERE j = '1'::jsonb;
+ j | c
+---+---
+ 1 | 2
+(1 row)
+
+RESET enable_hashagg;
+-- But safe wrappers such as int4->text must still push below grouping.
+CREATE TEMP TABLE pdt_int (i int, s text, ts timestamptz);
+SET enable_hashagg TO off;
+EXPLAIN (COSTS OFF)
+SELECT * FROM (SELECT i, count(*) c FROM pdt_int GROUP BY i) s
+WHERE i::text = '5';
+ QUERY PLAN
+-----------------------------------------------
+ GroupAggregate
+ Group Key: pdt_int.i
+ -> Sort
+ Sort Key: pdt_int.i
+ -> Seq Scan on pdt_int
+ Filter: ((i)::text = '5'::text)
+(6 rows)
+
+RESET enable_hashagg;
-- 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 07438694f6e..99d63f669d2 100644
--- a/src/test/regress/sql/subselect.sql
+++ b/src/test/regress/sql/subselect.sql
@@ -1052,6 +1052,76 @@ EXPLAIN (COSTS OFF)
SELECT * FROM (SELECT DISTINCT id FROM pdt) s
WHERE (CASE id WHEN 1 THEN 1 ELSE 0 END) = 1;
+-- Wrapped jsonb comparisons over grouped subqueries (DISTINCT ON and
+-- GROUP BY): wrapped quals must stay above grouping.
+CREATE TEMP TABLE pdt_json (id int, j jsonb);
+INSERT INTO pdt_json VALUES
+ (1, '1'),
+ (2, '1.0');
+
+EXPLAIN (COSTS OFF)
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j::text = '1.0';
+
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j::text = '1.0';
+
+EXPLAIN (COSTS OFF)
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j #>> '{}' = '1.0';
+
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j #>> '{}' = '1.0';
+
+EXPLAIN (COSTS OFF)
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1';
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1';
+
+EXPLAIN (COSTS OFF)
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1.0';
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1.0';
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j = '1'::jsonb;
+
+-- Same GROUP BY shape with hash aggregation disabled. That is the
+-- GroupAggregate plan that still pushed the wrapper onto Seq Scan
+-- without the GROUP BY check in qual_is_pushdown_safe.
+CREATE TEMP TABLE pdt_json_pk (id int primary key, j jsonb);
+INSERT INTO pdt_json_pk VALUES (1, '1'), (2, '1.0');
+SET enable_hashagg TO off;
+EXPLAIN (COSTS OFF)
+SELECT j, c FROM (SELECT j, count(*) c FROM pdt_json_pk GROUP BY j) s
+WHERE j::text = '1';
+
+SELECT j, c FROM (SELECT j, count(*) c FROM pdt_json_pk GROUP BY j) s
+WHERE j::text = '1';
+
+EXPLAIN (COSTS OFF)
+SELECT j, c FROM (SELECT j, count(*) c FROM pdt_json_pk GROUP BY j) s
+WHERE j::text = '1.0';
+
+SELECT j, c FROM (SELECT j, count(*) c FROM pdt_json_pk GROUP BY j) s
+WHERE j::text = '1.0';
+
+SELECT j, c FROM (SELECT j, count(*) c FROM pdt_json_pk GROUP BY j) s
+WHERE j = '1'::jsonb;
+RESET enable_hashagg;
+
+-- But safe wrappers such as int4->text must still push below grouping.
+CREATE TEMP TABLE pdt_int (i int, s text, ts timestamptz);
+SET enable_hashagg TO off;
+EXPLAIN (COSTS OFF)
+SELECT * FROM (SELECT i, count(*) c FROM pdt_int GROUP BY i) s
+WHERE i::text = '5';
+RESET enable_hashagg;
+
-- 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.53.0
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: BUG #19649: Qual pushdown into GROUP BY subqueries ignores non-equivalence-preserving references to grouping col
@ 2026-09-05 10:45 Andrey Rachitskiy <pl0h0yp1@gmail.com>
parent: Andrey Rachitskiy <pl0h0yp1@gmail.com>
0 siblings, 1 reply; 9+ messages in thread
From: Andrey Rachitskiy @ 2026-09-05 10:45 UTC (permalink / raw)
To: Andrei Lepikhov <lepihov@gmail.com>; +Cc: 303677365@qq.com, pgsql-bugs@lists.postgresql.org, Tender Wang <tndrwang@gmail.com>
сб, 5 сент. 2026 г. в 12:52, Andrey Rachitskiy <pl0h0yp1@gmail.com>:
> v3 in attachment.
>
> P.S. I'm still learning plans and could be mistaken, so please don't judge
> too harshly.
>
I looked more carefully at how the pushed qual is placed, and I had
misread the plan when I wrote v3. For a GROUP BY subquery the wrapped
qual is not pushed below the grouping. subquery_push_qual attaches it to
HAVING, and find_having_conflicts then keeps it there through the same
walker that the clauses.c fix changes. So v2 already produced correct
results for GROUP BY, by construction rather than by accident.
```
if (subquery->hasAggs || subquery->groupClause ||
subquery->groupingSets || subquery->havingQual)
subquery->havingQual = make_and_qual(subquery->havingQual, qual);
else
subquery->jointree->quals = make_and_qual(...);
```
So a wrapped jsonb qual such as j::text = '1' stays in HAVING and is not
lowered to WHERE. That is why the GroupAggregate and HashAggregate cases
already gave correct results under v2, with the filter on the Agg node.
DISTINCT, window PARTITION BY and set operations are different. They have
no HAVING, so subquery_push_qual routes the pushed qual to WHERE. That is
why point 6 in qual_is_pushdown_safe lists those three and omits GROUP BY.
The clauses.c fix covers both boundaries because both share the walker.
So the groupClause check I added to qual_is_pushdown_safe in v3 is
redundant for correctness. It only changes the plan shape: the filter
ends up on the Subquery Scan instead of on the Agg node. Both are
correct.
I attach v4, it is v2 with fixed comment.
Sorry for the noise, I'll be more attentive and take my time.
Attachments:
[text/x-patch] v4-0001-Fix-qual-pushdown-for-wrapped-grouping-comparison.patch (15.3K, ../../CAB8bMisiMCwe3EQoP_3_4acvfsqcDKfTOf5iHdqE8JGLcqvDBg@mail.gmail.com/3-v4-0001-Fix-qual-pushdown-for-wrapped-grouping-comparison.patch)
download | inline diff:
From 7d7dce051f78341d9be430814b479cbdcaa11202 Mon Sep 17 00:00:00 2001
From: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Date: Sat, 5 Sep 2026 14:59:32 +0500
Subject: [PATCH v4] Fix qual pushdown for wrapped grouping comparisons
A grouping column used through a wrapper inside a comparison operand can
apply a finer equivalence relation than the grouping boundary uses.
jsonb grouping merges 1 and 1.0, but a wrapper such as j::text or
j #>> '{}' can tell them apart. Pushing such a qual below DISTINCT or
GROUP BY then splits one group and changes counts and group keys.
Keep the direct-operand checks unchanged, and recurse into non-direct
comparison operands, applying the same operator and collation checks to
wrapped jsonb grouping Vars found there.
The walker is shared by both grouping boundaries, so this one change is
enough. qual_is_pushdown_safe uses it for DISTINCT, window PARTITION BY
and set operations. For GROUP BY the pushed qual goes to HAVING in
subquery_push_qual, and find_having_conflicts uses the same walker to
keep it there instead of lowering it to WHERE.
Add regression cases for wrapped jsonb comparisons under DISTINCT ON and
GROUP BY, and a guard that a safe wrapper such as i::text = '5' is still
pushed down to Seq Scan.
Bug: #19649
Author: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Reviewed-by: Andrei Lepikhov <lepihov@gmail.com>
Reported-by: chunling qin <303677365@qq.com>
---
src/backend/optimizer/util/clauses.c | 158 ++++++++++++++++++++----
src/test/regress/expected/subselect.out | 105 ++++++++++++++++
src/test/regress/sql/subselect.sql | 46 +++++++
3 files changed, 287 insertions(+), 22 deletions(-)
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 55cebe4a74b..77df991af8f 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -27,6 +27,7 @@
#include "catalog/pg_operator.h"
#include "catalog/pg_proc.h"
#include "catalog/pg_type.h"
+#include "catalog/pg_type_d.h"
#include "executor/executor.h"
#include "executor/functions.h"
#include "funcapi.h"
@@ -112,6 +113,9 @@ typedef struct
grouping_eqop_callback get_eqop;
void *cb_context;
Var *case_var;
+ Oid cmp_opno;
+ Oid cmp_inputcollid;
+ bool cmp_context_valid;
} grouping_walker_ctx;
static bool contain_agg_clause_walker(Node *node, void *context);
@@ -133,6 +137,15 @@ static List *find_nonnullable_vars_walker(Node *node, bool top_level);
static bool is_strict_saop(ScalarArrayOpExpr *expr, bool falseOK);
static bool convert_saop_to_hashed_saop_walker(Node *node, void *context);
static bool grouping_conflict_walker(Node *node, grouping_walker_ctx *ctx);
+static bool grouping_var_has_nondeterministic_collation(Var *var);
+static bool grouping_var_has_comparison_conflict(Var *var, Oid opno,
+ Oid inputcollid,
+ grouping_walker_ctx *ctx);
+static bool grouping_operand_has_comparison_conflict(Node *node, Oid opno,
+ Oid inputcollid,
+ grouping_walker_ctx *ctx);
+static bool grouping_operand_has_comparison_conflict_walker(Node *node,
+ void *context);
static bool grouping_check_operands(Oid opno, Oid inputcollid,
List *args, grouping_walker_ctx *ctx);
static bool grouping_check_operand(Node *arg, Oid opno, Oid inputcollid,
@@ -6417,15 +6430,13 @@ pull_paramids_walker(Node *node, Bitmapset **context)
* For a nondeterministic collation, every other reference is rejected: a
* comparison under a different collation, and any function or operator over
* the column, because we cannot tell whether the function yields the same
- * result for values the grouping treats as equal, and many do not. A column
- * with a deterministic collation is not restricted this way.
+ * result for values the grouping treats as equal, and many do not.
*
- * This leaves one case uncaught: with a deterministic collation, a function
- * over the column can still feed a finer comparison than the direct-operand
- * check sees, for example record_image_ops over a rebuilt record, or scale()
- * over numeric where two equal values differ in scale. Catching it would
- * require knowing that a type's equality is bitwise, which we do not test
- * here.
+ * In addition, within a comparison, if an operand is not a direct grouping
+ * Var, we recurse into it and apply the same opfamily/collation checks to
+ * wrapped jsonb grouping Vars found there. This catches wrappers such as
+ * CoerceViaIO and text-extraction operators over jsonb that can feed a
+ * comparison using a different equality relation than grouping does.
*
* Returns true if any such conflict exists.
*/
@@ -6468,6 +6479,13 @@ expression_has_grouping_conflict(Node *expr,
* ArrayCoerceExpr's elemexpr and a JsonConstructorExpr's coercion, which
* stand for something else.
*/
+static bool
+grouping_var_has_nondeterministic_collation(Var *var)
+{
+ return OidIsValid(var->varcollid) &&
+ !get_collation_isdeterministic(var->varcollid);
+}
+
static bool
grouping_conflict_walker(Node *node, grouping_walker_ctx *ctx)
{
@@ -6487,8 +6505,7 @@ grouping_conflict_walker(Node *node, grouping_walker_ctx *ctx)
* boolean is not collatable, so it takes the deterministic path here.
*/
if (OidIsValid(ctx->get_eqop(var, ctx->cb_context)) &&
- OidIsValid(var->varcollid) &&
- !get_collation_isdeterministic(var->varcollid))
+ grouping_var_has_nondeterministic_collation(var))
return true;
return false;
}
@@ -6642,6 +6659,109 @@ grouping_check_operands(Oid opno, Oid inputcollid, List *args,
return false;
}
+/*
+ * grouping_var_has_comparison_conflict
+ * Apply direct-operand grouping checks to one Var.
+ *
+ * Returns true when this Var is a grouping column and the surrounding
+ * comparison would apply a conflicting equivalence relation, either because
+ * the comparison operator is from an incompatible equality family or because
+ * a nondeterministic-collation grouping Var is compared under a different
+ * collation.
+ */
+static bool
+grouping_var_has_comparison_conflict(Var *var, Oid opno, Oid inputcollid,
+ grouping_walker_ctx *ctx)
+{
+ Oid grouping_eqop = ctx->get_eqop(var, ctx->cb_context);
+
+ if (!OidIsValid(grouping_eqop))
+ return false;
+
+ if (!equality_ops_are_compatible(opno, grouping_eqop))
+ return true;
+
+ if (grouping_var_has_nondeterministic_collation(var) &&
+ inputcollid != var->varcollid)
+ return true;
+
+ return false;
+}
+
+/*
+ * grouping_operand_has_comparison_conflict
+ * Recursively inspect a non-direct comparison operand.
+ *
+ * grouping_check_operand calls this only after determining that the operand is
+ * not itself a direct Var reference.
+ */
+static bool
+grouping_operand_has_comparison_conflict(Node *node, Oid opno,
+ Oid inputcollid,
+ grouping_walker_ctx *ctx)
+{
+ grouping_walker_ctx check_ctx = *ctx;
+
+ check_ctx.cmp_opno = opno;
+ check_ctx.cmp_inputcollid = inputcollid;
+ check_ctx.cmp_context_valid = true;
+
+ return grouping_operand_has_comparison_conflict_walker(node, &check_ctx);
+}
+
+/*
+ * grouping_operand_has_comparison_conflict_walker
+ * Walker for grouping_operand_has_comparison_conflict.
+ *
+ * 'context' is grouping_walker_ctx with cmp_* fields set for the surrounding
+ * comparison. We descend through wrapper structure and apply
+ * grouping_var_has_comparison_conflict to every grouping Var found in the
+ * operand subtree. CaseTestExpr is resolved through ctx->case_var,
+ * matching the CASE handling used by grouping_conflict_walker.
+ *
+ * Returns true if any wrapped jsonb grouping Var inside this operand would
+ * fail the same operator/collation checks that we use for direct operands.
+ */
+static bool
+grouping_operand_has_comparison_conflict_walker(Node *node, void *context)
+{
+ grouping_walker_ctx *ctx = (grouping_walker_ctx *) context;
+
+ if (node == NULL)
+ return false;
+
+ if (IsA(node, RelabelType))
+ return grouping_operand_has_comparison_conflict_walker(
+ (Node *) ((RelabelType *) node)->arg, context);
+
+ if (IsA(node, CaseTestExpr))
+ return grouping_operand_has_comparison_conflict_walker(
+ (Node *) ctx->case_var, context);
+
+ if (IsA(node, Var))
+ {
+ Var *var = (Var *) node;
+
+ /*
+ * Restrict this to jsonb. jsonb equality treats 1 and 1.0 as equal,
+ * so a wrapper such as ::text or #>> can tell apart values the
+ * grouping merged. Applying the check to every type would also
+ * reject wrappers that preserve the grouping equivalence, such as
+ * int4 to text, and block their safe pushdown.
+ */
+ if (getBaseType(var->vartype) != JSONBOID)
+ return false;
+
+ Assert(ctx->cmp_context_valid);
+ return grouping_var_has_comparison_conflict(var, ctx->cmp_opno,
+ ctx->cmp_inputcollid, ctx);
+ }
+
+ return expression_tree_walker(node,
+ grouping_operand_has_comparison_conflict_walker,
+ context);
+}
+
/*
* grouping_check_operand
* Handle one operand 'arg' of a comparison with operator 'opno' and
@@ -6670,22 +6790,16 @@ grouping_check_operand(Node *arg, Oid opno, Oid inputcollid,
if (node && IsA(node, Var))
{
Var *var = (Var *) node;
- Oid grouping_eqop = ctx->get_eqop(var, ctx->cb_context);
- if (OidIsValid(grouping_eqop))
- {
- /* incompatible equality semantics */
- if (!equality_ops_are_compatible(opno, grouping_eqop))
- return true;
- /* nondeterministic collation compared under a different collation */
- if (OidIsValid(var->varcollid) &&
- !get_collation_isdeterministic(var->varcollid) &&
- inputcollid != var->varcollid)
- return true;
- }
+ if (grouping_var_has_comparison_conflict(var, opno, inputcollid, ctx))
+ return true;
return false; /* direct operand handled; do not recurse */
}
+ /* Recurse into non-direct operands and reuse direct checks. */
+ if (grouping_operand_has_comparison_conflict(arg, opno, inputcollid, ctx))
+ return true;
+
return grouping_conflict_walker(arg, ctx);
}
diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out
index cf295d56507..ae3e76b856f 100644
--- a/src/test/regress/expected/subselect.out
+++ b/src/test/regress/expected/subselect.out
@@ -2151,6 +2151,111 @@ WHERE (CASE id WHEN 1 THEN 1 ELSE 0 END) = 1;
Filter: (CASE id WHEN 1 THEN 1 ELSE 0 END = 1)
(5 rows)
+-- Wrapped jsonb comparisons over grouped subqueries (DISTINCT ON and
+-- GROUP BY): wrapped quals must stay above grouping.
+CREATE TEMP TABLE pdt_json (id int, j jsonb);
+INSERT INTO pdt_json VALUES
+ (1, '1'),
+ (2, '1.0');
+EXPLAIN (COSTS OFF)
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j::text = '1.0';
+ QUERY PLAN
+-------------------------------------------------
+ Subquery Scan on s
+ Filter: ((s.j)::text = '1.0'::text)
+ -> Unique
+ -> Sort
+ Sort Key: pdt_json.j, pdt_json.id
+ -> Seq Scan on pdt_json
+(6 rows)
+
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j::text = '1.0';
+ id | j
+----+---
+(0 rows)
+
+EXPLAIN (COSTS OFF)
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j #>> '{}' = '1.0';
+ QUERY PLAN
+--------------------------------------------------
+ Subquery Scan on s
+ Filter: ((s.j #>> '{}'::text[]) = '1.0'::text)
+ -> Unique
+ -> Sort
+ Sort Key: pdt_json.j, pdt_json.id
+ -> Seq Scan on pdt_json
+(6 rows)
+
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j #>> '{}' = '1.0';
+ id | j
+----+---
+(0 rows)
+
+EXPLAIN (COSTS OFF)
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1';
+ QUERY PLAN
+--------------------------------------------------
+ Subquery Scan on s
+ -> HashAggregate
+ Group Key: pdt_json.j
+ Filter: ((pdt_json.j)::text = '1'::text)
+ -> Seq Scan on pdt_json
+(5 rows)
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1';
+ c
+---
+ 2
+(1 row)
+
+EXPLAIN (COSTS OFF)
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1.0';
+ QUERY PLAN
+----------------------------------------------------
+ Subquery Scan on s
+ -> HashAggregate
+ Group Key: pdt_json.j
+ Filter: ((pdt_json.j)::text = '1.0'::text)
+ -> Seq Scan on pdt_json
+(5 rows)
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1.0';
+ c
+---
+(0 rows)
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j = '1'::jsonb;
+ c
+---
+ 2
+(1 row)
+
+-- But safe wrappers such as int4->text must still push below grouping.
+CREATE TEMP TABLE pdt_int (i int, s text, ts timestamptz);
+SET enable_hashagg TO off;
+EXPLAIN (COSTS OFF)
+SELECT * FROM (SELECT i, count(*) c FROM pdt_int GROUP BY i) s
+WHERE i::text = '5';
+ QUERY PLAN
+-----------------------------------------------
+ GroupAggregate
+ Group Key: pdt_int.i
+ -> Sort
+ Sort Key: pdt_int.i
+ -> Seq Scan on pdt_int
+ Filter: ((i)::text = '5'::text)
+(6 rows)
+
+RESET enable_hashagg;
-- 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 07438694f6e..01937ae249c 100644
--- a/src/test/regress/sql/subselect.sql
+++ b/src/test/regress/sql/subselect.sql
@@ -1052,6 +1052,52 @@ EXPLAIN (COSTS OFF)
SELECT * FROM (SELECT DISTINCT id FROM pdt) s
WHERE (CASE id WHEN 1 THEN 1 ELSE 0 END) = 1;
+-- Wrapped jsonb comparisons over grouped subqueries (DISTINCT ON and
+-- GROUP BY): wrapped quals must stay above grouping.
+CREATE TEMP TABLE pdt_json (id int, j jsonb);
+INSERT INTO pdt_json VALUES
+ (1, '1'),
+ (2, '1.0');
+
+EXPLAIN (COSTS OFF)
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j::text = '1.0';
+
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j::text = '1.0';
+
+EXPLAIN (COSTS OFF)
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j #>> '{}' = '1.0';
+
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_json ORDER BY j, id) s
+WHERE j #>> '{}' = '1.0';
+
+EXPLAIN (COSTS OFF)
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1';
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1';
+
+EXPLAIN (COSTS OFF)
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1.0';
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j::text = '1.0';
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_json GROUP BY j) s
+WHERE j = '1'::jsonb;
+
+-- But safe wrappers such as int4->text must still push below grouping.
+CREATE TEMP TABLE pdt_int (i int, s text, ts timestamptz);
+SET enable_hashagg TO off;
+EXPLAIN (COSTS OFF)
+SELECT * FROM (SELECT i, count(*) c FROM pdt_int GROUP BY i) s
+WHERE i::text = '5';
+RESET enable_hashagg;
+
-- 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.53.0
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: BUG #19649: Qual pushdown into GROUP BY subqueries ignores non-equivalence-preserving references to grouping col
@ 2026-09-05 16:23 Andrei Lepikhov <lepihov@gmail.com>
parent: Andrey Rachitskiy <pl0h0yp1@gmail.com>
0 siblings, 1 reply; 9+ messages in thread
From: Andrei Lepikhov @ 2026-09-05 16:23 UTC (permalink / raw)
To: Andrey Rachitskiy <pl0h0yp1@gmail.com>; +Cc: 303677365@qq.com, pgsql-bugs@lists.postgresql.org, Tender Wang <tndrwang@gmail.com>
On 05/09/2026 12:45, Andrey Rachitskiy wrote:
> I attach v4, it is v2 with fixed comment.
> Sorry for the noise, I'll be more attentive and take my time.
>
Thanks for the quick turnaround.
My concern is with the approach, not the code itself. The key change in v4 is a
single line:
if (getBaseType(var->vartype) != JSONBOID)
return false;
This only addresses the specific type mentioned in the report. However, the
reporter could have demonstrated the same bug using numeric, without involving
jsonb at all. The same goes for float8. In core, the default btree opclasses
that make no image-equality promise are numeric, float8, interval, jsonb, record
and tsvector, among others. Group by any of those and wrap the column in any
expression, and the qual moves. This means we will see this issue reported
again, just with a different type mentioned.
What's more, I think your solution not even full. Just check something like the
following:
SELECT j, count(*) FROM t GROUP BY j HAVING starts_with(j::text, '1.');
So, how can we address the broader issue rather than just this specific case?
The property we need is already in the catalogue. Peter and Anastasia added
equalimage support functions in 612a1ab7672 for btree deduplication, and the
documented contract is exactly what we need. If that holds, no wrapping
expression can distinguish values that the grouping merged, whatever the wrapper is.
Even this approach is not free from issues, quite narrow ones though. If I
understand correctly, image equality is not quite the same as bitwise equality
for varlena, since TOAST compression is not applied consistently, so functions
like pg_column_size() can still be inconsistent. The attachment is a LLM
generated patch that demonstrates the idea, maybe in too much detail. One way or
another, we end up with worse query plans, sometimes for nothing.
Postgres is not the only one facing this issue. Let's try to find ideas in
others' experience.
The mainstream solution is to remove the coarse equality from the type system.
DuckDB canonicalises -0.0 and gives DECIMAL a fixed per-column scale. ClickHouse
allows COLLATE only in ORDER BY and fixes DECIMAL scale as well. Equality
becomes image equality, and the optimiser needs no guard at all.
The second is to declare the result unspecified [1]. SQLite reproduces our bug
through type affinity, and their answer is that the affinity of such a column is
indeterminate and the group representative is arbitrary, so any result is legal.
Some discussions in the Internet give me an idea that SQL Server restrict clause
pushdown in such cases.
I personally prefer the second approach, possibly with an image equality check.
GROUP BY already hands back an arbitrary member of the group. Postgres does not
promise which one, and any expression that can distinguish members of the class
is therefore reading something we never guaranteed.
[1] https://sqlite.org/forum/info/6dc048f81303cb97
--
regards, Andrei Lepikhov,
pgEdge
diff --git a/doc/src/sgml/btree.sgml b/doc/src/sgml/btree.sgml
index 027361f20bb..b1b3255d616 100644
--- a/doc/src/sgml/btree.sgml
+++ b/doc/src/sgml/btree.sgml
@@ -464,9 +464,12 @@ returns bool
<function>equalimage</function> (<quote>equality implies image
equality</quote>) support functions, registered under support
function number 4. These functions allow the core code to
- determine when it is safe to apply the btree deduplication
- optimization. Currently, <function>equalimage</function>
- functions are only called when building or rebuilding an index.
+ determine when two values that compare equal may be freely
+ substituted for one another. They are called when building or
+ rebuilding an index, to decide whether the btree deduplication
+ optimization is safe, and during query planning, to decide whether
+ an optimization that merges equal values may discard the
+ distinction between them.
</para>
<para>
An <function>equalimage</function> function must have the
diff --git a/src/backend/access/nbtree/nbtutils.c b/src/backend/access/nbtree/nbtutils.c
index 014faa1622f..e525bdf1847 100644
--- a/src/backend/access/nbtree/nbtutils.c
+++ b/src/backend/access/nbtree/nbtutils.c
@@ -1183,21 +1183,12 @@ _bt_allequalimage(Relation rel, bool debugmessage)
for (int i = 0; i < IndexRelationGetNumberOfKeyAttributes(rel); i++)
{
- Oid opfamily = rel->rd_opfamily[i];
- Oid opcintype = rel->rd_opcintype[i];
- Oid collation = rel->rd_indcollation[i];
- Oid equalimageproc;
-
- equalimageproc = get_opfamily_proc(opfamily, opcintype, opcintype,
- BTEQUALIMAGE_PROC);
-
/*
- * If there is no BTEQUALIMAGE_PROC then deduplication is assumed to
- * be unsafe. Otherwise, actually call proc and see what it says.
+ * An opclass that lacks a BTEQUALIMAGE_PROC, or whose procedure
+ * returns false, makes deduplication unsafe for the whole index.
*/
- if (!OidIsValid(equalimageproc) ||
- !DatumGetBool(OidFunctionCall1Coll(equalimageproc, collation,
- ObjectIdGetDatum(opcintype))))
+ if (!opfamily_is_equalimage(rel->rd_opfamily[i], rel->rd_opcintype[i],
+ rel->rd_indcollation[i]))
{
allequalimage = false;
break;
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index fb6f81453ea..deaee399042 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -14,7 +14,6 @@
*/
#include "postgres.h"
-#include "access/nbtree.h"
#include "access/sysattr.h"
#include "catalog/pg_constraint.h"
#include "catalog/pg_type.h"
@@ -885,7 +884,6 @@ create_grouping_expr_infos(PlannerInfo *root)
SortGroupClause *sgc = lfirst_node(SortGroupClause, lc);
TargetEntry *tle = get_sortgroupclause_tle(sgc, root->processed_tlist);
TypeCacheEntry *tce;
- Oid equalimageproc;
Assert(tle->ressortgroupref > 0);
@@ -911,22 +909,13 @@ create_grouping_expr_infos(PlannerInfo *root)
!OidIsValid(tce->btree_opintype))
return;
- equalimageproc = get_opfamily_proc(tce->btree_opf,
- tce->btree_opintype,
- tce->btree_opintype,
- BTEQUALIMAGE_PROC);
-
/*
- * If there is no BTEQUALIMAGE_PROC, eager aggregation is assumed to
- * be unsafe. Otherwise, we call the procedure to check. We must be
- * careful to pass the expression's actual collation, rather than the
- * data type's default collation, to ensure that non-deterministic
- * collations are correctly handled.
+ * We must be careful to pass the expression's actual collation, rather
+ * than the data type's default collation, to ensure that
+ * non-deterministic collations are correctly handled.
*/
- if (!OidIsValid(equalimageproc) ||
- !DatumGetBool(OidFunctionCall1Coll(equalimageproc,
- exprCollation((Node *) tle->expr),
- ObjectIdGetDatum(tce->btree_opintype))))
+ if (!opfamily_is_equalimage(tce->btree_opf, tce->btree_opintype,
+ exprCollation((Node *) tle->expr)))
return;
exprs = lappend(exprs, tle->expr);
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 55cebe4a74b..07f25501da9 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -6414,18 +6414,22 @@ pull_paramids_walker(Node *node, Bitmapset **context)
* semantics compatible with the grouping eqop, or, for a nondeterministic
* collation, when the comparison applies a collation other than the column's.
*
- * For a nondeterministic collation, every other reference is rejected: a
- * comparison under a different collation, and any function or operator over
- * the column, because we cannot tell whether the function yields the same
- * result for values the grouping treats as equal, and many do not. A column
- * with a deterministic collation is not restricted this way.
+ * Every other reference -- a comparison under a different collation, or any
+ * function or operator over the column -- is opaque to us, so we accept it
+ * only when the grouping's equality is image equality. Then the values the
+ * grouping merges are interchangeable without loss of semantic information,
+ * and whatever wraps the column is bound to return the same answer for all of
+ * them. When it is not image equality, as for numeric (1 and 1.0), jsonb,
+ * float8 (0 and -0), record, or text under a nondeterministic collation, no
+ * such reasoning is available, and many wrappers do in fact distinguish the
+ * values: 1.0::text is not 1::text.
*
- * This leaves one case uncaught: with a deterministic collation, a function
- * over the column can still feed a finer comparison than the direct-operand
- * check sees, for example record_image_ops over a rebuilt record, or scale()
- * over numeric where two equal values differ in scale. Catching it would
- * require knowing that a type's equality is bitwise, which we do not test
- * here.
+ * Image equality is not quite bitwise equality for varlena types, because
+ * TOAST compression is not applied consistently on input. Expressions that
+ * expose physical representation rather than value, pg_column_size() for one,
+ * can therefore still tell apart values that the grouping merges. Those are
+ * outside the semantic contract that an equalimage procedure describes, and we
+ * make no attempt to detect them.
*
* Returns true if any such conflict exists.
*/
@@ -6477,18 +6481,23 @@ grouping_conflict_walker(Node *node, grouping_walker_ctx *ctx)
if (IsA(node, Var))
{
Var *var = (Var *) node;
+ Oid grouping_eqop = ctx->get_eqop(var, ctx->cb_context);
/*
* A grouping column reaches here when it was not handled as a direct
- * operand by a comparison node above (see the function header). That
- * is safe for a deterministic collation, but not for a
- * nondeterministic one, where the reference may distinguish values
- * the grouping considers equal. A bare boolean qual is safe too:
- * boolean is not collatable, so it takes the deterministic path here.
+ * operand by a comparison node above (see the function header), so we
+ * know nothing about the expression it is embedded in. Accept it only
+ * if the grouping's equality is image equality, which makes any two
+ * values the grouping merges interchangeable for every expression.
+ *
+ * This subsumes the nondeterministic-collation case: the equalimage
+ * procedure of a collatable type is handed the column's collation and
+ * answers false for a nondeterministic one. A bare boolean qual takes
+ * this path too, and stays safe, boolean equality being image
+ * equality.
*/
- if (OidIsValid(ctx->get_eqop(var, ctx->cb_context)) &&
- OidIsValid(var->varcollid) &&
- !get_collation_isdeterministic(var->varcollid))
+ if (OidIsValid(grouping_eqop) &&
+ !equality_op_is_equalimage(grouping_eqop, var->varcollid))
return true;
return false;
}
diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c
index ee69f81945f..264a5575a04 100644
--- a/src/backend/optimizer/util/relnode.c
+++ b/src/backend/optimizer/util/relnode.c
@@ -16,7 +16,6 @@
#include <limits.h>
-#include "access/nbtree.h"
#include "catalog/pg_constraint.h"
#include "miscadmin.h"
#include "nodes/nodeFuncs.h"
@@ -3038,7 +3037,6 @@ init_grouping_targets(PlannerInfo *root, RelOptInfo *rel,
*/
SortGroupClause *sgc;
TypeCacheEntry *tce;
- Oid equalimageproc;
/*
* But first, check if equality implies image equality for this
@@ -3051,22 +3049,13 @@ init_grouping_targets(PlannerInfo *root, RelOptInfo *rel,
!OidIsValid(tce->btree_opintype))
return false;
- equalimageproc = get_opfamily_proc(tce->btree_opf,
- tce->btree_opintype,
- tce->btree_opintype,
- BTEQUALIMAGE_PROC);
-
/*
- * If there is no BTEQUALIMAGE_PROC, eager aggregation is assumed
- * to be unsafe. Otherwise, we call the procedure to check. We
- * must be careful to pass the expression's actual collation,
+ * We must be careful to pass the expression's actual collation,
* rather than the data type's default collation, to ensure that
* non-deterministic collations are correctly handled.
*/
- if (!OidIsValid(equalimageproc) ||
- !DatumGetBool(OidFunctionCall1Coll(equalimageproc,
- exprCollation((Node *) expr),
- ObjectIdGetDatum(tce->btree_opintype))))
+ if (!opfamily_is_equalimage(tce->btree_opf, tce->btree_opintype,
+ exprCollation((Node *) expr)))
return false;
/* Create the SortGroupClause. */
diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index 9ef3922d17c..360ba470ef1 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -17,6 +17,7 @@
#include "access/hash.h"
#include "access/htup_details.h"
+#include "access/nbtree.h"
#include "bootstrap/bootstrap.h"
#include "catalog/namespace.h"
#include "catalog/pg_am.h"
@@ -1038,6 +1039,109 @@ get_opfamily_proc(Oid opfamily, Oid lefttype, Oid righttype, int16 procnum)
return result;
}
+/*
+ * opfamily_is_equalimage
+ *
+ * Does opfamily promise "equality implies image equality" for the given
+ * input type and collation?
+ *
+ * A true result means that whenever the opfamily's ordering function reports
+ * two values equal, those values are interchangeable without any loss of
+ * semantic information; that is, no expression can tell them apart. Callers
+ * rely on this when they want to substitute one member of an equivalence
+ * class for another, as B-tree deduplication does.
+ *
+ * An opfamily that registers no BTEQUALIMAGE_PROC makes no such promise, so
+ * we must assume the property does not hold. Note that callers must pass the
+ * collation actually in use rather than the type's default collation: for a
+ * collatable type the answer depends on it, since a nondeterministic
+ * collation is not image equality.
+ */
+bool
+opfamily_is_equalimage(Oid opfamily, Oid opcintype, Oid collation)
+{
+ Oid equalimageproc;
+
+ equalimageproc = get_opfamily_proc(opfamily, opcintype, opcintype,
+ BTEQUALIMAGE_PROC);
+
+ if (!OidIsValid(equalimageproc))
+ return false;
+
+ return DatumGetBool(OidFunctionCall1Coll(equalimageproc, collation,
+ ObjectIdGetDatum(opcintype)));
+}
+
+/*
+ * equality_op_is_equalimage
+ *
+ * Does eqop define an equivalence under which equal values are
+ * interchangeable without any loss of semantic information?
+ *
+ * This is the operator-level counterpart of opfamily_is_equalimage(), for
+ * callers that know only the equality operator some mechanism uses to decide
+ * which values to merge -- a SortGroupClause's eqop, typically -- and not the
+ * opfamily it came from.
+ *
+ * Not knowing the opfamily is why we must demand a promise from every family
+ * in which eqop is the equality member, rather than accepting the first "yes"
+ * we find. Those families need not agree: texteq is the equality member of
+ * both text_ops and text_pattern_ops, and under a nondeterministic collation
+ * they describe different equivalences, the former case-folding where the
+ * latter is bytewise. text_pattern_ops registers btequalimage, which answers
+ * true whatever collation it is handed, so trusting it alone would let a
+ * caller substitute values that a case-insensitive grouping merged.
+ *
+ * A false result means "not proven", not "proven false", and callers must
+ * treat it as "not image equality". A type with no ordering opclass at all,
+ * such as xid, always lands there.
+ *
+ * 'collation' must be the collation actually applied to the values, not the
+ * type's default; see opfamily_is_equalimage().
+ */
+bool
+equality_op_is_equalimage(Oid eqop, Oid collation)
+{
+ Oid lefttype;
+ Oid righttype;
+ List *opfamilies;
+ bool result;
+ ListCell *lc;
+
+ op_input_types(eqop, &lefttype, &righttype);
+
+ /*
+ * An equalimage procedure describes a single type, so a cross-type
+ * operator gives us nothing to ask about. Grouping equality operators are
+ * never cross-type, so this costs no optimization in practice.
+ */
+ if (lefttype != righttype)
+ return false;
+
+ /*
+ * Collect the opfamilies before calling any of their procedures: the
+ * procedure is user-supplied code that can throw, and we would rather not
+ * be holding a syscache list reference when it does.
+ */
+ opfamilies = get_mergejoin_opfamilies(eqop);
+
+ /* No ordering opfamily at all means nothing promised anything. */
+ result = (opfamilies != NIL);
+
+ foreach(lc, opfamilies)
+ {
+ if (!opfamily_is_equalimage(lfirst_oid(lc), lefttype, collation))
+ {
+ result = false;
+ break;
+ }
+ }
+
+ list_free(opfamilies);
+
+ return result;
+}
+
/* ---------- ATTRIBUTE CACHES ---------- */
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index 865980cb0f1..09887ddf093 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -95,6 +95,8 @@ extern bool collations_agree_on_equality(Oid coll1, Oid coll2);
extern bool op_is_safe_index_member(Oid opno);
extern Oid get_opfamily_proc(Oid opfamily, Oid lefttype, Oid righttype,
int16 procnum);
+extern bool opfamily_is_equalimage(Oid opfamily, Oid opcintype, Oid collation);
+extern bool equality_op_is_equalimage(Oid eqop, Oid collation);
extern char *get_attname(Oid relid, AttrNumber attnum, bool missing_ok);
extern AttrNumber get_attnum(Oid relid, const char *attname);
extern char get_attgenerated(Oid relid, AttrNumber attnum);
diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out
index 7d07619956f..bae145a3b88 100644
--- a/src/test/regress/expected/aggregates.out
+++ b/src/test/regress/expected/aggregates.out
@@ -1809,6 +1809,130 @@ select a, count(*) from t_having group by a having a = row(1.0)::avg_rec;
drop table t_having;
drop type avg_rec;
+-- A HAVING clause that reaches the grouping column through a wrapper, rather
+-- than as a direct operand of a comparison, must NOT be pushed down to WHERE
+-- unless the grouping's equality is image equality: the wrapper can tell apart
+-- values that GROUP BY merged into one group.
+create temp table t_eqimg (n numeric, f float8, j jsonb, i int);
+insert into t_eqimg values (1, '0', '1', 1), (1.0, '-0', '1.0', 1);
+-- baselines: each of these is a single group of two rows
+select n, count(*) from t_eqimg group by n;
+ n | count
+---+-------
+ 1 | 2
+(1 row)
+
+select f, count(*) from t_eqimg group by f;
+ f | count
+---+-------
+ 0 | 2
+(1 row)
+
+select j, count(*) from t_eqimg group by j;
+ j | count
+---+-------
+ 1 | 2
+(1 row)
+
+-- numeric equality ignores scale, so the clause must stay in HAVING
+explain (costs off)
+select n, count(*) from t_eqimg group by n having n::text = '1';
+ QUERY PLAN
+-----------------------------------
+ HashAggregate
+ Group Key: n
+ Filter: ((n)::text = '1'::text)
+ -> Seq Scan on t_eqimg
+(4 rows)
+
+select n, count(*) from t_eqimg group by n having n::text = '1';
+ n | count
+---+-------
+ 1 | 2
+(1 row)
+
+-- float8 equality merges 0 and -0
+explain (costs off)
+select f, count(*) from t_eqimg group by f having f::text = '0';
+ QUERY PLAN
+-----------------------------------
+ HashAggregate
+ Group Key: f
+ Filter: ((f)::text = '0'::text)
+ -> Seq Scan on t_eqimg
+(4 rows)
+
+select f, count(*) from t_eqimg group by f having f::text = '0';
+ f | count
+---+-------
+ 0 | 2
+(1 row)
+
+-- jsonb numbers compare as numeric but print their trailing zeroes
+explain (costs off)
+select j, count(*) from t_eqimg group by j having j::text = '1';
+ QUERY PLAN
+-----------------------------------
+ HashAggregate
+ Group Key: j
+ Filter: ((j)::text = '1'::text)
+ -> Seq Scan on t_eqimg
+(4 rows)
+
+select j, count(*) from t_eqimg group by j having j::text = '1';
+ j | count
+---+-------
+ 1 | 2
+(1 row)
+
+-- The same conflict reached through an outer WHERE over a GROUP BY subquery,
+-- which subquery_push_qual turns into a HAVING clause before we get to it.
+-- A WHERE clause may only select the subquery's output rows, never alter
+-- them, so both the count and the group key must be unaffected here.
+explain (costs off)
+select n, c from (select n, count(*) c from t_eqimg group by n) s
+where n::text = '1';
+ QUERY PLAN
+-------------------------------------------
+ HashAggregate
+ Group Key: t_eqimg.n
+ Filter: ((t_eqimg.n)::text = '1'::text)
+ -> Seq Scan on t_eqimg
+(4 rows)
+
+select n, c from (select n, count(*) c from t_eqimg group by n) s
+where n::text = '1';
+ n | c
+---+---
+ 1 | 2
+(1 row)
+
+select n, c from (select n, count(*) c from t_eqimg group by n) s
+where n::text = '1.0';
+ n | c
+---+---
+(0 rows)
+
+-- int equality is image equality, so a wrapped reference is still pushable
+explain (costs off)
+select i, count(*) from t_eqimg group by i having i + 1 = 2;
+ QUERY PLAN
+-------------------------------------
+ GroupAggregate
+ Group Key: i
+ -> Sort
+ Sort Key: i
+ -> Seq Scan on t_eqimg
+ Filter: ((i + 1) = 2)
+(6 rows)
+
+select i, count(*) from t_eqimg group by i having i + 1 = 2;
+ i | count
+---+-------
+ 1 | 2
+(1 row)
+
+drop table t_eqimg;
--
-- Test GROUP BY matching of join columns that are type-coerced due to USING
--
diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out
index cf295d56507..1dad491f525 100644
--- a/src/test/regress/expected/subselect.out
+++ b/src/test/regress/expected/subselect.out
@@ -2270,6 +2270,81 @@ WHERE a *= ROW(1.0)::t_rec;
(1.0)
(2 rows)
+ROLLBACK;
+--
+-- A qual that reaches a grouping column of the subquery through a wrapper,
+-- rather than as a direct operand of a comparison, is only pushable when the
+-- grouping's equality is image equality. numeric equality is not: 1 and 1.0
+-- are equal but do not print alike, so a pushed-down qual could both drop a
+-- row the grouping would have kept and change which row represents the group.
+--
+BEGIN;
+CREATE TEMP TABLE eqimg_num (n numeric);
+INSERT INTO eqimg_num VALUES (1), (1.0);
+-- the subquery emits a single row, so the outer WHERE can only keep or drop it
+SELECT n FROM (SELECT DISTINCT n FROM eqimg_num) s;
+ n
+---
+ 1
+(1 row)
+
+EXPLAIN (COSTS OFF)
+SELECT n FROM (SELECT DISTINCT n FROM eqimg_num) s WHERE n::text = '1.0';
+ QUERY PLAN
+---------------------------------------
+ Subquery Scan on s
+ Filter: ((s.n)::text = '1.0'::text)
+ -> HashAggregate
+ Group Key: eqimg_num.n
+ -> Seq Scan on eqimg_num
+(5 rows)
+
+SELECT n FROM (SELECT DISTINCT n FROM eqimg_num) s WHERE n::text = '1.0';
+ n
+---
+(0 rows)
+
+-- UNION groups by the same equality
+EXPLAIN (COSTS OFF)
+SELECT n FROM (SELECT n FROM eqimg_num UNION SELECT n FROM eqimg_num) s
+WHERE n::text = '1.0';
+ QUERY PLAN
+-----------------------------------------------------
+ Subquery Scan on s
+ Filter: ((s.n)::text = '1.0'::text)
+ -> HashAggregate
+ Group Key: eqimg_num.n
+ -> Append
+ -> Seq Scan on eqimg_num
+ -> Seq Scan on eqimg_num eqimg_num_1
+(7 rows)
+
+SELECT n FROM (SELECT n FROM eqimg_num UNION SELECT n FROM eqimg_num) s
+WHERE n::text = '1.0';
+ n
+---
+(0 rows)
+
+-- int equality is image equality, so the same shape of qual is pushable
+CREATE TEMP TABLE eqimg_int (i int);
+INSERT INTO eqimg_int VALUES (1), (1);
+EXPLAIN (COSTS OFF)
+SELECT i FROM (SELECT DISTINCT i FROM eqimg_int) s WHERE i + 1 = 2;
+ QUERY PLAN
+-------------------------------------
+ Unique
+ -> Sort
+ Sort Key: eqimg_int.i
+ -> Seq Scan on eqimg_int
+ Filter: ((i + 1) = 2)
+(5 rows)
+
+SELECT i FROM (SELECT DISTINCT i FROM eqimg_int) s WHERE i + 1 = 2;
+ i
+---
+ 1
+(1 row)
+
ROLLBACK;
--
-- Test that LIMIT can be pushed to SORT through a subquery that just projects
diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql
index 91f8342166f..c9893ef17e6 100644
--- a/src/test/regress/sql/aggregates.sql
+++ b/src/test/regress/sql/aggregates.sql
@@ -652,6 +652,52 @@ select a, count(*) from t_having group by a having a = row(1.0)::avg_rec;
drop table t_having;
drop type avg_rec;
+-- A HAVING clause that reaches the grouping column through a wrapper, rather
+-- than as a direct operand of a comparison, must NOT be pushed down to WHERE
+-- unless the grouping's equality is image equality: the wrapper can tell apart
+-- values that GROUP BY merged into one group.
+create temp table t_eqimg (n numeric, f float8, j jsonb, i int);
+insert into t_eqimg values (1, '0', '1', 1), (1.0, '-0', '1.0', 1);
+
+-- baselines: each of these is a single group of two rows
+select n, count(*) from t_eqimg group by n;
+select f, count(*) from t_eqimg group by f;
+select j, count(*) from t_eqimg group by j;
+
+-- numeric equality ignores scale, so the clause must stay in HAVING
+explain (costs off)
+select n, count(*) from t_eqimg group by n having n::text = '1';
+select n, count(*) from t_eqimg group by n having n::text = '1';
+
+-- float8 equality merges 0 and -0
+explain (costs off)
+select f, count(*) from t_eqimg group by f having f::text = '0';
+select f, count(*) from t_eqimg group by f having f::text = '0';
+
+-- jsonb numbers compare as numeric but print their trailing zeroes
+explain (costs off)
+select j, count(*) from t_eqimg group by j having j::text = '1';
+select j, count(*) from t_eqimg group by j having j::text = '1';
+
+-- The same conflict reached through an outer WHERE over a GROUP BY subquery,
+-- which subquery_push_qual turns into a HAVING clause before we get to it.
+-- A WHERE clause may only select the subquery's output rows, never alter
+-- them, so both the count and the group key must be unaffected here.
+explain (costs off)
+select n, c from (select n, count(*) c from t_eqimg group by n) s
+where n::text = '1';
+select n, c from (select n, count(*) c from t_eqimg group by n) s
+where n::text = '1';
+select n, c from (select n, count(*) c from t_eqimg group by n) s
+where n::text = '1.0';
+
+-- int equality is image equality, so a wrapped reference is still pushable
+explain (costs off)
+select i, count(*) from t_eqimg group by i having i + 1 = 2;
+select i, count(*) from t_eqimg group by i having i + 1 = 2;
+
+drop table t_eqimg;
+
--
-- Test GROUP BY matching of join columns that are type-coerced due to USING
--
diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql
index 07438694f6e..d6ef3badaa2 100644
--- a/src/test/regress/sql/subselect.sql
+++ b/src/test/regress/sql/subselect.sql
@@ -1110,6 +1110,45 @@ WHERE a *= ROW(1.0)::t_rec;
ROLLBACK;
+--
+-- A qual that reaches a grouping column of the subquery through a wrapper,
+-- rather than as a direct operand of a comparison, is only pushable when the
+-- grouping's equality is image equality. numeric equality is not: 1 and 1.0
+-- are equal but do not print alike, so a pushed-down qual could both drop a
+-- row the grouping would have kept and change which row represents the group.
+--
+BEGIN;
+
+CREATE TEMP TABLE eqimg_num (n numeric);
+INSERT INTO eqimg_num VALUES (1), (1.0);
+
+-- the subquery emits a single row, so the outer WHERE can only keep or drop it
+SELECT n FROM (SELECT DISTINCT n FROM eqimg_num) s;
+
+EXPLAIN (COSTS OFF)
+SELECT n FROM (SELECT DISTINCT n FROM eqimg_num) s WHERE n::text = '1.0';
+
+SELECT n FROM (SELECT DISTINCT n FROM eqimg_num) s WHERE n::text = '1.0';
+
+-- UNION groups by the same equality
+EXPLAIN (COSTS OFF)
+SELECT n FROM (SELECT n FROM eqimg_num UNION SELECT n FROM eqimg_num) s
+WHERE n::text = '1.0';
+
+SELECT n FROM (SELECT n FROM eqimg_num UNION SELECT n FROM eqimg_num) s
+WHERE n::text = '1.0';
+
+-- int equality is image equality, so the same shape of qual is pushable
+CREATE TEMP TABLE eqimg_int (i int);
+INSERT INTO eqimg_int VALUES (1), (1);
+
+EXPLAIN (COSTS OFF)
+SELECT i FROM (SELECT DISTINCT i FROM eqimg_int) s WHERE i + 1 = 2;
+
+SELECT i FROM (SELECT DISTINCT i FROM eqimg_int) s WHERE i + 1 = 2;
+
+ROLLBACK;
+
--
-- Test that LIMIT can be pushed to SORT through a subquery that just projects
-- columns. We check for that having happened by looking to see if EXPLAIN
Attachments:
[text/plain] demo.diff (25.1K, ../../7c27d19d-3324-4977-a8b2-ea89af0eac79@gmail.com/2-demo.diff)
download | inline diff:
diff --git a/doc/src/sgml/btree.sgml b/doc/src/sgml/btree.sgml
index 027361f20bb..b1b3255d616 100644
--- a/doc/src/sgml/btree.sgml
+++ b/doc/src/sgml/btree.sgml
@@ -464,9 +464,12 @@ returns bool
<function>equalimage</function> (<quote>equality implies image
equality</quote>) support functions, registered under support
function number 4. These functions allow the core code to
- determine when it is safe to apply the btree deduplication
- optimization. Currently, <function>equalimage</function>
- functions are only called when building or rebuilding an index.
+ determine when two values that compare equal may be freely
+ substituted for one another. They are called when building or
+ rebuilding an index, to decide whether the btree deduplication
+ optimization is safe, and during query planning, to decide whether
+ an optimization that merges equal values may discard the
+ distinction between them.
</para>
<para>
An <function>equalimage</function> function must have the
diff --git a/src/backend/access/nbtree/nbtutils.c b/src/backend/access/nbtree/nbtutils.c
index 014faa1622f..e525bdf1847 100644
--- a/src/backend/access/nbtree/nbtutils.c
+++ b/src/backend/access/nbtree/nbtutils.c
@@ -1183,21 +1183,12 @@ _bt_allequalimage(Relation rel, bool debugmessage)
for (int i = 0; i < IndexRelationGetNumberOfKeyAttributes(rel); i++)
{
- Oid opfamily = rel->rd_opfamily[i];
- Oid opcintype = rel->rd_opcintype[i];
- Oid collation = rel->rd_indcollation[i];
- Oid equalimageproc;
-
- equalimageproc = get_opfamily_proc(opfamily, opcintype, opcintype,
- BTEQUALIMAGE_PROC);
-
/*
- * If there is no BTEQUALIMAGE_PROC then deduplication is assumed to
- * be unsafe. Otherwise, actually call proc and see what it says.
+ * An opclass that lacks a BTEQUALIMAGE_PROC, or whose procedure
+ * returns false, makes deduplication unsafe for the whole index.
*/
- if (!OidIsValid(equalimageproc) ||
- !DatumGetBool(OidFunctionCall1Coll(equalimageproc, collation,
- ObjectIdGetDatum(opcintype))))
+ if (!opfamily_is_equalimage(rel->rd_opfamily[i], rel->rd_opcintype[i],
+ rel->rd_indcollation[i]))
{
allequalimage = false;
break;
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index fb6f81453ea..deaee399042 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -14,7 +14,6 @@
*/
#include "postgres.h"
-#include "access/nbtree.h"
#include "access/sysattr.h"
#include "catalog/pg_constraint.h"
#include "catalog/pg_type.h"
@@ -885,7 +884,6 @@ create_grouping_expr_infos(PlannerInfo *root)
SortGroupClause *sgc = lfirst_node(SortGroupClause, lc);
TargetEntry *tle = get_sortgroupclause_tle(sgc, root->processed_tlist);
TypeCacheEntry *tce;
- Oid equalimageproc;
Assert(tle->ressortgroupref > 0);
@@ -911,22 +909,13 @@ create_grouping_expr_infos(PlannerInfo *root)
!OidIsValid(tce->btree_opintype))
return;
- equalimageproc = get_opfamily_proc(tce->btree_opf,
- tce->btree_opintype,
- tce->btree_opintype,
- BTEQUALIMAGE_PROC);
-
/*
- * If there is no BTEQUALIMAGE_PROC, eager aggregation is assumed to
- * be unsafe. Otherwise, we call the procedure to check. We must be
- * careful to pass the expression's actual collation, rather than the
- * data type's default collation, to ensure that non-deterministic
- * collations are correctly handled.
+ * We must be careful to pass the expression's actual collation, rather
+ * than the data type's default collation, to ensure that
+ * non-deterministic collations are correctly handled.
*/
- if (!OidIsValid(equalimageproc) ||
- !DatumGetBool(OidFunctionCall1Coll(equalimageproc,
- exprCollation((Node *) tle->expr),
- ObjectIdGetDatum(tce->btree_opintype))))
+ if (!opfamily_is_equalimage(tce->btree_opf, tce->btree_opintype,
+ exprCollation((Node *) tle->expr)))
return;
exprs = lappend(exprs, tle->expr);
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 55cebe4a74b..07f25501da9 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -6414,18 +6414,22 @@ pull_paramids_walker(Node *node, Bitmapset **context)
* semantics compatible with the grouping eqop, or, for a nondeterministic
* collation, when the comparison applies a collation other than the column's.
*
- * For a nondeterministic collation, every other reference is rejected: a
- * comparison under a different collation, and any function or operator over
- * the column, because we cannot tell whether the function yields the same
- * result for values the grouping treats as equal, and many do not. A column
- * with a deterministic collation is not restricted this way.
+ * Every other reference -- a comparison under a different collation, or any
+ * function or operator over the column -- is opaque to us, so we accept it
+ * only when the grouping's equality is image equality. Then the values the
+ * grouping merges are interchangeable without loss of semantic information,
+ * and whatever wraps the column is bound to return the same answer for all of
+ * them. When it is not image equality, as for numeric (1 and 1.0), jsonb,
+ * float8 (0 and -0), record, or text under a nondeterministic collation, no
+ * such reasoning is available, and many wrappers do in fact distinguish the
+ * values: 1.0::text is not 1::text.
*
- * This leaves one case uncaught: with a deterministic collation, a function
- * over the column can still feed a finer comparison than the direct-operand
- * check sees, for example record_image_ops over a rebuilt record, or scale()
- * over numeric where two equal values differ in scale. Catching it would
- * require knowing that a type's equality is bitwise, which we do not test
- * here.
+ * Image equality is not quite bitwise equality for varlena types, because
+ * TOAST compression is not applied consistently on input. Expressions that
+ * expose physical representation rather than value, pg_column_size() for one,
+ * can therefore still tell apart values that the grouping merges. Those are
+ * outside the semantic contract that an equalimage procedure describes, and we
+ * make no attempt to detect them.
*
* Returns true if any such conflict exists.
*/
@@ -6477,18 +6481,23 @@ grouping_conflict_walker(Node *node, grouping_walker_ctx *ctx)
if (IsA(node, Var))
{
Var *var = (Var *) node;
+ Oid grouping_eqop = ctx->get_eqop(var, ctx->cb_context);
/*
* A grouping column reaches here when it was not handled as a direct
- * operand by a comparison node above (see the function header). That
- * is safe for a deterministic collation, but not for a
- * nondeterministic one, where the reference may distinguish values
- * the grouping considers equal. A bare boolean qual is safe too:
- * boolean is not collatable, so it takes the deterministic path here.
+ * operand by a comparison node above (see the function header), so we
+ * know nothing about the expression it is embedded in. Accept it only
+ * if the grouping's equality is image equality, which makes any two
+ * values the grouping merges interchangeable for every expression.
+ *
+ * This subsumes the nondeterministic-collation case: the equalimage
+ * procedure of a collatable type is handed the column's collation and
+ * answers false for a nondeterministic one. A bare boolean qual takes
+ * this path too, and stays safe, boolean equality being image
+ * equality.
*/
- if (OidIsValid(ctx->get_eqop(var, ctx->cb_context)) &&
- OidIsValid(var->varcollid) &&
- !get_collation_isdeterministic(var->varcollid))
+ if (OidIsValid(grouping_eqop) &&
+ !equality_op_is_equalimage(grouping_eqop, var->varcollid))
return true;
return false;
}
diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c
index ee69f81945f..264a5575a04 100644
--- a/src/backend/optimizer/util/relnode.c
+++ b/src/backend/optimizer/util/relnode.c
@@ -16,7 +16,6 @@
#include <limits.h>
-#include "access/nbtree.h"
#include "catalog/pg_constraint.h"
#include "miscadmin.h"
#include "nodes/nodeFuncs.h"
@@ -3038,7 +3037,6 @@ init_grouping_targets(PlannerInfo *root, RelOptInfo *rel,
*/
SortGroupClause *sgc;
TypeCacheEntry *tce;
- Oid equalimageproc;
/*
* But first, check if equality implies image equality for this
@@ -3051,22 +3049,13 @@ init_grouping_targets(PlannerInfo *root, RelOptInfo *rel,
!OidIsValid(tce->btree_opintype))
return false;
- equalimageproc = get_opfamily_proc(tce->btree_opf,
- tce->btree_opintype,
- tce->btree_opintype,
- BTEQUALIMAGE_PROC);
-
/*
- * If there is no BTEQUALIMAGE_PROC, eager aggregation is assumed
- * to be unsafe. Otherwise, we call the procedure to check. We
- * must be careful to pass the expression's actual collation,
+ * We must be careful to pass the expression's actual collation,
* rather than the data type's default collation, to ensure that
* non-deterministic collations are correctly handled.
*/
- if (!OidIsValid(equalimageproc) ||
- !DatumGetBool(OidFunctionCall1Coll(equalimageproc,
- exprCollation((Node *) expr),
- ObjectIdGetDatum(tce->btree_opintype))))
+ if (!opfamily_is_equalimage(tce->btree_opf, tce->btree_opintype,
+ exprCollation((Node *) expr)))
return false;
/* Create the SortGroupClause. */
diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index 9ef3922d17c..360ba470ef1 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -17,6 +17,7 @@
#include "access/hash.h"
#include "access/htup_details.h"
+#include "access/nbtree.h"
#include "bootstrap/bootstrap.h"
#include "catalog/namespace.h"
#include "catalog/pg_am.h"
@@ -1038,6 +1039,109 @@ get_opfamily_proc(Oid opfamily, Oid lefttype, Oid righttype, int16 procnum)
return result;
}
+/*
+ * opfamily_is_equalimage
+ *
+ * Does opfamily promise "equality implies image equality" for the given
+ * input type and collation?
+ *
+ * A true result means that whenever the opfamily's ordering function reports
+ * two values equal, those values are interchangeable without any loss of
+ * semantic information; that is, no expression can tell them apart. Callers
+ * rely on this when they want to substitute one member of an equivalence
+ * class for another, as B-tree deduplication does.
+ *
+ * An opfamily that registers no BTEQUALIMAGE_PROC makes no such promise, so
+ * we must assume the property does not hold. Note that callers must pass the
+ * collation actually in use rather than the type's default collation: for a
+ * collatable type the answer depends on it, since a nondeterministic
+ * collation is not image equality.
+ */
+bool
+opfamily_is_equalimage(Oid opfamily, Oid opcintype, Oid collation)
+{
+ Oid equalimageproc;
+
+ equalimageproc = get_opfamily_proc(opfamily, opcintype, opcintype,
+ BTEQUALIMAGE_PROC);
+
+ if (!OidIsValid(equalimageproc))
+ return false;
+
+ return DatumGetBool(OidFunctionCall1Coll(equalimageproc, collation,
+ ObjectIdGetDatum(opcintype)));
+}
+
+/*
+ * equality_op_is_equalimage
+ *
+ * Does eqop define an equivalence under which equal values are
+ * interchangeable without any loss of semantic information?
+ *
+ * This is the operator-level counterpart of opfamily_is_equalimage(), for
+ * callers that know only the equality operator some mechanism uses to decide
+ * which values to merge -- a SortGroupClause's eqop, typically -- and not the
+ * opfamily it came from.
+ *
+ * Not knowing the opfamily is why we must demand a promise from every family
+ * in which eqop is the equality member, rather than accepting the first "yes"
+ * we find. Those families need not agree: texteq is the equality member of
+ * both text_ops and text_pattern_ops, and under a nondeterministic collation
+ * they describe different equivalences, the former case-folding where the
+ * latter is bytewise. text_pattern_ops registers btequalimage, which answers
+ * true whatever collation it is handed, so trusting it alone would let a
+ * caller substitute values that a case-insensitive grouping merged.
+ *
+ * A false result means "not proven", not "proven false", and callers must
+ * treat it as "not image equality". A type with no ordering opclass at all,
+ * such as xid, always lands there.
+ *
+ * 'collation' must be the collation actually applied to the values, not the
+ * type's default; see opfamily_is_equalimage().
+ */
+bool
+equality_op_is_equalimage(Oid eqop, Oid collation)
+{
+ Oid lefttype;
+ Oid righttype;
+ List *opfamilies;
+ bool result;
+ ListCell *lc;
+
+ op_input_types(eqop, &lefttype, &righttype);
+
+ /*
+ * An equalimage procedure describes a single type, so a cross-type
+ * operator gives us nothing to ask about. Grouping equality operators are
+ * never cross-type, so this costs no optimization in practice.
+ */
+ if (lefttype != righttype)
+ return false;
+
+ /*
+ * Collect the opfamilies before calling any of their procedures: the
+ * procedure is user-supplied code that can throw, and we would rather not
+ * be holding a syscache list reference when it does.
+ */
+ opfamilies = get_mergejoin_opfamilies(eqop);
+
+ /* No ordering opfamily at all means nothing promised anything. */
+ result = (opfamilies != NIL);
+
+ foreach(lc, opfamilies)
+ {
+ if (!opfamily_is_equalimage(lfirst_oid(lc), lefttype, collation))
+ {
+ result = false;
+ break;
+ }
+ }
+
+ list_free(opfamilies);
+
+ return result;
+}
+
/* ---------- ATTRIBUTE CACHES ---------- */
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index 865980cb0f1..09887ddf093 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -95,6 +95,8 @@ extern bool collations_agree_on_equality(Oid coll1, Oid coll2);
extern bool op_is_safe_index_member(Oid opno);
extern Oid get_opfamily_proc(Oid opfamily, Oid lefttype, Oid righttype,
int16 procnum);
+extern bool opfamily_is_equalimage(Oid opfamily, Oid opcintype, Oid collation);
+extern bool equality_op_is_equalimage(Oid eqop, Oid collation);
extern char *get_attname(Oid relid, AttrNumber attnum, bool missing_ok);
extern AttrNumber get_attnum(Oid relid, const char *attname);
extern char get_attgenerated(Oid relid, AttrNumber attnum);
diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out
index 7d07619956f..bae145a3b88 100644
--- a/src/test/regress/expected/aggregates.out
+++ b/src/test/regress/expected/aggregates.out
@@ -1809,6 +1809,130 @@ select a, count(*) from t_having group by a having a = row(1.0)::avg_rec;
drop table t_having;
drop type avg_rec;
+-- A HAVING clause that reaches the grouping column through a wrapper, rather
+-- than as a direct operand of a comparison, must NOT be pushed down to WHERE
+-- unless the grouping's equality is image equality: the wrapper can tell apart
+-- values that GROUP BY merged into one group.
+create temp table t_eqimg (n numeric, f float8, j jsonb, i int);
+insert into t_eqimg values (1, '0', '1', 1), (1.0, '-0', '1.0', 1);
+-- baselines: each of these is a single group of two rows
+select n, count(*) from t_eqimg group by n;
+ n | count
+---+-------
+ 1 | 2
+(1 row)
+
+select f, count(*) from t_eqimg group by f;
+ f | count
+---+-------
+ 0 | 2
+(1 row)
+
+select j, count(*) from t_eqimg group by j;
+ j | count
+---+-------
+ 1 | 2
+(1 row)
+
+-- numeric equality ignores scale, so the clause must stay in HAVING
+explain (costs off)
+select n, count(*) from t_eqimg group by n having n::text = '1';
+ QUERY PLAN
+-----------------------------------
+ HashAggregate
+ Group Key: n
+ Filter: ((n)::text = '1'::text)
+ -> Seq Scan on t_eqimg
+(4 rows)
+
+select n, count(*) from t_eqimg group by n having n::text = '1';
+ n | count
+---+-------
+ 1 | 2
+(1 row)
+
+-- float8 equality merges 0 and -0
+explain (costs off)
+select f, count(*) from t_eqimg group by f having f::text = '0';
+ QUERY PLAN
+-----------------------------------
+ HashAggregate
+ Group Key: f
+ Filter: ((f)::text = '0'::text)
+ -> Seq Scan on t_eqimg
+(4 rows)
+
+select f, count(*) from t_eqimg group by f having f::text = '0';
+ f | count
+---+-------
+ 0 | 2
+(1 row)
+
+-- jsonb numbers compare as numeric but print their trailing zeroes
+explain (costs off)
+select j, count(*) from t_eqimg group by j having j::text = '1';
+ QUERY PLAN
+-----------------------------------
+ HashAggregate
+ Group Key: j
+ Filter: ((j)::text = '1'::text)
+ -> Seq Scan on t_eqimg
+(4 rows)
+
+select j, count(*) from t_eqimg group by j having j::text = '1';
+ j | count
+---+-------
+ 1 | 2
+(1 row)
+
+-- The same conflict reached through an outer WHERE over a GROUP BY subquery,
+-- which subquery_push_qual turns into a HAVING clause before we get to it.
+-- A WHERE clause may only select the subquery's output rows, never alter
+-- them, so both the count and the group key must be unaffected here.
+explain (costs off)
+select n, c from (select n, count(*) c from t_eqimg group by n) s
+where n::text = '1';
+ QUERY PLAN
+-------------------------------------------
+ HashAggregate
+ Group Key: t_eqimg.n
+ Filter: ((t_eqimg.n)::text = '1'::text)
+ -> Seq Scan on t_eqimg
+(4 rows)
+
+select n, c from (select n, count(*) c from t_eqimg group by n) s
+where n::text = '1';
+ n | c
+---+---
+ 1 | 2
+(1 row)
+
+select n, c from (select n, count(*) c from t_eqimg group by n) s
+where n::text = '1.0';
+ n | c
+---+---
+(0 rows)
+
+-- int equality is image equality, so a wrapped reference is still pushable
+explain (costs off)
+select i, count(*) from t_eqimg group by i having i + 1 = 2;
+ QUERY PLAN
+-------------------------------------
+ GroupAggregate
+ Group Key: i
+ -> Sort
+ Sort Key: i
+ -> Seq Scan on t_eqimg
+ Filter: ((i + 1) = 2)
+(6 rows)
+
+select i, count(*) from t_eqimg group by i having i + 1 = 2;
+ i | count
+---+-------
+ 1 | 2
+(1 row)
+
+drop table t_eqimg;
--
-- Test GROUP BY matching of join columns that are type-coerced due to USING
--
diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out
index cf295d56507..1dad491f525 100644
--- a/src/test/regress/expected/subselect.out
+++ b/src/test/regress/expected/subselect.out
@@ -2270,6 +2270,81 @@ WHERE a *= ROW(1.0)::t_rec;
(1.0)
(2 rows)
+ROLLBACK;
+--
+-- A qual that reaches a grouping column of the subquery through a wrapper,
+-- rather than as a direct operand of a comparison, is only pushable when the
+-- grouping's equality is image equality. numeric equality is not: 1 and 1.0
+-- are equal but do not print alike, so a pushed-down qual could both drop a
+-- row the grouping would have kept and change which row represents the group.
+--
+BEGIN;
+CREATE TEMP TABLE eqimg_num (n numeric);
+INSERT INTO eqimg_num VALUES (1), (1.0);
+-- the subquery emits a single row, so the outer WHERE can only keep or drop it
+SELECT n FROM (SELECT DISTINCT n FROM eqimg_num) s;
+ n
+---
+ 1
+(1 row)
+
+EXPLAIN (COSTS OFF)
+SELECT n FROM (SELECT DISTINCT n FROM eqimg_num) s WHERE n::text = '1.0';
+ QUERY PLAN
+---------------------------------------
+ Subquery Scan on s
+ Filter: ((s.n)::text = '1.0'::text)
+ -> HashAggregate
+ Group Key: eqimg_num.n
+ -> Seq Scan on eqimg_num
+(5 rows)
+
+SELECT n FROM (SELECT DISTINCT n FROM eqimg_num) s WHERE n::text = '1.0';
+ n
+---
+(0 rows)
+
+-- UNION groups by the same equality
+EXPLAIN (COSTS OFF)
+SELECT n FROM (SELECT n FROM eqimg_num UNION SELECT n FROM eqimg_num) s
+WHERE n::text = '1.0';
+ QUERY PLAN
+-----------------------------------------------------
+ Subquery Scan on s
+ Filter: ((s.n)::text = '1.0'::text)
+ -> HashAggregate
+ Group Key: eqimg_num.n
+ -> Append
+ -> Seq Scan on eqimg_num
+ -> Seq Scan on eqimg_num eqimg_num_1
+(7 rows)
+
+SELECT n FROM (SELECT n FROM eqimg_num UNION SELECT n FROM eqimg_num) s
+WHERE n::text = '1.0';
+ n
+---
+(0 rows)
+
+-- int equality is image equality, so the same shape of qual is pushable
+CREATE TEMP TABLE eqimg_int (i int);
+INSERT INTO eqimg_int VALUES (1), (1);
+EXPLAIN (COSTS OFF)
+SELECT i FROM (SELECT DISTINCT i FROM eqimg_int) s WHERE i + 1 = 2;
+ QUERY PLAN
+-------------------------------------
+ Unique
+ -> Sort
+ Sort Key: eqimg_int.i
+ -> Seq Scan on eqimg_int
+ Filter: ((i + 1) = 2)
+(5 rows)
+
+SELECT i FROM (SELECT DISTINCT i FROM eqimg_int) s WHERE i + 1 = 2;
+ i
+---
+ 1
+(1 row)
+
ROLLBACK;
--
-- Test that LIMIT can be pushed to SORT through a subquery that just projects
diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql
index 91f8342166f..c9893ef17e6 100644
--- a/src/test/regress/sql/aggregates.sql
+++ b/src/test/regress/sql/aggregates.sql
@@ -652,6 +652,52 @@ select a, count(*) from t_having group by a having a = row(1.0)::avg_rec;
drop table t_having;
drop type avg_rec;
+-- A HAVING clause that reaches the grouping column through a wrapper, rather
+-- than as a direct operand of a comparison, must NOT be pushed down to WHERE
+-- unless the grouping's equality is image equality: the wrapper can tell apart
+-- values that GROUP BY merged into one group.
+create temp table t_eqimg (n numeric, f float8, j jsonb, i int);
+insert into t_eqimg values (1, '0', '1', 1), (1.0, '-0', '1.0', 1);
+
+-- baselines: each of these is a single group of two rows
+select n, count(*) from t_eqimg group by n;
+select f, count(*) from t_eqimg group by f;
+select j, count(*) from t_eqimg group by j;
+
+-- numeric equality ignores scale, so the clause must stay in HAVING
+explain (costs off)
+select n, count(*) from t_eqimg group by n having n::text = '1';
+select n, count(*) from t_eqimg group by n having n::text = '1';
+
+-- float8 equality merges 0 and -0
+explain (costs off)
+select f, count(*) from t_eqimg group by f having f::text = '0';
+select f, count(*) from t_eqimg group by f having f::text = '0';
+
+-- jsonb numbers compare as numeric but print their trailing zeroes
+explain (costs off)
+select j, count(*) from t_eqimg group by j having j::text = '1';
+select j, count(*) from t_eqimg group by j having j::text = '1';
+
+-- The same conflict reached through an outer WHERE over a GROUP BY subquery,
+-- which subquery_push_qual turns into a HAVING clause before we get to it.
+-- A WHERE clause may only select the subquery's output rows, never alter
+-- them, so both the count and the group key must be unaffected here.
+explain (costs off)
+select n, c from (select n, count(*) c from t_eqimg group by n) s
+where n::text = '1';
+select n, c from (select n, count(*) c from t_eqimg group by n) s
+where n::text = '1';
+select n, c from (select n, count(*) c from t_eqimg group by n) s
+where n::text = '1.0';
+
+-- int equality is image equality, so a wrapped reference is still pushable
+explain (costs off)
+select i, count(*) from t_eqimg group by i having i + 1 = 2;
+select i, count(*) from t_eqimg group by i having i + 1 = 2;
+
+drop table t_eqimg;
+
--
-- Test GROUP BY matching of join columns that are type-coerced due to USING
--
diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql
index 07438694f6e..d6ef3badaa2 100644
--- a/src/test/regress/sql/subselect.sql
+++ b/src/test/regress/sql/subselect.sql
@@ -1110,6 +1110,45 @@ WHERE a *= ROW(1.0)::t_rec;
ROLLBACK;
+--
+-- A qual that reaches a grouping column of the subquery through a wrapper,
+-- rather than as a direct operand of a comparison, is only pushable when the
+-- grouping's equality is image equality. numeric equality is not: 1 and 1.0
+-- are equal but do not print alike, so a pushed-down qual could both drop a
+-- row the grouping would have kept and change which row represents the group.
+--
+BEGIN;
+
+CREATE TEMP TABLE eqimg_num (n numeric);
+INSERT INTO eqimg_num VALUES (1), (1.0);
+
+-- the subquery emits a single row, so the outer WHERE can only keep or drop it
+SELECT n FROM (SELECT DISTINCT n FROM eqimg_num) s;
+
+EXPLAIN (COSTS OFF)
+SELECT n FROM (SELECT DISTINCT n FROM eqimg_num) s WHERE n::text = '1.0';
+
+SELECT n FROM (SELECT DISTINCT n FROM eqimg_num) s WHERE n::text = '1.0';
+
+-- UNION groups by the same equality
+EXPLAIN (COSTS OFF)
+SELECT n FROM (SELECT n FROM eqimg_num UNION SELECT n FROM eqimg_num) s
+WHERE n::text = '1.0';
+
+SELECT n FROM (SELECT n FROM eqimg_num UNION SELECT n FROM eqimg_num) s
+WHERE n::text = '1.0';
+
+-- int equality is image equality, so the same shape of qual is pushable
+CREATE TEMP TABLE eqimg_int (i int);
+INSERT INTO eqimg_int VALUES (1), (1);
+
+EXPLAIN (COSTS OFF)
+SELECT i FROM (SELECT DISTINCT i FROM eqimg_int) s WHERE i + 1 = 2;
+
+SELECT i FROM (SELECT DISTINCT i FROM eqimg_int) s WHERE i + 1 = 2;
+
+ROLLBACK;
+
--
-- Test that LIMIT can be pushed to SORT through a subquery that just projects
-- columns. We check for that having happened by looking to see if EXPLAIN
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: BUG #19649: Qual pushdown into GROUP BY subqueries ignores non-equivalence-preserving references to grouping col
@ 2026-09-05 16:48 Andrey Rachitskiy <pl0h0yp1@gmail.com>
parent: Andrei Lepikhov <lepihov@gmail.com>
0 siblings, 1 reply; 9+ messages in thread
From: Andrey Rachitskiy @ 2026-09-05 16:48 UTC (permalink / raw)
To: Andrei Lepikhov <lepihov@gmail.com>; +Cc: 303677365@qq.com, pgsql-bugs@lists.postgresql.org, Tender Wang <tndrwang@gmail.com>
сб, 5 сент. 2026 г. в 21:23, Andrei Lepikhov <lepihov@gmail.com>:
> My concern is with the approach, not the code itself. The key change in v4
> is a
> single line:
>
> if (getBaseType(var->vartype) != JSONBOID)
> return false;
>
> This only addresses the specific type mentioned in the report. However, the
> reporter could have demonstrated the same bug using numeric, without
> involving
> jsonb at all. The same goes for float8. In core, the default btree
> opclasses
> that make no image-equality promise are numeric, float8, interval, jsonb,
> record
> and tsvector, among others.
>
I agree, I deliberately didn't include them. I didn't like the code I ended
up with when I took them into account. And I couldn't figure out a better
way to do it, so I just stuck with jsonb.
>
> The property we need is already in the catalogue. Peter and Anastasia added
> equalimage support functions in 612a1ab7672 for btree deduplication, and
> the
> documented contract is exactly what we need. If that holds, no wrapping
> expression can distinguish values that the grouping merged, whatever the
> wrapper is.
>
>
Regarding BTEQUALIMAGE_PROC, I agree as well — I also considered it, but
rejected it because I thought it would degrade the execution plan, although
in our case it's actually a perfect fit.
> The second is to declare the result unspecified [1]. SQLite reproduces our
> bug
> through type affinity, and their answer is that the affinity of such a
> column is
> indeterminate and the group representative is arbitrary, so any result is
> legal.
> Some discussions in the Internet give me an idea that SQL Server restrict
> clause
> pushdown in such cases.
>
> I personally prefer the second approach, possibly with an image equality
> check.
> GROUP BY already hands back an arbitrary member of the group. Postgres
> does not
> promise which one, and any expression that can distinguish members of the
> class
> is therefore reading something we never guaranteed.
>
> [1] https://sqlite.org/forum/info/6dc048f81303cb97
>
>
>
I'll take a look at the code from [0].
Perhaps we will be able to find some option in the discussion process and I
will try to implement it.
--
Regards,
Rachitskiy Andrey
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: BUG #19649: Qual pushdown into GROUP BY subqueries ignores non-equivalence-preserving references to grouping col
@ 2026-09-06 13:09 Andrey Rachitskiy <pl0h0yp1@gmail.com>
parent: Andrey Rachitskiy <pl0h0yp1@gmail.com>
0 siblings, 0 replies; 9+ messages in thread
From: Andrey Rachitskiy @ 2026-09-06 13:09 UTC (permalink / raw)
To: Andrei Lepikhov <lepihov@gmail.com>; +Cc: 303677365@qq.com, pgsql-bugs@lists.postgresql.org, Tender Wang <tndrwang@gmail.com>
сб, 5 сент. 2026 г. в 21:48, Andrey Rachitskiy <pl0h0yp1@gmail.com>:
> I will try to implement it.
>
>
Hi, Tender, Andrei!
Please review v5. It uses BTEQUALIMAGE_PROC and updates btree.sgml /
select.sgml.A small refactoring is also included.
Attachments:
[text/x-patch] v5-0001-Fix-qual-pushdown-using-btree-equalimage.patch (28.0K, ../../CAB8bMivMXBOL3Utbaw0wmsNo76fP5H6aeSvek-92jzPH8ZWqGA@mail.gmail.com/3-v5-0001-Fix-qual-pushdown-using-btree-equalimage.patch)
download | inline diff:
From 6785bca7570a7ec0a2b6eec8aeac1e50886bc1b0 Mon Sep 17 00:00:00 2001
From: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Date: Sun, 6 Sep 2026 15:15:31 +0500
Subject: [PATCH v5] Fix qual pushdown using btree equalimage
Refuse to push a non-operand reference to a grouping column below the
grouping boundary unless the grouping equality is image equality
(BTEQUALIMAGE_PROC). That covers jsonb, numeric, float8 and similar
types, and subsumes the old nondeterministic-collation check for
wrapped references.
Cache the default btree equalimage support procedure OID in
TypeCacheEntry. Add type_is_equalimage() for eager aggregation,
opfamily_is_equalimage() for btree deduplication, and
equality_op_is_equalimage() for the pushdown walker.
Document in select.sgml that when equality merges non-interchangeable
images, which member appears after GROUP BY, DISTINCT, or a set
operation other than UNION ALL is unspecified. Update btree.sgml so
equalimage is not described as index-only.
Bug: #19649
Author: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Reported-by: chunling qin <303677365@qq.com>
---
doc/src/sgml/btree.sgml | 9 +-
doc/src/sgml/ref/select.sgml | 16 +++
src/backend/access/nbtree/nbtutils.c | 17 +---
src/backend/optimizer/plan/initsplan.c | 30 +-----
src/backend/optimizer/util/clauses.c | 37 ++++---
src/backend/optimizer/util/relnode.c | 30 +-----
src/backend/utils/cache/lsyscache.c | 90 +++++++++++++++++
src/backend/utils/cache/typcache.c | 49 ++++++++-
src/include/utils/lsyscache.h | 2 +
src/include/utils/typcache.h | 4 +
src/test/regress/expected/subselect.out | 128 ++++++++++++++++++++++++
src/test/regress/sql/subselect.sql | 60 +++++++++++
12 files changed, 383 insertions(+), 89 deletions(-)
diff --git a/doc/src/sgml/btree.sgml b/doc/src/sgml/btree.sgml
index 027361f20bb..704c1c74e55 100644
--- a/doc/src/sgml/btree.sgml
+++ b/doc/src/sgml/btree.sgml
@@ -464,9 +464,12 @@ returns bool
<function>equalimage</function> (<quote>equality implies image
equality</quote>) support functions, registered under support
function number 4. These functions allow the core code to
- determine when it is safe to apply the btree deduplication
- optimization. Currently, <function>equalimage</function>
- functions are only called when building or rebuilding an index.
+ determine when two values that compare equal may be freely
+ substituted for one another. They are called when building or
+ rebuilding an index, to decide whether the btree deduplication
+ optimization is safe, and during query planning, to decide whether
+ an optimization that merges equal values may discard distinctions
+ among those values.
</para>
<para>
An <function>equalimage</function> function must have the
diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
index 68fb4911769..4060f2bc564 100644
--- a/doc/src/sgml/ref/select.sgml
+++ b/doc/src/sgml/ref/select.sgml
@@ -857,6 +857,22 @@ GROUP BY [ ALL | DISTINCT ] <replaceable class="parameter">grouping_element</rep
input-column name rather than an output column name.
</para>
+ <para>
+ For some data types, values that compare as equal under the type's
+ equality operator are not interchangeable in every expression.
+ Examples include <type>numeric</type> values that differ only in
+ display scale, <type>float8</type> <literal>0</literal> and
+ <literal>-0</literal>, and <type>jsonb</type> numbers written with
+ different amounts of trailing precision. When such values are
+ merged by <literal>GROUP BY</literal>, <literal>DISTINCT</literal>,
+ or a set operation other than <literal>UNION ALL</literal>, which
+ member of each set of equals appears in the output (and therefore
+ what a distinguishing expression such as a cast to <type>text</type>
+ sees) is unspecified. The same holds for <literal>DISTINCT ON</literal>
+ unless <literal>ORDER BY</literal> determines which row of each set is
+ kept (see <xref linkend="sql-distinct"/>).
+ </para>
+
<para>
If any of <literal>GROUPING SETS</literal>, <literal>ROLLUP</literal> or
<literal>CUBE</literal> are present as grouping elements, then the
diff --git a/src/backend/access/nbtree/nbtutils.c b/src/backend/access/nbtree/nbtutils.c
index 014faa1622f..e525bdf1847 100644
--- a/src/backend/access/nbtree/nbtutils.c
+++ b/src/backend/access/nbtree/nbtutils.c
@@ -1183,21 +1183,12 @@ _bt_allequalimage(Relation rel, bool debugmessage)
for (int i = 0; i < IndexRelationGetNumberOfKeyAttributes(rel); i++)
{
- Oid opfamily = rel->rd_opfamily[i];
- Oid opcintype = rel->rd_opcintype[i];
- Oid collation = rel->rd_indcollation[i];
- Oid equalimageproc;
-
- equalimageproc = get_opfamily_proc(opfamily, opcintype, opcintype,
- BTEQUALIMAGE_PROC);
-
/*
- * If there is no BTEQUALIMAGE_PROC then deduplication is assumed to
- * be unsafe. Otherwise, actually call proc and see what it says.
+ * An opclass that lacks a BTEQUALIMAGE_PROC, or whose procedure
+ * returns false, makes deduplication unsafe for the whole index.
*/
- if (!OidIsValid(equalimageproc) ||
- !DatumGetBool(OidFunctionCall1Coll(equalimageproc, collation,
- ObjectIdGetDatum(opcintype))))
+ if (!opfamily_is_equalimage(rel->rd_opfamily[i], rel->rd_opcintype[i],
+ rel->rd_indcollation[i]))
{
allequalimage = false;
break;
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index fb6f81453ea..e4c0e432fac 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -14,7 +14,6 @@
*/
#include "postgres.h"
-#include "access/nbtree.h"
#include "access/sysattr.h"
#include "catalog/pg_constraint.h"
#include "catalog/pg_type.h"
@@ -884,8 +883,6 @@ create_grouping_expr_infos(PlannerInfo *root)
{
SortGroupClause *sgc = lfirst_node(SortGroupClause, lc);
TargetEntry *tle = get_sortgroupclause_tle(sgc, root->processed_tlist);
- TypeCacheEntry *tce;
- Oid equalimageproc;
Assert(tle->ressortgroupref > 0);
@@ -903,30 +900,11 @@ create_grouping_expr_infos(PlannerInfo *root)
*
* For instance, the NUMERIC data type is not supported, as values
* that are considered equal by the equality operator (e.g., 0 and
- * 0.0) can have different scales.
+ * 0.0) can have different scales. Pass the expression's actual
+ * collation rather than the type default.
*/
- tce = lookup_type_cache(exprType((Node *) tle->expr),
- TYPECACHE_BTREE_OPFAMILY);
- if (!OidIsValid(tce->btree_opf) ||
- !OidIsValid(tce->btree_opintype))
- return;
-
- equalimageproc = get_opfamily_proc(tce->btree_opf,
- tce->btree_opintype,
- tce->btree_opintype,
- BTEQUALIMAGE_PROC);
-
- /*
- * If there is no BTEQUALIMAGE_PROC, eager aggregation is assumed to
- * be unsafe. Otherwise, we call the procedure to check. We must be
- * careful to pass the expression's actual collation, rather than the
- * data type's default collation, to ensure that non-deterministic
- * collations are correctly handled.
- */
- if (!OidIsValid(equalimageproc) ||
- !DatumGetBool(OidFunctionCall1Coll(equalimageproc,
- exprCollation((Node *) tle->expr),
- ObjectIdGetDatum(tce->btree_opintype))))
+ if (!type_is_equalimage(exprType((Node *) tle->expr),
+ exprCollation((Node *) tle->expr)))
return;
exprs = lappend(exprs, tle->expr);
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 55cebe4a74b..02052402595 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -6414,18 +6414,18 @@ pull_paramids_walker(Node *node, Bitmapset **context)
* semantics compatible with the grouping eqop, or, for a nondeterministic
* collation, when the comparison applies a collation other than the column's.
*
- * For a nondeterministic collation, every other reference is rejected: a
- * comparison under a different collation, and any function or operator over
- * the column, because we cannot tell whether the function yields the same
- * result for values the grouping treats as equal, and many do not. A column
- * with a deterministic collation is not restricted this way.
+ * Every other reference -- a wrapper, a non-comparison operator, or a bare
+ * boolean column -- is opaque to us. Accept it only when the grouping's
+ * equality is image equality (see equality_op_is_equalimage). Then values
+ * the grouping merges are interchangeable for ordinary expressions.
+ * Otherwise a wrapper such as ::text can tell apart numeric 1 and 1.0,
+ * jsonb 1 and 1.0, or float8 0 and -0. This also covers text under a
+ * nondeterministic collation: the equalimage procedure answers false for
+ * those collations.
*
- * This leaves one case uncaught: with a deterministic collation, a function
- * over the column can still feed a finer comparison than the direct-operand
- * check sees, for example record_image_ops over a rebuilt record, or scale()
- * over numeric where two equal values differ in scale. Catching it would
- * require knowing that a type's equality is bitwise, which we do not test
- * here.
+ * Image equality is not quite bitwise equality for varlena (TOAST). We do
+ * not try to catch expressions that expose physical representation, such as
+ * pg_column_size().
*
* Returns true if any such conflict exists.
*/
@@ -6477,18 +6477,17 @@ grouping_conflict_walker(Node *node, grouping_walker_ctx *ctx)
if (IsA(node, Var))
{
Var *var = (Var *) node;
+ Oid grouping_eqop = ctx->get_eqop(var, ctx->cb_context);
/*
* A grouping column reaches here when it was not handled as a direct
- * operand by a comparison node above (see the function header). That
- * is safe for a deterministic collation, but not for a
- * nondeterministic one, where the reference may distinguish values
- * the grouping considers equal. A bare boolean qual is safe too:
- * boolean is not collatable, so it takes the deterministic path here.
+ * operand by a comparison node above. Accept it only if grouping
+ * equality is image equality. That subsumes the old
+ * nondeterministic-collation check. A bare boolean qual stays safe:
+ * boolean equality is image equality.
*/
- if (OidIsValid(ctx->get_eqop(var, ctx->cb_context)) &&
- OidIsValid(var->varcollid) &&
- !get_collation_isdeterministic(var->varcollid))
+ if (OidIsValid(grouping_eqop) &&
+ !equality_op_is_equalimage(grouping_eqop, var->varcollid))
return true;
return false;
}
diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c
index ee69f81945f..327593e282b 100644
--- a/src/backend/optimizer/util/relnode.c
+++ b/src/backend/optimizer/util/relnode.c
@@ -16,7 +16,6 @@
#include <limits.h>
-#include "access/nbtree.h"
#include "catalog/pg_constraint.h"
#include "miscadmin.h"
#include "nodes/nodeFuncs.h"
@@ -3037,36 +3036,15 @@ init_grouping_targets(PlannerInfo *root, RelOptInfo *rel,
* 'destiny', which is crucial for maintaining correctness.
*/
SortGroupClause *sgc;
- TypeCacheEntry *tce;
- Oid equalimageproc;
/*
* But first, check if equality implies image equality for this
* expression. If not, we cannot use it as a grouping key. See
- * comments in create_grouping_expr_infos().
+ * comments in create_grouping_expr_infos(). Pass the
+ * expression's actual collation rather than the type default.
*/
- tce = lookup_type_cache(exprType((Node *) expr),
- TYPECACHE_BTREE_OPFAMILY);
- if (!OidIsValid(tce->btree_opf) ||
- !OidIsValid(tce->btree_opintype))
- return false;
-
- equalimageproc = get_opfamily_proc(tce->btree_opf,
- tce->btree_opintype,
- tce->btree_opintype,
- BTEQUALIMAGE_PROC);
-
- /*
- * If there is no BTEQUALIMAGE_PROC, eager aggregation is assumed
- * to be unsafe. Otherwise, we call the procedure to check. We
- * must be careful to pass the expression's actual collation,
- * rather than the data type's default collation, to ensure that
- * non-deterministic collations are correctly handled.
- */
- if (!OidIsValid(equalimageproc) ||
- !DatumGetBool(OidFunctionCall1Coll(equalimageproc,
- exprCollation((Node *) expr),
- ObjectIdGetDatum(tce->btree_opintype))))
+ if (!type_is_equalimage(exprType((Node *) expr),
+ exprCollation((Node *) expr)))
return false;
/* Create the SortGroupClause. */
diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index 9ef3922d17c..859abb1312e 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -17,6 +17,7 @@
#include "access/hash.h"
#include "access/htup_details.h"
+#include "access/nbtree.h"
#include "bootstrap/bootstrap.h"
#include "catalog/namespace.h"
#include "catalog/pg_am.h"
@@ -1038,6 +1039,95 @@ get_opfamily_proc(Oid opfamily, Oid lefttype, Oid righttype, int16 procnum)
return result;
}
+/*
+ * opfamily_is_equalimage
+ * Return true if opfamily promises "equality implies image equality"
+ * for the given input type and collation.
+ *
+ * A true result means that whenever the opfamily's ordering method reports
+ * two values equal, those values are interchangeable without loss of
+ * semantic information. Used by B-tree deduplication and by
+ * equality_op_is_equalimage(). Callers that know a type OID rather than an
+ * opfamily should use type_is_equalimage() instead, which caches the support
+ * procedure in TypeCacheEntry.
+ *
+ * An opfamily that registers no BTEQUALIMAGE_PROC makes no such promise.
+ * Pass the collation actually in use, not the type's default.
+ */
+bool
+opfamily_is_equalimage(Oid opfamily, Oid opcintype, Oid collation)
+{
+ Oid equalimageproc;
+
+ equalimageproc = get_opfamily_proc(opfamily, opcintype, opcintype,
+ BTEQUALIMAGE_PROC);
+ if (!OidIsValid(equalimageproc))
+ return false;
+
+ return DatumGetBool(OidFunctionCall1Coll(equalimageproc, collation,
+ ObjectIdGetDatum(opcintype)));
+}
+
+/*
+ * equality_op_is_equalimage
+ * Return true if eqop defines an equivalence under which equal values
+ * are interchangeable without loss of semantic information.
+ *
+ * Used when we know a grouping equality operator and not its opfamily.
+ *
+ * When eqop is the type's default equality operator, defer to
+ * type_is_equalimage(). Otherwise require a promise from every mergejoin
+ * opfamily in which eqop is the equality member: texteq belongs to both
+ * text_ops and text_pattern_ops, and under a nondeterministic collation they
+ * disagree. text_pattern_ops registers btequalimage for any collation, so
+ * trusting it alone would be wrong for a case-insensitive grouping.
+ *
+ * A false result means "not proven". Cross-type operators always land there.
+ * 'collation' must be the collation actually applied to the values.
+ */
+bool
+equality_op_is_equalimage(Oid eqop, Oid collation)
+{
+ Oid lefttype;
+ Oid righttype;
+ TypeCacheEntry *typentry;
+ List *opfamilies;
+ bool result;
+ ListCell *lc;
+
+ op_input_types(eqop, &lefttype, &righttype);
+
+ /* Equalimage describes one type. Grouping eqops are never cross-type. */
+ if (lefttype != righttype)
+ return false;
+
+ /*
+ * Common case: grouping uses the type's default equality. Share the
+ * typcache path used by eager aggregation.
+ */
+ typentry = lookup_type_cache(lefttype, TYPECACHE_EQ_OPR);
+ if (OidIsValid(typentry->eq_opr) && eqop == typentry->eq_opr)
+ return type_is_equalimage(lefttype, collation);
+
+ opfamilies = get_mergejoin_opfamilies(eqop);
+
+ /* No ordering opfamily at all means nothing promised anything. */
+ result = (opfamilies != NIL);
+
+ foreach(lc, opfamilies)
+ {
+ if (!opfamily_is_equalimage(lfirst_oid(lc), lefttype, collation))
+ {
+ result = false;
+ break;
+ }
+ }
+
+ list_free(opfamilies);
+
+ return result;
+}
+
/* ---------- ATTRIBUTE CACHES ---------- */
diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c
index eca2d73231a..59970765891 100644
--- a/src/backend/utils/cache/typcache.c
+++ b/src/backend/utils/cache/typcache.c
@@ -117,6 +117,7 @@ static TypeCacheEntry *firstDomainTypeEntry = NULL;
#define TCFLAGS_HAVE_FIELD_EXTENDED_HASHING 0x040000
#define TCFLAGS_CHECKED_DOMAIN_CONSTRAINTS 0x080000
#define TCFLAGS_DOMAIN_BASE_IS_COMPOSITE 0x100000
+#define TCFLAGS_CHECKED_EQUALIMAGE_PROC 0x200000
/* The flags associated with equality/comparison/hashing are all but these: */
#define TCFLAGS_OPERATOR_FLAGS \
@@ -584,7 +585,7 @@ lookup_type_cache(Oid type_id, int flags)
if ((flags & (TYPECACHE_EQ_OPR | TYPECACHE_LT_OPR | TYPECACHE_GT_OPR |
TYPECACHE_CMP_PROC |
TYPECACHE_EQ_OPR_FINFO | TYPECACHE_CMP_PROC_FINFO |
- TYPECACHE_BTREE_OPFAMILY)) &&
+ TYPECACHE_BTREE_OPFAMILY | TYPECACHE_EQUALIMAGE_PROC)) &&
!(typentry->flags & TCFLAGS_CHECKED_BTREE_OPCLASS))
{
Oid opclass;
@@ -609,7 +610,8 @@ lookup_type_cache(Oid type_id, int flags)
typentry->flags &= ~(TCFLAGS_CHECKED_EQ_OPR |
TCFLAGS_CHECKED_LT_OPR |
TCFLAGS_CHECKED_GT_OPR |
- TCFLAGS_CHECKED_CMP_PROC);
+ TCFLAGS_CHECKED_CMP_PROC |
+ TCFLAGS_CHECKED_EQUALIMAGE_PROC);
typentry->flags |= TCFLAGS_CHECKED_BTREE_OPCLASS;
}
@@ -780,6 +782,25 @@ lookup_type_cache(Oid type_id, int flags)
typentry->cmp_proc = cmp_proc;
typentry->flags |= TCFLAGS_CHECKED_CMP_PROC;
}
+ if ((flags & TYPECACHE_EQUALIMAGE_PROC) &&
+ !(typentry->flags & TCFLAGS_CHECKED_EQUALIMAGE_PROC))
+ {
+ Oid equalimage_proc = InvalidOid;
+
+ /*
+ * Cache only the support-function OID. Whether equality implies
+ * image equality can still depend on collation, so callers must
+ * invoke the procedure with the collation actually in use.
+ */
+ if (typentry->btree_opf != InvalidOid)
+ equalimage_proc = get_opfamily_proc(typentry->btree_opf,
+ typentry->btree_opintype,
+ typentry->btree_opintype,
+ BTEQUALIMAGE_PROC);
+
+ typentry->equalimage_proc = equalimage_proc;
+ typentry->flags |= TCFLAGS_CHECKED_EQUALIMAGE_PROC;
+ }
if ((flags & (TYPECACHE_HASH_PROC | TYPECACHE_HASH_PROC_FINFO)) &&
!(typentry->flags & TCFLAGS_CHECKED_HASH_PROC))
{
@@ -980,6 +1001,30 @@ lookup_type_cache(Oid type_id, int flags)
return typentry;
}
+/*
+ * type_is_equalimage
+ * Return true if the type's default btree equality implies image
+ * equality under the given collation.
+ *
+ * This is the type-oriented counterpart of opfamily_is_equalimage(), for
+ * callers that know a type OID rather than an opfamily. The equalimage
+ * support procedure OID is cached in TypeCacheEntry; the boolean answer is
+ * not, because it can depend on collation.
+ */
+bool
+type_is_equalimage(Oid type_id, Oid collation)
+{
+ TypeCacheEntry *typentry;
+
+ typentry = lookup_type_cache(type_id, TYPECACHE_EQUALIMAGE_PROC);
+ if (!OidIsValid(typentry->equalimage_proc))
+ return false;
+
+ return DatumGetBool(OidFunctionCall1Coll(typentry->equalimage_proc,
+ collation,
+ ObjectIdGetDatum(typentry->btree_opintype)));
+}
+
/*
* load_typcache_tupdesc --- helper routine to set up composite type's tupDesc
*/
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index 865980cb0f1..09887ddf093 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -95,6 +95,8 @@ extern bool collations_agree_on_equality(Oid coll1, Oid coll2);
extern bool op_is_safe_index_member(Oid opno);
extern Oid get_opfamily_proc(Oid opfamily, Oid lefttype, Oid righttype,
int16 procnum);
+extern bool opfamily_is_equalimage(Oid opfamily, Oid opcintype, Oid collation);
+extern bool equality_op_is_equalimage(Oid eqop, Oid collation);
extern char *get_attname(Oid relid, AttrNumber attnum, bool missing_ok);
extern AttrNumber get_attnum(Oid relid, const char *attname);
extern char get_attgenerated(Oid relid, AttrNumber attnum);
diff --git a/src/include/utils/typcache.h b/src/include/utils/typcache.h
index 5a4aa9ec840..952f478266c 100644
--- a/src/include/utils/typcache.h
+++ b/src/include/utils/typcache.h
@@ -65,6 +65,7 @@ typedef struct TypeCacheEntry
Oid cmp_proc; /* the btree comparison function */
Oid hash_proc; /* the hash calculation function */
Oid hash_extended_proc; /* the extended hash calculation function */
+ Oid equalimage_proc; /* btree equalimage support function */
/*
* Pre-set-up fmgr call info for the equality operator, the btree
@@ -152,6 +153,7 @@ typedef struct TypeCacheEntry
#define TYPECACHE_HASH_EXTENDED_PROC 0x04000
#define TYPECACHE_HASH_EXTENDED_PROC_FINFO 0x08000
#define TYPECACHE_MULTIRANGE_INFO 0x10000
+#define TYPECACHE_EQUALIMAGE_PROC 0x20000
/* This value will not equal any valid tupledesc identifier, nor 0 */
#define INVALID_TUPLEDESC_IDENTIFIER ((uint64) 1)
@@ -178,6 +180,8 @@ typedef struct SharedRecordTypmodRegistry SharedRecordTypmodRegistry;
extern TypeCacheEntry *lookup_type_cache(Oid type_id, int flags);
+extern bool type_is_equalimage(Oid type_id, Oid collation);
+
extern void InitDomainConstraintRef(Oid type_id, DomainConstraintRef *ref,
MemoryContext refctx, bool need_exprstate);
diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out
index cf295d56507..1ca3094175e 100644
--- a/src/test/regress/expected/subselect.out
+++ b/src/test/regress/expected/subselect.out
@@ -2151,6 +2151,134 @@ WHERE (CASE id WHEN 1 THEN 1 ELSE 0 END) = 1;
Filter: (CASE id WHEN 1 THEN 1 ELSE 0 END = 1)
(5 rows)
+-- Wrapped references over grouped subqueries. When grouping equality is
+-- not image equality (jsonb, numeric), a wrapper must not be pushed below
+-- the grouping boundary. int equality is image equality, so i::text
+-- remains pushable.
+CREATE TEMP TABLE pdt_eqimg (id int, j jsonb, n numeric, i int);
+INSERT INTO pdt_eqimg VALUES
+ (1, '1', 1, 1),
+ (2, '1.0', 1.0, 1);
+-- jsonb DISTINCT ON: ::text wrapper stays above Unique
+EXPLAIN (COSTS OFF)
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_eqimg ORDER BY j, id) s
+WHERE j::text = '1.0';
+ QUERY PLAN
+---------------------------------------------------
+ Subquery Scan on s
+ Filter: ((s.j)::text = '1.0'::text)
+ -> Unique
+ -> Sort
+ Sort Key: pdt_eqimg.j, pdt_eqimg.id
+ -> Seq Scan on pdt_eqimg
+(6 rows)
+
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_eqimg ORDER BY j, id) s
+WHERE j::text = '1.0';
+ id | j
+----+---
+(0 rows)
+
+-- jsonb GROUP BY: ::text matching the group representative keeps count = 2
+EXPLAIN (COSTS OFF)
+SELECT c FROM (SELECT j, count(*) c FROM pdt_eqimg GROUP BY j) s
+WHERE j::text = '1';
+ QUERY PLAN
+---------------------------------------------------
+ Subquery Scan on s
+ -> HashAggregate
+ Group Key: pdt_eqimg.j
+ Filter: ((pdt_eqimg.j)::text = '1'::text)
+ -> Seq Scan on pdt_eqimg
+(5 rows)
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_eqimg GROUP BY j) s
+WHERE j::text = '1';
+ c
+---
+ 2
+(1 row)
+
+-- jsonb GROUP BY: other image yields no row, not a split group
+SELECT c FROM (SELECT j, count(*) c FROM pdt_eqimg GROUP BY j) s
+WHERE j::text = '1.0';
+ c
+---
+(0 rows)
+
+-- jsonb GROUP BY: same-eqop comparison remains pushable / correct
+EXPLAIN (COSTS OFF)
+SELECT c FROM (SELECT j, count(*) c FROM pdt_eqimg GROUP BY j) s
+WHERE j = '1'::jsonb;
+ QUERY PLAN
+----------------------------------------
+ Subquery Scan on s
+ -> GroupAggregate
+ -> Seq Scan on pdt_eqimg
+ Filter: (j = '1'::jsonb)
+(4 rows)
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_eqimg GROUP BY j) s
+WHERE j = '1'::jsonb;
+ c
+---
+ 2
+(1 row)
+
+-- jsonb GROUP BY: wrapped HAVING stays above the grouping
+EXPLAIN (COSTS OFF)
+SELECT j, count(*) FROM pdt_eqimg GROUP BY j
+HAVING starts_with(j::text, '1.');
+ QUERY PLAN
+----------------------------------------------
+ HashAggregate
+ Group Key: j
+ Filter: starts_with((j)::text, '1.'::text)
+ -> Seq Scan on pdt_eqimg
+(4 rows)
+
+SELECT j, count(*) FROM pdt_eqimg GROUP BY j
+HAVING starts_with(j::text, '1.');
+ j | count
+---+-------
+(0 rows)
+
+-- numeric GROUP BY: ::text wrapper stays on the Agg (not jsonb-only)
+EXPLAIN (COSTS OFF)
+SELECT c FROM (SELECT n, count(*) c FROM pdt_eqimg GROUP BY n) s
+WHERE n::text = '1';
+ QUERY PLAN
+---------------------------------------------------
+ Subquery Scan on s
+ -> HashAggregate
+ Group Key: pdt_eqimg.n
+ Filter: ((pdt_eqimg.n)::text = '1'::text)
+ -> Seq Scan on pdt_eqimg
+(5 rows)
+
+SELECT c FROM (SELECT n, count(*) c FROM pdt_eqimg GROUP BY n) s
+WHERE n::text = '1';
+ c
+---
+ 2
+(1 row)
+
+-- int GROUP BY: ::text is still pushed to Seq Scan
+SET enable_hashagg TO off;
+EXPLAIN (COSTS OFF)
+SELECT * FROM (SELECT i, count(*) c FROM pdt_eqimg GROUP BY i) s
+WHERE i::text = '5';
+ QUERY PLAN
+-----------------------------------------------
+ GroupAggregate
+ Group Key: pdt_eqimg.i
+ -> Sort
+ Sort Key: pdt_eqimg.i
+ -> Seq Scan on pdt_eqimg
+ Filter: ((i)::text = '5'::text)
+(6 rows)
+
+RESET enable_hashagg;
-- 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 07438694f6e..71295d2f4e7 100644
--- a/src/test/regress/sql/subselect.sql
+++ b/src/test/regress/sql/subselect.sql
@@ -1052,6 +1052,66 @@ EXPLAIN (COSTS OFF)
SELECT * FROM (SELECT DISTINCT id FROM pdt) s
WHERE (CASE id WHEN 1 THEN 1 ELSE 0 END) = 1;
+-- Wrapped references over grouped subqueries. When grouping equality is
+-- not image equality (jsonb, numeric), a wrapper must not be pushed below
+-- the grouping boundary. int equality is image equality, so i::text
+-- remains pushable.
+CREATE TEMP TABLE pdt_eqimg (id int, j jsonb, n numeric, i int);
+INSERT INTO pdt_eqimg VALUES
+ (1, '1', 1, 1),
+ (2, '1.0', 1.0, 1);
+
+-- jsonb DISTINCT ON: ::text wrapper stays above Unique
+EXPLAIN (COSTS OFF)
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_eqimg ORDER BY j, id) s
+WHERE j::text = '1.0';
+
+SELECT * FROM (SELECT DISTINCT ON (j) id, j FROM pdt_eqimg ORDER BY j, id) s
+WHERE j::text = '1.0';
+
+-- jsonb GROUP BY: ::text matching the group representative keeps count = 2
+EXPLAIN (COSTS OFF)
+SELECT c FROM (SELECT j, count(*) c FROM pdt_eqimg GROUP BY j) s
+WHERE j::text = '1';
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_eqimg GROUP BY j) s
+WHERE j::text = '1';
+
+-- jsonb GROUP BY: other image yields no row, not a split group
+SELECT c FROM (SELECT j, count(*) c FROM pdt_eqimg GROUP BY j) s
+WHERE j::text = '1.0';
+
+-- jsonb GROUP BY: same-eqop comparison remains pushable / correct
+EXPLAIN (COSTS OFF)
+SELECT c FROM (SELECT j, count(*) c FROM pdt_eqimg GROUP BY j) s
+WHERE j = '1'::jsonb;
+
+SELECT c FROM (SELECT j, count(*) c FROM pdt_eqimg GROUP BY j) s
+WHERE j = '1'::jsonb;
+
+-- jsonb GROUP BY: wrapped HAVING stays above the grouping
+EXPLAIN (COSTS OFF)
+SELECT j, count(*) FROM pdt_eqimg GROUP BY j
+HAVING starts_with(j::text, '1.');
+
+SELECT j, count(*) FROM pdt_eqimg GROUP BY j
+HAVING starts_with(j::text, '1.');
+
+-- numeric GROUP BY: ::text wrapper stays on the Agg (not jsonb-only)
+EXPLAIN (COSTS OFF)
+SELECT c FROM (SELECT n, count(*) c FROM pdt_eqimg GROUP BY n) s
+WHERE n::text = '1';
+
+SELECT c FROM (SELECT n, count(*) c FROM pdt_eqimg GROUP BY n) s
+WHERE n::text = '1';
+
+-- int GROUP BY: ::text is still pushed to Seq Scan
+SET enable_hashagg TO off;
+EXPLAIN (COSTS OFF)
+SELECT * FROM (SELECT i, count(*) c FROM pdt_eqimg GROUP BY i) s
+WHERE i::text = '5';
+RESET enable_hashagg;
+
-- 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.53.0
^ permalink raw reply [nested|flat] 9+ messages in thread
end of thread, other threads:[~2026-09-06 13:09 UTC | newest]
Thread overview: 9+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-09-03 06:41 BUG #19649: Qual pushdown into GROUP BY subqueries ignores non-equivalence-preserving references to grouping col PG Bug reporting form <noreply@postgresql.org>
2026-09-03 21:17 ` Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-09-04 15:12 ` Andrei Lepikhov <lepihov@gmail.com>
2026-09-04 16:10 ` Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-09-05 07:52 ` Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-09-05 10:45 ` Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-09-05 16:23 ` Andrei Lepikhov <lepihov@gmail.com>
2026-09-05 16:48 ` Andrey Rachitskiy <pl0h0yp1@gmail.com>
2026-09-06 13:09 ` Andrey Rachitskiy <pl0h0yp1@gmail.com>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox