From 0b28ea79c044a0d3779081dc909a6dc0ce93b991 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Tue, 19 May 2015 11:47:42 -0400
Subject: [PATCH] Avoid collation dependence in indexes of system catalogs.
No index in template0 should have collation-dependent ordering, especially
not indexes on shared catalogs. For most textual columns we avoid this
issue by using type "name" (which sorts per strcmp()). However there are a
few indexed columns that we'd prefer to use "text" for, and for that, the
default opclass text_ops is unsafe. Fortunately, text_pattern_ops is safe
(it sorts per memcmp()), and it has no real functional disadvantage for our
purposes. So change the indexes on pg_seclabel.provider and
pg_shseclabel.provider to use text_pattern_ops.
In passing, also mark pg_replication_origin.roname as using
text_pattern_ops --- for some reason it was labeled varchar_pattern_ops
which is just wrong, even though it accidentally worked.
Add regression test queries to catch future errors of these kinds.
We still can't do anything about the misdeclared pg_seclabel and
pg_shseclabel indexes in back branches :-(
---
src/include/catalog/catversion.h | 2 +-
src/include/catalog/indexing.h | 6 +-
src/test/regress/expected/opr_sanity.out | 83 +++++++++++++++++++++++-
src/test/regress/sql/opr_sanity.sql | 73 ++++++++++++++++++++-
4 files changed, 154 insertions(+), 10 deletions(-)
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index 9a10fadfb58..601258f2c2f 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -53,6 +53,6 @@
*/
/* yyyymmddN */
-#define CATALOG_VERSION_NO 201505153
+#define CATALOG_VERSION_NO 201505191
#endif
diff --git a/src/include/catalog/indexing.h b/src/include/catalog/indexing.h
index f20567ed5f5..1c486c4b9cb 100644
--- a/src/include/catalog/indexing.h
+++ b/src/include/catalog/indexing.h
@@ -290,10 +290,10 @@ DECLARE_UNIQUE_INDEX(pg_default_acl_oid_index, 828, on pg_default_acl using btre
DECLARE_UNIQUE_INDEX(pg_db_role_setting_databaseid_rol_index, 2965, on pg_db_role_setting using btree(setdatabase oid_ops, setrole oid_ops));
#define DbRoleSettingDatidRolidIndexId 2965
-DECLARE_UNIQUE_INDEX(pg_seclabel_object_index, 3597, on pg_seclabel using btree(objoid oid_ops, classoid oid_ops, objsubid int4_ops, provider text_ops));
+DECLARE_UNIQUE_INDEX(pg_seclabel_object_index, 3597, on pg_seclabel using btree(objoid oid_ops, classoid oid_ops, objsubid int4_ops, provider text_pattern_ops));
#define SecLabelObjectIndexId 3597
-DECLARE_UNIQUE_INDEX(pg_shseclabel_object_index, 3593, on pg_shseclabel using btree(objoid oid_ops, classoid oid_ops, provider text_ops));
+DECLARE_UNIQUE_INDEX(pg_shseclabel_object_index, 3593, on pg_shseclabel using btree(objoid oid_ops, classoid oid_ops, provider text_pattern_ops));
#define SharedSecLabelObjectIndexId 3593
DECLARE_UNIQUE_INDEX(pg_extension_oid_index, 3080, on pg_extension using btree(oid oid_ops));
@@ -313,7 +313,7 @@ DECLARE_UNIQUE_INDEX(pg_policy_polrelid_polname_index, 3258, on pg_policy using
DECLARE_UNIQUE_INDEX(pg_replication_origin_roiident_index, 6001, on pg_replication_origin using btree(roident oid_ops));
#define ReplicationOriginIdentIndex 6001
-DECLARE_UNIQUE_INDEX(pg_replication_origin_roname_index, 6002, on pg_replication_origin using btree(roname varchar_pattern_ops));
+DECLARE_UNIQUE_INDEX(pg_replication_origin_roname_index, 6002, on pg_replication_origin using btree(roname text_pattern_ops));
#define ReplicationOriginNameIndex 6002
DECLARE_UNIQUE_INDEX(pg_tablesample_method_name_index, 3331, on pg_tablesample_method using btree(tsmname name_ops));
diff --git a/src/test/regress/expected/opr_sanity.out b/src/test/regress/expected/opr_sanity.out
index 0d2fce9f87b..b88912352eb 100644
--- a/src/test/regress/expected/opr_sanity.out
+++ b/src/test/regress/expected/opr_sanity.out
@@ -2,7 +2,7 @@
-- OPR_SANITY
-- Sanity checks for common errors in making operator/procedure system tables:
-- pg_operator, pg_proc, pg_cast, pg_aggregate, pg_am,
--- pg_amop, pg_amproc, pg_opclass, pg_opfamily.
+-- pg_amop, pg_amproc, pg_opclass, pg_opfamily, pg_index.
--
-- Every test failure in this file should be closely inspected.
-- The description of the failing test should be read carefully before
@@ -27,7 +27,9 @@ SELECT ($1 = $2) OR
($2 = 'pg_catalog.any'::pg_catalog.regtype) OR
($2 = 'pg_catalog.anyarray'::pg_catalog.regtype AND
EXISTS(select 1 from pg_catalog.pg_type where
- oid = $1 and typelem != 0 and typlen = -1))
+ oid = $1 and typelem != 0 and typlen = -1)) OR
+ ($2 = 'pg_catalog.anyrange'::pg_catalog.regtype AND
+ (select typtype from pg_catalog.pg_type where oid = $1) = 'r')
$$ language sql strict stable;
-- This one ignores castcontext, so it considers only physical equivalence
-- and not whether the coercion can be invoked implicitly.
@@ -39,7 +41,9 @@ SELECT ($1 = $2) OR
($2 = 'pg_catalog.any'::pg_catalog.regtype) OR
($2 = 'pg_catalog.anyarray'::pg_catalog.regtype AND
EXISTS(select 1 from pg_catalog.pg_type where
- oid = $1 and typelem != 0 and typlen = -1))
+ oid = $1 and typelem != 0 and typlen = -1)) OR
+ ($2 = 'pg_catalog.anyrange'::pg_catalog.regtype AND
+ (select typtype from pg_catalog.pg_type where oid = $1) = 'r')
$$ language sql strict stable;
-- **************** pg_proc ****************
-- Look for illegal values in pg_proc fields.
@@ -2014,3 +2018,76 @@ WHERE p1.amproc = p2.oid AND
--------------+--------+--------
(0 rows)
+-- **************** pg_index ****************
+-- Look for illegal values in pg_index fields.
+SELECT p1.indexrelid, p1.indrelid
+FROM pg_index as p1
+WHERE p1.indexrelid = 0 OR p1.indrelid = 0 OR
+ p1.indnatts <= 0 OR p1.indnatts > 32;
+ indexrelid | indrelid
+------------+----------
+(0 rows)
+
+-- oidvector and int2vector fields should be of length indnatts.
+SELECT p1.indexrelid, p1.indrelid
+FROM pg_index as p1
+WHERE array_lower(indkey, 1) != 0 OR array_upper(indkey, 1) != indnatts-1 OR
+ array_lower(indclass, 1) != 0 OR array_upper(indclass, 1) != indnatts-1 OR
+ array_lower(indcollation, 1) != 0 OR array_upper(indcollation, 1) != indnatts-1 OR
+ array_lower(indoption, 1) != 0 OR array_upper(indoption, 1) != indnatts-1;
+ indexrelid | indrelid
+------------+----------
+(0 rows)
+
+-- Check that opclasses and collations match the underlying columns.
+-- (As written, this test ignores expression indexes.)
+SELECT indexrelid::regclass, indrelid::regclass, attname, atttypid::regtype, opcname
+FROM (SELECT indexrelid, indrelid, unnest(indkey) as ikey,
+ unnest(indclass) as iclass, unnest(indcollation) as icoll
+ FROM pg_index) ss,
+ pg_attribute a,
+ pg_opclass opc
+WHERE a.attrelid = indrelid AND a.attnum = ikey AND opc.oid = iclass AND
+ (NOT binary_coercible(atttypid, opcintype) OR icoll != attcollation);
+ indexrelid | indrelid | attname | atttypid | opcname
+------------+----------+---------+----------+---------
+(0 rows)
+
+-- For system catalogs, be even tighter: nearly all indexes should be
+-- exact type matches not binary-coercible matches. At this writing
+-- the only exception is an OID index on a regproc column.
+SELECT indexrelid::regclass, indrelid::regclass, attname, atttypid::regtype, opcname
+FROM (SELECT indexrelid, indrelid, unnest(indkey) as ikey,
+ unnest(indclass) as iclass, unnest(indcollation) as icoll
+ FROM pg_index
+ WHERE indrelid < 16384) ss,
+ pg_attribute a,
+ pg_opclass opc
+WHERE a.attrelid = indrelid AND a.attnum = ikey AND opc.oid = iclass AND
+ (opcintype != atttypid OR icoll != attcollation)
+ORDER BY 1;
+ indexrelid | indrelid | attname | atttypid | opcname
+--------------------------+--------------+----------+----------+---------
+ pg_aggregate_fnoid_index | pg_aggregate | aggfnoid | regproc | oid_ops
+(1 row)
+
+-- Check for system catalogs with collation-sensitive ordering. This is not
+-- a representational error in pg_index, but simply wrong catalog design.
+-- It's bad because we expect to be able to clone template0 and assign the
+-- copy a different database collation. It would especially not work for
+-- shared catalogs. Note that although text columns will show a collation
+-- in indcollation, they're still okay to index with text_pattern_ops,
+-- so allow that case.
+SELECT indexrelid::regclass, indrelid::regclass, iclass, icoll
+FROM (SELECT indexrelid, indrelid,
+ unnest(indclass) as iclass, unnest(indcollation) as icoll
+ FROM pg_index
+ WHERE indrelid < 16384) ss
+WHERE icoll != 0 AND iclass !=
+ (SELECT oid FROM pg_opclass
+ WHERE opcname = 'text_pattern_ops' AND opcmethod =
+ (SELECT oid FROM pg_am WHERE amname = 'btree'));
+ indexrelid | indrelid | iclass | icoll
+------------+----------+--------+-------
+(0 rows)
+
diff --git a/src/test/regress/sql/opr_sanity.sql b/src/test/regress/sql/opr_sanity.sql
index 7159a8377ee..6ee01555bd1 100644
--- a/src/test/regress/sql/opr_sanity.sql
+++ b/src/test/regress/sql/opr_sanity.sql
@@ -2,7 +2,7 @@
-- OPR_SANITY
-- Sanity checks for common errors in making operator/procedure system tables:
-- pg_operator, pg_proc, pg_cast, pg_aggregate, pg_am,
--- pg_amop, pg_amproc, pg_opclass, pg_opfamily.
+-- pg_amop, pg_amproc, pg_opclass, pg_opfamily, pg_index.
--
-- Every test failure in this file should be closely inspected.
-- The description of the failing test should be read carefully before
@@ -30,7 +30,9 @@ SELECT ($1 = $2) OR
($2 = 'pg_catalog.any'::pg_catalog.regtype) OR
($2 = 'pg_catalog.anyarray'::pg_catalog.regtype AND
EXISTS(select 1 from pg_catalog.pg_type where
- oid = $1 and typelem != 0 and typlen = -1))
+ oid = $1 and typelem != 0 and typlen = -1)) OR
+ ($2 = 'pg_catalog.anyrange'::pg_catalog.regtype AND
+ (select typtype from pg_catalog.pg_type where oid = $1) = 'r')
$$ language sql strict stable;
-- This one ignores castcontext, so it considers only physical equivalence
@@ -43,7 +45,9 @@ SELECT ($1 = $2) OR
($2 = 'pg_catalog.any'::pg_catalog.regtype) OR
($2 = 'pg_catalog.anyarray'::pg_catalog.regtype AND
EXISTS(select 1 from pg_catalog.pg_type where
- oid = $1 and typelem != 0 and typlen = -1))
+ oid = $1 and typelem != 0 and typlen = -1)) OR
+ ($2 = 'pg_catalog.anyrange'::pg_catalog.regtype AND
+ (select typtype from pg_catalog.pg_type where oid = $1) = 'r')
$$ language sql strict stable;
-- **************** pg_proc ****************
@@ -1316,3 +1320,66 @@ FROM pg_amproc AS p1, pg_proc AS p2
WHERE p1.amproc = p2.oid AND
p1.amproclefttype != p1.amprocrighttype AND
p2.provolatile = 'v';
+
+-- **************** pg_index ****************
+
+-- Look for illegal values in pg_index fields.
+
+SELECT p1.indexrelid, p1.indrelid
+FROM pg_index as p1
+WHERE p1.indexrelid = 0 OR p1.indrelid = 0 OR
+ p1.indnatts <= 0 OR p1.indnatts > 32;
+
+-- oidvector and int2vector fields should be of length indnatts.
+
+SELECT p1.indexrelid, p1.indrelid
+FROM pg_index as p1
+WHERE array_lower(indkey, 1) != 0 OR array_upper(indkey, 1) != indnatts-1 OR
+ array_lower(indclass, 1) != 0 OR array_upper(indclass, 1) != indnatts-1 OR
+ array_lower(indcollation, 1) != 0 OR array_upper(indcollation, 1) != indnatts-1 OR
+ array_lower(indoption, 1) != 0 OR array_upper(indoption, 1) != indnatts-1;
+
+-- Check that opclasses and collations match the underlying columns.
+-- (As written, this test ignores expression indexes.)
+
+SELECT indexrelid::regclass, indrelid::regclass, attname, atttypid::regtype, opcname
+FROM (SELECT indexrelid, indrelid, unnest(indkey) as ikey,
+ unnest(indclass) as iclass, unnest(indcollation) as icoll
+ FROM pg_index) ss,
+ pg_attribute a,
+ pg_opclass opc
+WHERE a.attrelid = indrelid AND a.attnum = ikey AND opc.oid = iclass AND
+ (NOT binary_coercible(atttypid, opcintype) OR icoll != attcollation);
+
+-- For system catalogs, be even tighter: nearly all indexes should be
+-- exact type matches not binary-coercible matches. At this writing
+-- the only exception is an OID index on a regproc column.
+
+SELECT indexrelid::regclass, indrelid::regclass, attname, atttypid::regtype, opcname
+FROM (SELECT indexrelid, indrelid, unnest(indkey) as ikey,
+ unnest(indclass) as iclass, unnest(indcollation) as icoll
+ FROM pg_index
+ WHERE indrelid < 16384) ss,
+ pg_attribute a,
+ pg_opclass opc
+WHERE a.attrelid = indrelid AND a.attnum = ikey AND opc.oid = iclass AND
+ (opcintype != atttypid OR icoll != attcollation)
+ORDER BY 1;
+
+-- Check for system catalogs with collation-sensitive ordering. This is not
+-- a representational error in pg_index, but simply wrong catalog design.
+-- It's bad because we expect to be able to clone template0 and assign the
+-- copy a different database collation. It would especially not work for
+-- shared catalogs. Note that although text columns will show a collation
+-- in indcollation, they're still okay to index with text_pattern_ops,
+-- so allow that case.
+
+SELECT indexrelid::regclass, indrelid::regclass, iclass, icoll
+FROM (SELECT indexrelid, indrelid,
+ unnest(indclass) as iclass, unnest(indcollation) as icoll
+ FROM pg_index
+ WHERE indrelid < 16384) ss
+WHERE icoll != 0 AND iclass !=
+ (SELECT oid FROM pg_opclass
+ WHERE opcname = 'text_pattern_ops' AND opcmethod =
+ (SELECT oid FROM pg_am WHERE amname = 'btree'));
--
GitLab