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 <zhong950419@gmail.com>
Co-authored-by: Peter Geoghegan <pg@bowt.ie>
Reported-by: Ke <kehan5800@gmail.com>
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
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

@@ -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,

doc/src/sgml/btree.sgml

@@ -875,10 +875,18 @@ options(<replaceable>relopts</replaceable> <type>local_relopts *</type>) returns
    <itemizedlist>
     <listitem>
      <para>
-      <type>text</type>, <type>varchar</type>, and <type>char</type>
-      cannot use deduplication when a
-      <emphasis>nondeterministic</emphasis> collation is used.  Case
-      and accent differences must be preserved among equal datums.
+      <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>character</type> (and its aliases <type>char</type> and
+      <type>bpchar</type>) cannot use deduplication.  Trailing spaces are
+      disregarded when comparing two values, but must be preserved among
+      equal datums.
      </para>
     </listitem>
 

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);
 }
 
 /*-------------------------------------------------------------------------

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);

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
 --

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
 --