Hi hackers, I used Opus to go over the new features in PG19, and it found wrong results from eager aggregation on a bpchar column with no length.
create table e1 (k int, pat text);
insert into e1 values (1, 'a'), (2, 'a ');
create table e2 (c bpchar, v int);
insert into e2 select case when g % 2 = 0 then 'a' else 'a ' end, 1
from generate_series(1, 20000) g;
analyze e1, e2;
select e1.k, count(*) from e1 join e2 on e2.c like e1.pat
group by e1.k order by 1;
18 returns
k | count
---+-------
1 | 10000
2 | 10000
19 and master return
k | count
---+-------
2 | 20000
bpchar equality ignores trailing spaces, but bpchar_ops and
bpchar_pattern_ops have an equalimage function, and eager aggregation
trusts it. nbtree deduplication does too, also on 18. In the attached
bpchar-nbtree-ios.sql the heap has octet_length 1 x 98 and 3 x 1, and
the index-only scan returns 1 x 99. BUG #19749 [1] has the assert.
The patches follow the interval_ops fix [2]. 0001 drops support
function 4 from both opclasses, catversion bump needed. 0002 is a
test. The .txt file is for back branches, where the equalimage
functions also return false for bpchar. Opus drafted them.
This also ends deduplication for char(n), where it was safe. Peter,
do you want to keep it there? Old indexes need REINDEX, and amcheck
flags them until then, so I am not sure about the back branches. The
eager aggregation part is new in 19, so I suggest an open item.
[1] https://postgr.es/m/[email protected]
[2] https://postgr.es/m/[email protected]
Thanks,
Shihao
From 0737d0776fc65f044ee409ae8767c71da0cb9ddb Mon Sep 17 00:00:00 2001 From: shihao zhong <[email protected]> Date: Mon, 5 Oct 2026 11:55:08 -0400 Subject: [PATCH] Remove equalimage support from bpchar_ops and bpchar_pattern_ops bpchar equality ignores trailing spaces, so equal values in a column with no length can have different images. Deduplication could make an index-only scan return the wrong value. Drop support function 4 from both opclasses, like interval_ops. For clusters with old catalogs, make the two equalimage functions return false for bpchar. Existing indexes need a REINDEX. Bug: #19749 Reported-by: Ke <[email protected]> Discussion: https://postgr.es/m/[email protected] --- contrib/amcheck/verify_nbtree.c | 15 ++++++--- doc/src/sgml/btree.sgml | 9 +++++- src/backend/utils/adt/datum.c | 14 ++++----- src/backend/utils/adt/varlena.c | 9 +++++- src/include/catalog/pg_amproc.dat | 5 --- src/test/regress/expected/opr_sanity.out | 39 ++++++++++++------------ 6 files changed, 53 insertions(+), 38 deletions(-) diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c index c7cd8d3f043..37dd80347da 100644 --- a/contrib/amcheck/verify_nbtree.c +++ b/contrib/amcheck/verify_nbtree.c @@ -331,13 +331,18 @@ bt_index_check_callback(Relation indrel, Relation heaprel, void *state, bool rea if (allequalimage && !_bt_allequalimage(indrel, false)) { bool has_interval_ops = false; + bool has_bpchar_ops = false; for (int i = 0; i < IndexRelationGetNumberOfKeyAttributes(indrel); i++) - if (indrel->rd_opfamily[i] == INTERVAL_BTREE_FAM_OID) - { + { + Oid opfamily = indrel->rd_opfamily[i]; + + if (opfamily == INTERVAL_BTREE_FAM_OID) has_interval_ops = true; - break; - } + else if (opfamily == BPCHAR_BTREE_FAM_OID || + opfamily == BPCHAR_PATTERN_BTREE_FAM_OID) + has_bpchar_ops = true; + } ereport(ERROR, (errcode(ERRCODE_INDEX_CORRUPTED), @@ -345,6 +350,8 @@ bt_index_check_callback(Relation indrel, Relation heaprel, void *state, bool rea RelationGetRelationName(indrel)), has_interval_ops ? errhint("This is known of \"interval\" indexes last built on a version predating 2023-11.") + : has_bpchar_ops + ? errhint("This is known of \"character\" indexes last built on a version predating 2026-11.") : 0)); } diff --git a/doc/src/sgml/btree.sgml b/doc/src/sgml/btree.sgml index 027361f20bb..3e5ca7a4430 100644 --- a/doc/src/sgml/btree.sgml +++ b/doc/src/sgml/btree.sgml @@ -875,13 +875,20 @@ options(<replaceable>relopts</replaceable> <type>local_relopts *</type>) returns <itemizedlist> <listitem> <para> - <type>text</type>, <type>varchar</type>, and <type>char</type> + <type>text</type> and <type>varchar</type> cannot use deduplication when a <emphasis>nondeterministic</emphasis> collation is used. Case and accent differences must be preserved among equal datums. </para> </listitem> + <listitem> + <para> + <type>char</type> cannot use deduplication. Trailing spaces + must be preserved among equal datums. + </para> + </listitem> + <listitem> <para> <type>numeric</type> cannot use deduplication. Numeric display diff --git a/src/backend/utils/adt/datum.c b/src/backend/utils/adt/datum.c index 5aab1edb7bc..2e906add921 100644 --- a/src/backend/utils/adt/datum.c +++ b/src/backend/utils/adt/datum.c @@ -43,6 +43,7 @@ #include "postgres.h" #include "access/detoast.h" +#include "catalog/pg_type_d.h" #include "common/hashfn.h" #include "fmgr.h" #include "utils/datum.h" @@ -421,20 +422,17 @@ datum_image_hash(Datum value, bool typByVal, int typLen) * datum_image_eq() in all cases can use this as their "equalimage" support * function. * - * Currently, we unconditionally assume that any B-Tree operator class that - * registers btequalimage as its support function 4 must be able to safely use - * optimizations like deduplication (i.e. we return true unconditionally). If - * it ever proved necessary to rescind support for an operator class, we could - * do that in a targeted fashion by doing something with the opcintype - * argument. + * Earlier minor releases erroneously associated this function with + * bpchar_pattern_ops. Detect that case to rescind deduplication support, + * without requiring initdb. *------------------------------------------------------------------------- */ Datum btequalimage(PG_FUNCTION_ARGS) { - /* Oid opcintype = PG_GETARG_OID(0); */ + Oid opcintype = PG_GETARG_OID(0); - PG_RETURN_BOOL(true); + PG_RETURN_BOOL(opcintype != BPCHAROID); } /*------------------------------------------------------------------------- diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c index 63662e3f779..51f6ab0960f 100644 --- a/src/backend/utils/adt/varlena.c +++ b/src/backend/utils/adt/varlena.c @@ -2655,16 +2655,23 @@ varstr_abbrev_abort(int memtupcount, SortSupport ssup) /* * Generic equalimage support function for character type's operator classes. * Disables the use of deduplication with nondeterministic collations. + * + * Earlier minor releases erroneously associated this function with + * bpchar_ops. Detect that case to rescind deduplication support, without + * requiring initdb. */ Datum btvarstrequalimage(PG_FUNCTION_ARGS) { - /* Oid opcintype = PG_GETARG_OID(0); */ + Oid opcintype = PG_GETARG_OID(0); Oid collid = PG_GET_COLLATION(); pg_locale_t locale; check_collation_set(collid); + if (opcintype == BPCHAROID) + PG_RETURN_BOOL(false); + locale = pg_newlocale_from_collation(collid); PG_RETURN_BOOL(locale->deterministic); diff --git a/src/include/catalog/pg_amproc.dat b/src/include/catalog/pg_amproc.dat index e3477500baa..6d35e50abbf 100644 --- a/src/include/catalog/pg_amproc.dat +++ b/src/include/catalog/pg_amproc.dat @@ -30,9 +30,6 @@ { amprocfamily => 'btree/bpchar_ops', amproclefttype => 'bpchar', amprocrighttype => 'bpchar', amprocnum => '2', amproc => 'bpchar_sortsupport' }, -{ amprocfamily => 'btree/bpchar_ops', amproclefttype => 'bpchar', - amprocrighttype => 'bpchar', amprocnum => '4', - amproc => 'btvarstrequalimage' }, { amprocfamily => 'btree/bytea_ops', amproclefttype => 'bytea', amprocrighttype => 'bytea', amprocnum => '1', amproc => 'byteacmp' }, { amprocfamily => 'btree/bytea_ops', amproclefttype => 'bytea', @@ -265,8 +262,6 @@ { amprocfamily => 'btree/bpchar_pattern_ops', amproclefttype => 'bpchar', amprocrighttype => 'bpchar', amprocnum => '2', amproc => 'btbpchar_pattern_sortsupport' }, -{ amprocfamily => 'btree/bpchar_pattern_ops', amproclefttype => 'bpchar', - amprocrighttype => 'bpchar', amprocnum => '4', amproc => 'btequalimage' }, { amprocfamily => 'btree/money_ops', amproclefttype => 'money', amprocrighttype => 'money', amprocnum => '1', amproc => 'cash_cmp' }, { amprocfamily => 'btree/money_ops', amproclefttype => 'money', diff --git a/src/test/regress/expected/opr_sanity.out b/src/test/regress/expected/opr_sanity.out index 20bf9ea9cdf..6c492b56538 100644 --- a/src/test/regress/expected/opr_sanity.out +++ b/src/test/regress/expected/opr_sanity.out @@ -2213,25 +2213,26 @@ LEFT JOIN pg_amproc AS amp ON amp.amprocfamily = opf.oid AND WHERE am.amname = 'btree' AND amp.amproc IS DISTINCT FROM 'btequalimage'::regproc ORDER BY 1, 2, 3; - proc | opfamily_name | opclass_name | opcintype ---------------------+------------------+------------------+------------------ - btvarstrequalimage | bpchar_ops | bpchar_ops | character - btvarstrequalimage | text_ops | name_ops | name - btvarstrequalimage | text_ops | text_ops | text - btvarstrequalimage | text_ops | varchar_ops | text - | array_ops | array_ops | anyarray - | float_ops | float4_ops | real - | float_ops | float8_ops | double precision - | interval_ops | interval_ops | interval - | jsonb_ops | jsonb_ops | jsonb - | multirange_ops | multirange_ops | anymultirange - | numeric_ops | numeric_ops | numeric - | range_ops | range_ops | anyrange - | record_image_ops | record_image_ops | record - | record_ops | record_ops | record - | tsquery_ops | tsquery_ops | tsquery - | tsvector_ops | tsvector_ops | tsvector -(16 rows) + proc | opfamily_name | opclass_name | opcintype +--------------------+--------------------+--------------------+------------------ + btvarstrequalimage | text_ops | name_ops | name + btvarstrequalimage | text_ops | text_ops | text + btvarstrequalimage | text_ops | varchar_ops | text + | array_ops | array_ops | anyarray + | bpchar_ops | bpchar_ops | character + | bpchar_pattern_ops | bpchar_pattern_ops | character + | float_ops | float4_ops | real + | float_ops | float8_ops | double precision + | interval_ops | interval_ops | interval + | jsonb_ops | jsonb_ops | jsonb + | multirange_ops | multirange_ops | anymultirange + | numeric_ops | numeric_ops | numeric + | range_ops | range_ops | anyrange + | record_image_ops | record_image_ops | record + | record_ops | record_ops | record + | tsquery_ops | tsquery_ops | tsquery + | tsvector_ops | tsvector_ops | tsvector +(17 rows) -- **************** pg_index **************** -- Look for illegal values in pg_index fields. -- 2.37.1 (Apple Git-137.1)
v1-0001-Remove-equalimage-support-from-bpchar_ops-and-bpc.patch
Description: Binary data
v1-0002-Add-test-for-eager-aggregation-with-a-bpchar-colu.patch
Description: Binary data
