v1-0001-Prohibit-creating-text_pattern_ops-indexes-with-n.patch

From 3b1f52e5292303cba8804ae1f6676973ef903ec0 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Wed, 18 Sep 2019 14:28:31 +0200
Subject: [PATCH v1] Prohibit creating text_pattern_ops indexes with
 nondeterministic collations

Discussion: https://www.postgresql.org/message-id/22566.1568675619@sss.pgh.pa.us
---
 src/backend/catalog/index.c        | 40 +++++++++++++++++++++++++++
 src/backend/utils/adt/varchar.c    | 32 +++++-----------------
 src/backend/utils/adt/varlena.c    | 43 +++++-------------------------
 src/include/catalog/pg_opclass.dat |  9 ++++---
 4 files changed, 58 insertions(+), 66 deletions(-)

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 1dac2803b0..f2adee3156 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -762,6 +762,46 @@ index_create(Relation heapRelation,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("user-defined indexes on system catalog tables are not supported")));
 
+	/*
+	 * XXX We cannot use a text_pattern_ops index for nondeterministic
+	 * collations, because these operators intentionally ignore the collation.
+	 * However, the planner has no way to know that, so it might choose such
+	 * an index for an "=" clause, which would lead to wrong results.  This
+	 * check here doesn't prevent choosing the index, but it will at least
+	 * error out if the index is chosen.  A text_pattern_ops index on a column
+	 * with nondeterministic collation is pretty useless anyway, since LIKE
+	 * etc. won't work there either.  A future possibility would be to
+	 * annotate the operator class or its members in the catalog to avoid the
+	 * index.  Another alternative is to stay away from the *_pattern_ops
+	 * operator classes and prefer creating LIKE-supporting indexes with
+	 * COLLATE "C".
+	 */
+	for (i = 0; i < indexInfo->ii_NumIndexAttrs; i++)
+	{
+		Oid			collation = collationObjectId[i];
+		Oid			opclass = classObjectId[i];
+
+		if (collation)
+		{
+			if (!get_collation_isdeterministic(collation) &&
+				(opclass == TEXT_BTREE_PATTERN_OPS_OID ||
+				 opclass == VARCHAR_BTREE_PATTERN_OPS_OID ||
+				 opclass == BPCHAR_BTREE_PATTERN_OPS_OID))
+			{
+				HeapTuple	classtup;
+
+				classtup = SearchSysCache1(CLAOID, ObjectIdGetDatum(opclass));
+				if (!HeapTupleIsValid(classtup))
+					elog(ERROR, "cache lookup failed for operator class %u", opclass);
+				ereport(ERROR,
+						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+						 errmsg("nondeterministic collations are not supported for operator class \"%s\"",
+								NameStr(((Form_pg_opclass) GETSTRUCT(classtup))->opcname))));
+				ReleaseSysCache(classtup);
+			}
+		}
+	}
+
 	/*
 	 * Concurrent index build on a system catalog is unsafe because we tend to
 	 * release locks before committing in catalogs.
diff --git a/src/backend/utils/adt/varchar.c b/src/backend/utils/adt/varchar.c
index 332dc860c4..b21137c583 100644
--- a/src/backend/utils/adt/varchar.c
+++ b/src/backend/utils/adt/varchar.c
@@ -1105,23 +1105,12 @@ hashbpcharextended(PG_FUNCTION_ARGS)
  */
 
 static int
-internal_bpchar_pattern_compare(BpChar *arg1, BpChar *arg2, Oid collid)
+internal_bpchar_pattern_compare(BpChar *arg1, BpChar *arg2)
 {
 	int			result;
 	int			len1,
 				len2;
 
-	check_collation_set(collid);
-
-	/*
-	 * see internal_text_pattern_compare()
-	 */
-	if (!get_collation_isdeterministic(collid))
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("nondeterministic collations are not supported for operator class \"%s\"",
-						"bpchar_pattern_ops")));
-
 	len1 = bcTruelen(arg1);
 	len2 = bcTruelen(arg2);
 
@@ -1144,7 +1133,7 @@ bpchar_pattern_lt(PG_FUNCTION_ARGS)
 	BpChar	   *arg2 = PG_GETARG_BPCHAR_PP(1);
 	int			result;
 
-	result = internal_bpchar_pattern_compare(arg1, arg2, PG_GET_COLLATION());
+	result = internal_bpchar_pattern_compare(arg1, arg2);
 
 	PG_FREE_IF_COPY(arg1, 0);
 	PG_FREE_IF_COPY(arg2, 1);
