agora inbox for pgsql-bugs@postgresql.org
help / color / mirror / Atom feedFrom: Andrei Lepikhov <lepihov@gmail.com>
To: Andrey Rachitskiy <pl0h0yp1@gmail.com>
Cc: 303677365@qq.com, pgsql-bugs@lists.postgresql.org, Tender Wang <tndrwang@gmail.com>
Subject: Re: BUG #19649: Qual pushdown into GROUP BY subqueries ignores non-equivalence-preserving references to grouping col
Date: Sat, 5 Sep 2026 18:23:52 +0200
Message-ID: <7c27d19d-3324-4977-a8b2-ea89af0eac79@gmail.com> (raw)
In-Reply-To: <CAB8bMisiMCwe3EQoP_3_4acvfsqcDKfTOf5iHdqE8JGLcqvDBg@mail.gmail.com>
References: <19649-2cabf1440793cc71@postgresql.org>
<CAB8bMiss5S2SZonboQfbTMJsNCd+k_J8AbeS-vXheVUrR_TUhQ@mail.gmail.com>
<60c6b1f7-f435-4ca7-992a-482417af5130@gmail.com>
<CAB8bMivStAVVSPonHGeP0-Uxb5bd2mgLNyBunYxHG1s+UxnKOQ@mail.gmail.com>
<CAB8bMivO2qD9obEMdPxDpQ+Zanq_e_fHA2c1tKe=YeRxjnbXWA@mail.gmail.com>
<CAB8bMisiMCwe3EQoP_3_4acvfsqcDKfTOf5iHdqE8JGLcqvDBg@mail.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
view thread (9+ messages) latest in thread
Message-ID: <7c27d19d-3324-4977-a8b2-ea89af0eac79@gmail.com>
Permalink: ../7c27d19d-3324-4977-a8b2-ea89af0eac79@gmail.com/
Also on: postgresql.org/message-id/7c27d19d-3324-4977-a8b2-ea89af0eac79@gmail.com
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: lepihov@gmail.com, pl0h0yp1@gmail.com, tndrwang@gmail.com
Subject: Re: BUG #19649: Qual pushdown into GROUP BY subqueries ignores non-equivalence-preserving references to grouping col
In-Reply-To: <7c27d19d-3324-4977-a8b2-ea89af0eac79@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox