rdelaage
/
postgresql
mirror of https://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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 :-(
This commit is contained in:
Tom Lane 2015-05-19 11:47:42 -04:00
parent afee04352b
commit 0b28ea79c0
4 changed files with 154 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/include/catalog/catversion.h
View File

@ -53,6 +53,6 @@
*/
/* yyyymmddN */
#define CATALOG_VERSION_NO 201505153
#define CATALOG_VERSION_NO 201505191
#endif

6
src/include/catalog/indexing.h
View File

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

83
src/test/regress/expected/opr_sanity.out
View File

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

73
src/test/regress/sql/opr_sanity.sql
View File

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