@@ -1160,7 +1149,7 @@ bpchar_pattern_le(PG_FUNCTION_ARGS)
 	BpChar	   *arg2 = PG_GETARG_BPCHAR_PP(1);
 	int			result;
 
-	result = internal_bpchar_pattern_compare(arg1, arg2, PG_GET_COLLATION());
+	result = internal_bpchar_pattern_compare(arg1, arg2);
 
 	PG_FREE_IF_COPY(arg1, 0);
 	PG_FREE_IF_COPY(arg2, 1);
@@ -1176,7 +1165,7 @@ bpchar_pattern_ge(PG_FUNCTION_ARGS)
 	BpChar	   *arg2 = PG_GETARG_BPCHAR_PP(1);
 	int			result;
 
-	result = internal_bpchar_pattern_compare(arg1, arg2, PG_GET_COLLATION());
+	result = internal_bpchar_pattern_compare(arg1, arg2);
 
 	PG_FREE_IF_COPY(arg1, 0);
 	PG_FREE_IF_COPY(arg2, 1);
@@ -1192,7 +1181,7 @@ bpchar_pattern_gt(PG_FUNCTION_ARGS)
 	BpChar	   *arg2 = PG_GETARG_BPCHAR_PP(1);
 	int			result;
 
-	result = internal_bpchar_pattern_compare(arg1, arg2, PG_GET_COLLATION());
+	result = internal_bpchar_pattern_compare(arg1, arg2);
 
 	PG_FREE_IF_COPY(arg1, 0);
 	PG_FREE_IF_COPY(arg2, 1);
@@ -1208,7 +1197,7 @@ btbpchar_pattern_cmp(PG_FUNCTION_ARGS)
 	BpChar	   *arg2 = PG_GETARG_BPCHAR_PP(1);
 	int			result;
 
-	result = internal_bpchar_pattern_compare(arg1, arg2, PG_GET_COLLATION());
+	result = internal_bpchar_pattern_compare(arg1, arg2);
 
 	PG_FREE_IF_COPY(arg1, 0);
 	PG_FREE_IF_COPY(arg2, 1);
@@ -1221,17 +1210,8 @@ Datum
 btbpchar_pattern_sortsupport(PG_FUNCTION_ARGS)
 {
 	SortSupport ssup = (SortSupport) PG_GETARG_POINTER(0);
-	Oid			collid = ssup->ssup_collation;
 	MemoryContext oldcontext;
 
-	check_collation_set(collid);
-
-	if (!get_collation_isdeterministic(collid))
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("nondeterministic collations are not supported for operator class \"%s\"",
-						"bpchar_pattern_ops")));
-
 	oldcontext = MemoryContextSwitchTo(ssup->ssup_cxt);
 
 	/* Use generic string SortSupport, forcing "C" collation */
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index 0864838867..b44f895d95 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -2996,34 +2996,12 @@ textgename(PG_FUNCTION_ARGS)
  */
 
 static int
-internal_text_pattern_compare(text *arg1, text *arg2, Oid collid)
+internal_text_pattern_compare(text *arg1, text *arg2)
 {
 	int			result;
 	int			len1,
 				len2;
 
-	check_collation_set(collid);
-
-	/*
-	 * XXX We cannot use a text_pattern_ops index for nondeterministic
-	 * collations, because these operators intentionally ignore the collation.
-	 * However, the planner has no way to know that, so it might choose such
-	 * an index for an "=" clause, which would lead to wrong results.  This
-	 * check here doesn't prevent choosing the index, but it will at least
-	 * error out if the index is chosen.  A text_pattern_ops index on a column
-	 * with nondeterministic collation is pretty useless anyway, since LIKE
-	 * etc. won't work there either.  A future possibility would be to
-	 * annotate the operator class or its members in the catalog to avoid the
-	 * index.  Another alternative is to stay away from the *_pattern_ops
-	 * operator classes and prefer creating LIKE-supporting indexes with
-	 * COLLATE "C".
-	 */
-	if (!get_collation_isdeterministic(collid))
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("nondeterministic collations are not supported for operator class \"%s\"",
-						"text_pattern_ops")));
-
 	len1 = VARSIZE_ANY_EXHDR(arg1);
 	len2 = VARSIZE_ANY_EXHDR(arg2);
 
@@ -3046,7 +3024,7 @@ text_pattern_lt(PG_FUNCTION_ARGS)
 	text	   *arg2 = PG_GETARG_TEXT_PP(1);
 	int			result;
 
-	result = internal_text_pattern_compare(arg1, arg2, PG_GET_COLLATION());
+	result = internal_text_pattern_compare(arg1, arg2);
 
 	PG_FREE_IF_COPY(arg1, 0);
 	PG_FREE_IF_COPY(arg2, 1);
