From a32b8dc9865aa5189d20eca273700b10dfd92fc7 Mon Sep 17 00:00:00 2001 From: Peter Geoghegan Date: Tue, 6 Oct 2026 18:21:39 -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/include/catalog/catversion.h | 2 +- src/include/catalog/pg_amproc.dat | 7 ---- src/test/regress/expected/btree_index.out | 2 -- src/test/regress/expected/opr_sanity.out | 40 ++++++++++++----------- src/test/regress/sql/btree_index.sql | 3 -- 7 files changed, 35 insertions(+), 52 deletions(-) diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c index 455eb90f38c..97c519b47f5 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/include/catalog/catversion.h b/src/include/catalog/catversion.h index 7f7667f6179..9b6ef64a04e 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -57,6 +57,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 202610021 +#define CATALOG_VERSION_NO 202610061 #endif diff --git a/src/include/catalog/pg_amproc.dat b/src/include/catalog/pg_amproc.dat index 4a1efdbc899..27fb8cd2d35 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', @@ -224,8 +221,6 @@ { amprocfamily => 'btree/oidvector_ops', amproclefttype => 'oidvector', amprocrighttype => 'oidvector', amprocnum => '1', amproc => 'btoidvectorcmp' }, -{ amprocfamily => 'btree/oidvector_ops', amproclefttype => 'oidvector', - amprocrighttype => 'oidvector', amprocnum => '4', amproc => 'btequalimage' }, { amprocfamily => 'btree/text_ops', amproclefttype => 'text', amprocrighttype => 'text', amprocnum => '1', amproc => 'bttextcmp' }, { amprocfamily => 'btree/text_ops', amproclefttype => 'text', @@ -273,8 +268,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/btree_index.out b/src/test/regress/expected/btree_index.out index 3a83e9a0534..7c53d598649 100644 --- a/src/test/regress/expected/btree_index.out +++ b/src/test/regress/expected/btree_index.out @@ -765,8 +765,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/expected/opr_sanity.out b/src/test/regress/expected/opr_sanity.out index 67cae397861..907d63daa3f 100644 --- a/src/test/regress/expected/opr_sanity.out +++ b/src/test/regress/expected/opr_sanity.out @@ -2225,25 +2225,27 @@ 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 + | oidvector_ops | oidvector_ops | oidvector + | 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 +(18 rows) -- **************** pg_index **************** -- Look for illegal values in pg_index fields. diff --git a/src/test/regress/sql/btree_index.sql b/src/test/regress/sql/btree_index.sql index a08bb101c20..6990d7818da 100644 --- a/src/test/regress/sql/btree_index.sql +++ b/src/test/regress/sql/btree_index.sql @@ -411,9 +411,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 --