From 882fd416d6ed605de09c5ae636bf753bcd5f47d0 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Tue, 6 Oct 2026 18:21:35 -0400 Subject: [PATCH] Rescind unsafe deduplication support. bpchar equality ignores trailing spaces, so equal values in a column with no length specifier can have different images. Similarly, oidvector equality intentionally pays no attention to the array lower bound field. We nevertheless allowed nbtree deduplication to be used with indexes built with bpchar_ops, bpchar_pattern_ops, or oidvector_ops. A posting list split could therefore place a TID in an index tuple whose key was equal to, but not bytewise identical to, the corresponding table value. This allowed index-only scans to return subtly wrong results. To fix, drop support function 4 from the affected opclasses (we can't change catalog contents on branches 14 through 18, so we just disable deduplication by adding hard-coding to the relevant support functions). Deliberately leave the oidvector opclass alone on branches 14 through 18, though. Disallowing oidvector deduplication on these branches would make pg_amcheck report that pg_proc_proname_args_nsp_index has a corrupt metapage, needlessly alarming users. System catalog oidvector values always have a lower bound of 0, so the issue can't cause harm there; a user can only hit it by indexing an oidvector column that stores the output of a function like trim_array(). It seems very unlikely that any user would ever do that. Users should REINDEX any B-Tree index on a bpchar column. Author: Shihao Zhong Co-authored-by: Peter Geoghegan Reported-by: Ke Reported-by: Tom Lane Reviewed-by: Tom Lane Bug: #19749 Discussion: https://postgr.es/m/19749-d46adf7c7d910b07@postgresql.org Discussion: https://postgr.es/m/CAGRkXqQQ311Lg=62x5UdZtPuRFgpT6XpPMw4X9E3tgbTu162Mw@mail.gmail.com Backpatch-through: 14 --- contrib/amcheck/verify_nbtree.c | 17 +---------------- doc/src/sgml/btree.sgml | 16 ++++++++++++---- src/backend/utils/adt/datum.c | 14 ++++++-------- src/backend/utils/adt/varlena.c | 9 ++++++++- src/test/regress/expected/btree_index.out | 2 -- src/test/regress/sql/btree_index.sql | 3 --- 6 files changed, 27 insertions(+), 34 deletions(-) diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c index c7cd8d3f043..5d98d8a2ad3 100644 --- a/contrib/amcheck/verify_nbtree.c +++ b/contrib/amcheck/verify_nbtree.c @@ -33,7 +33,6 @@ #include "verify_common.h" #include "catalog/index.h" #include "catalog/pg_am.h" -#include "catalog/pg_opfamily_d.h" #include "common/pg_prng.h" #include "lib/bloomfilter.h" #include "miscadmin.h" @@ -329,24 +328,10 @@ bt_index_check_callback(Relation indrel, Relation heaprel, void *state, bool rea errmsg("index \"%s\" metapage has equalimage field set on unsupported nbtree version", RelationGetRelationName(indrel)))); if (allequalimage && !_bt_allequalimage(indrel, false)) - { - bool has_interval_ops = false; - - for (int i = 0; i < IndexRelationGetNumberOfKeyAttributes(indrel); i++) - if (indrel->rd_opfamily[i] == INTERVAL_BTREE_FAM_OID) - { - has_interval_ops = true; - break; - } - ereport(ERROR, (errcode(ERRCODE_INDEX_CORRUPTED), errmsg("index \"%s\" metapage incorrectly indicates that deduplication is safe", - RelationGetRelationName(indrel)), - has_interval_ops - ? errhint("This is known of \"interval\" indexes last built on a version predating 2023-11.") - : 0)); - } + RelationGetRelationName(indrel)))); /* Check index, possibly against table it is an index on */ bt_check_every_level(indrel, heaprel, heapkeyspace, readonly, diff --git a/doc/src/sgml/btree.sgml b/doc/src/sgml/btree.sgml index 027361f20bb..3eef5202c56 100644 --- a/doc/src/sgml/btree.sgml +++ b/doc/src/sgml/btree.sgml @@ -875,10 +875,18 @@ options(relopts local_relopts *) returns - text, varchar, and char - cannot use deduplication when a - nondeterministic collation is used. Case - and accent differences must be preserved among equal datums. + text and varchar cannot use deduplication when + a nondeterministic collation is used. Case and + accent differences must be preserved among equal datums. + + + + + + character (and its aliases char and + bpchar) cannot use deduplication. Trailing spaces are + disregarded when comparing two values, but must be preserved among + equal datums. 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/test/regress/expected/btree_index.out b/src/test/regress/expected/btree_index.out index 21dc9b5783a..9fb4529b0c6 100644 --- a/src/test/regress/expected/btree_index.out +++ b/src/test/regress/expected/btree_index.out @@ -608,8 +608,6 @@ select * from btree_bpchar where f1::bpchar like 'foo%'; fool (2 rows) --- get test coverage for "single value" deduplication strategy: -insert into btree_bpchar select 'foo' from generate_series(1,1500); -- -- Perform unique checking, with and without the use of deduplication -- diff --git a/src/test/regress/sql/btree_index.sql b/src/test/regress/sql/btree_index.sql index 6aaaa386abc..4d862130ffb 100644 --- a/src/test/regress/sql/btree_index.sql +++ b/src/test/regress/sql/btree_index.sql @@ -326,9 +326,6 @@ explain (costs off) select * from btree_bpchar where f1::bpchar like 'foo%'; select * from btree_bpchar where f1::bpchar like 'foo%'; --- get test coverage for "single value" deduplication strategy: -insert into btree_bpchar select 'foo' from generate_series(1,1500); - -- -- Perform unique checking, with and without the use of deduplication --