@@ -3062,7 +3040,7 @@ text_pattern_le(PG_FUNCTION_ARGS)
 	text	   *arg2 = PG_GETARG_TEXT_PP(1);
 	int			result;
 
-	result = internal_text_pattern_compare(arg1, arg2, PG_GET_COLLATION());
+	result = internal_text_pattern_compare(arg1, arg2);
 
 	PG_FREE_IF_COPY(arg1, 0);
 	PG_FREE_IF_COPY(arg2, 1);
@@ -3078,7 +3056,7 @@ text_pattern_ge(PG_FUNCTION_ARGS)
 	text	   *arg2 = PG_GETARG_TEXT_PP(1);
 	int			result;
 
-	result = internal_text_pattern_compare(arg1, arg2, PG_GET_COLLATION());
+	result = internal_text_pattern_compare(arg1, arg2);
 
 	PG_FREE_IF_COPY(arg1, 0);
 	PG_FREE_IF_COPY(arg2, 1);
@@ -3094,7 +3072,7 @@ text_pattern_gt(PG_FUNCTION_ARGS)
 	text	   *arg2 = PG_GETARG_TEXT_PP(1);
 	int			result;
 
-	result = internal_text_pattern_compare(arg1, arg2, PG_GET_COLLATION());
+	result = internal_text_pattern_compare(arg1, arg2);
 
 	PG_FREE_IF_COPY(arg1, 0);
 	PG_FREE_IF_COPY(arg2, 1);
@@ -3110,7 +3088,7 @@ bttext_pattern_cmp(PG_FUNCTION_ARGS)
 	text	   *arg2 = PG_GETARG_TEXT_PP(1);
 	int			result;
 
-	result = internal_text_pattern_compare(arg1, arg2, PG_GET_COLLATION());
+	result = internal_text_pattern_compare(arg1, arg2);
 
 	PG_FREE_IF_COPY(arg1, 0);
 	PG_FREE_IF_COPY(arg2, 1);
@@ -3123,17 +3101,8 @@ Datum
 bttext_pattern_sortsupport(PG_FUNCTION_ARGS)
 {
 	SortSupport ssup = (SortSupport) PG_GETARG_POINTER(0);
-	Oid			collid = ssup->ssup_collation;
 	MemoryContext oldcontext;
 
-	check_collation_set(collid);
-
-	if (!get_collation_isdeterministic(collid))
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("nondeterministic collations are not supported for operator class \"%s\"",
-						"text_pattern_ops")));
-
 	oldcontext = MemoryContextSwitchTo(ssup->ssup_cxt);
 
 	/* Use generic string SortSupport, forcing "C" collation */
diff --git a/src/include/catalog/pg_opclass.dat b/src/include/catalog/pg_opclass.dat
index fdfea85efe..9bab83e91e 100644
--- a/src/include/catalog/pg_opclass.dat
+++ b/src/include/catalog/pg_opclass.dat
@@ -146,13 +146,16 @@
   opcfamily => 'btree/datetime_ops', opcintype => 'timestamp' },
 { opcmethod => 'hash', opcname => 'timestamp_ops',
   opcfamily => 'hash/timestamp_ops', opcintype => 'timestamp' },
-{ opcmethod => 'btree', opcname => 'text_pattern_ops',
+{ oid => '4187', oid_symbol => 'TEXT_BTREE_PATTERN_OPS_OID',
+  opcmethod => 'btree', opcname => 'text_pattern_ops',
   opcfamily => 'btree/text_pattern_ops', opcintype => 'text',
   opcdefault => 'f' },
-{ opcmethod => 'btree', opcname => 'varchar_pattern_ops',
+{ oid => '4188', oid_symbol => 'VARCHAR_BTREE_PATTERN_OPS_OID',
+  opcmethod => 'btree', opcname => 'varchar_pattern_ops',
   opcfamily => 'btree/text_pattern_ops', opcintype => 'text',
   opcdefault => 'f' },
-{ opcmethod => 'btree', opcname => 'bpchar_pattern_ops',
+{ oid => '4189', oid_symbol => 'BPCHAR_BTREE_PATTERN_OPS_OID',
+  opcmethod => 'btree', opcname => 'bpchar_pattern_ops',
   opcfamily => 'btree/bpchar_pattern_ops', opcintype => 'bpchar',
   opcdefault => 'f' },
 { opcmethod => 'btree', opcname => 'money_ops', opcfamily => 'btree/money_ops',
-- 
2.23.0