From de0d4f0b175d0eb8efd05a1560138ac458837404 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Tue, 6 Oct 2026 18:21:29 -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 | 13 +------------ doc/src/sgml/btree.sgml | 16 ++++++++++++---- src/backend/utils/adt/datum.c | 6 +++--- 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, 24 insertions(+), 25 deletions(-) diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c index 38ad19f9d3a..90de2fcef30 100644 --- a/contrib/amcheck/verify_nbtree.c +++ b/contrib/amcheck/verify_nbtree.c @@ -32,7 +32,6 @@ #include "access/xact.h" #include "catalog/index.h" #include "catalog/pg_am.h" -#include "catalog/pg_opfamily_d.h" #include "commands/tablecmds.h" #include "common/pg_prng.h" #include "lib/bloomfilter.h" @@ -344,20 +343,10 @@ bt_index_check_internal(Oid indrelid, bool parentcheck, bool heapallindexed, 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; 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, parentcheck, diff --git a/doc/src/sgml/btree.sgml b/doc/src/sgml/btree.sgml index 6f608a14bf3..45acb9c4104 100644 --- a/doc/src/sgml/btree.sgml +++ b/doc/src/sgml/btree.sgml @@ -846,10 +846,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 7024dc99875..f824a46ef9c 100644 --- a/src/backend/utils/adt/datum.c +++ b/src/backend/utils/adt/datum.c @@ -423,8 +423,8 @@ datum_image_hash(Datum value, bool typByVal, int typLen) * function. * * Earlier minor releases erroneously associated this function with - * interval_ops. Detect that case to rescind deduplication support, without - * requiring initdb. + * interval_ops and bpchar_pattern_ops. Detect those cases to rescind + * deduplication support, without requiring initdb. *------------------------------------------------------------------------- */ Datum @@ -432,7 +432,7 @@ btequalimage(PG_FUNCTION_ARGS) { Oid opcintype = PG_GETARG_OID(0); - PG_RETURN_BOOL(opcintype != INTERVALOID); + PG_RETURN_BOOL(opcintype != INTERVALOID && opcintype != BPCHAROID); } /*------------------------------------------------------------------------- diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c index f4faae03aa8..73b7e8aed68 100644 --- a/src/backend/utils/adt/varlena.c +++ b/src/backend/utils/adt/varlena.c @@ -2849,15 +2849,22 @@ 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(); check_collation_set(collid); + if (opcintype == BPCHAROID) + PG_RETURN_BOOL(false); + if (lc_collate_is_c(collid) || collid == DEFAULT_COLLATION_OID || get_collation_isdeterministic(collid)) diff --git a/src/test/regress/expected/btree_index.out b/src/test/regress/expected/btree_index.out index 93ed5e8cc00..3e14d06b3c6 100644 --- a/src/test/regress/expected/btree_index.out +++ b/src/test/regress/expected/btree_index.out @@ -310,8 +310,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 239f4a4755f..af7fb42df6d 100644 --- a/src/test/regress/sql/btree_index.sql +++ b/src/test/regress/sql/btree_index.sql @@ -162,9 +162,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 --