From b82a7be603f1811a0a707b53c62de6d5d9431740 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 18 May 2015 20:07:44 -0400 Subject: [PATCH] Change pg_seclabel.provider and pg_shseclabel.provider to type "name". These were "text", but that's a bad idea because it has collation-dependent ordering. No index in template0 should have collation-dependent ordering, especially not indexes on shared catalogs. There was general agreement that provider names don't need to be longer than other identifiers, so we can fix this at a small waste of table space by changing from text to name. There's no way to fix the problem in the back branches, but we can hope that security labels don't yet have widespread-enough usage to make it urgent to fix. There needs to be a regression sanity test to prevent us from making this same mistake again; but before putting that in, we'll need to get rid of similar brain fade in the recently-added pg_replication_origin catalog. Note: for lack of a suitable testing environment, I've not really exercised this change. I trust the buildfarm will show up any mistakes. --- doc/src/sgml/catalogs.sgml | 6 +++--- src/backend/commands/seclabel.c | 24 ++++++++++++++---------- src/include/catalog/catversion.h | 2 +- src/include/catalog/indexing.h | 4 ++-- src/include/catalog/pg_seclabel.h | 2 +- src/include/catalog/pg_shseclabel.h | 2 +- 6 files changed, 22 insertions(+), 18 deletions(-) diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 5b36487609..ad36585400 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -5648,7 +5648,7 @@ provider - text + name The label provider associated with this label. @@ -5937,7 +5937,7 @@ provider - text + name The label provider associated with this label. @@ -9025,7 +9025,7 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx provider - text + name pg_seclabel.provider The label provider associated with this label. diff --git a/src/backend/commands/seclabel.c b/src/backend/commands/seclabel.c index 1ef98ce353..8412dcc587 100644 --- a/src/backend/commands/seclabel.c +++ b/src/backend/commands/seclabel.c @@ -163,8 +163,8 @@ GetSharedSecurityLabel(const ObjectAddress *object, const char *provider) ObjectIdGetDatum(object->classId)); ScanKeyInit(&keys[2], Anum_pg_shseclabel_provider, - BTEqualStrategyNumber, F_TEXTEQ, - CStringGetTextDatum(provider)); + BTEqualStrategyNumber, F_NAMEEQ, + CStringGetDatum(provider)); pg_shseclabel = heap_open(SharedSecLabelRelationId, AccessShareLock); @@ -220,8 +220,8 @@ GetSecurityLabel(const ObjectAddress *object, const char *provider) Int32GetDatum(object->objectSubId)); ScanKeyInit(&keys[3], Anum_pg_seclabel_provider, - BTEqualStrategyNumber, F_TEXTEQ, - CStringGetTextDatum(provider)); + BTEqualStrategyNumber, F_NAMEEQ, + CStringGetDatum(provider)); pg_seclabel = heap_open(SecLabelRelationId, AccessShareLock); @@ -256,6 +256,7 @@ SetSharedSecurityLabel(const ObjectAddress *object, SysScanDesc scan; HeapTuple oldtup; HeapTuple newtup = NULL; + NameData providername; Datum values[Natts_pg_shseclabel]; bool nulls[Natts_pg_shseclabel]; bool replaces[Natts_pg_shseclabel]; @@ -265,7 +266,8 @@ SetSharedSecurityLabel(const ObjectAddress *object, memset(replaces, false, sizeof(replaces)); values[Anum_pg_shseclabel_objoid - 1] = ObjectIdGetDatum(object->objectId); values[Anum_pg_shseclabel_classoid - 1] = ObjectIdGetDatum(object->classId); - values[Anum_pg_shseclabel_provider - 1] = CStringGetTextDatum(provider); + namestrcpy(&providername, provider); + values[Anum_pg_shseclabel_provider - 1] = NameGetDatum(&providername); if (label != NULL) values[Anum_pg_shseclabel_label - 1] = CStringGetTextDatum(label); @@ -280,8 +282,8 @@ SetSharedSecurityLabel(const ObjectAddress *object, ObjectIdGetDatum(object->classId)); ScanKeyInit(&keys[2], Anum_pg_shseclabel_provider, - BTEqualStrategyNumber, F_TEXTEQ, - CStringGetTextDatum(provider)); + BTEqualStrategyNumber, F_NAMEEQ, + CStringGetDatum(provider)); pg_shseclabel = heap_open(SharedSecLabelRelationId, RowExclusiveLock); @@ -335,6 +337,7 @@ SetSecurityLabel(const ObjectAddress *object, SysScanDesc scan; HeapTuple oldtup; HeapTuple newtup = NULL; + NameData providername; Datum values[Natts_pg_seclabel]; bool nulls[Natts_pg_seclabel]; bool replaces[Natts_pg_seclabel]; @@ -352,7 +355,8 @@ SetSecurityLabel(const ObjectAddress *object, values[Anum_pg_seclabel_objoid - 1] = ObjectIdGetDatum(object->objectId); values[Anum_pg_seclabel_classoid - 1] = ObjectIdGetDatum(object->classId); values[Anum_pg_seclabel_objsubid - 1] = Int32GetDatum(object->objectSubId); - values[Anum_pg_seclabel_provider - 1] = CStringGetTextDatum(provider); + namestrcpy(&providername, provider); + values[Anum_pg_seclabel_provider - 1] = NameGetDatum(&providername); if (label != NULL) values[Anum_pg_seclabel_label - 1] = CStringGetTextDatum(label); @@ -371,8 +375,8 @@ SetSecurityLabel(const ObjectAddress *object, Int32GetDatum(object->objectSubId)); ScanKeyInit(&keys[3], Anum_pg_seclabel_provider, - BTEqualStrategyNumber, F_TEXTEQ, - CStringGetTextDatum(provider)); + BTEqualStrategyNumber, F_NAMEEQ, + CStringGetDatum(provider)); pg_seclabel = heap_open(SecLabelRelationId, RowExclusiveLock); diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index 9a10fadfb5..8af748b54f 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 201505181 #endif diff --git a/src/include/catalog/indexing.h b/src/include/catalog/indexing.h index f20567ed5f..0f61566237 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 name_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 name_ops)); #define SharedSecLabelObjectIndexId 3593 DECLARE_UNIQUE_INDEX(pg_extension_oid_index, 3080, on pg_extension using btree(oid oid_ops)); diff --git a/src/include/catalog/pg_seclabel.h b/src/include/catalog/pg_seclabel.h index c9f5b0cfdf..f6498142f9 100644 --- a/src/include/catalog/pg_seclabel.h +++ b/src/include/catalog/pg_seclabel.h @@ -25,9 +25,9 @@ CATALOG(pg_seclabel,3596) BKI_WITHOUT_OIDS Oid objoid; /* OID of the object itself */ Oid classoid; /* OID of table containing the object */ int32 objsubid; /* column number, or 0 if not used */ + NameData provider; /* name of label provider */ #ifdef CATALOG_VARLEN /* variable-length fields start here */ - text provider BKI_FORCE_NOT_NULL; /* name of label provider */ text label BKI_FORCE_NOT_NULL; /* security label of the object */ #endif } FormData_pg_seclabel; diff --git a/src/include/catalog/pg_shseclabel.h b/src/include/catalog/pg_shseclabel.h index 3977b42f87..77d438926a 100644 --- a/src/include/catalog/pg_shseclabel.h +++ b/src/include/catalog/pg_shseclabel.h @@ -24,9 +24,9 @@ CATALOG(pg_shseclabel,3592) BKI_SHARED_RELATION BKI_WITHOUT_OIDS { Oid objoid; /* OID of the shared object itself */ Oid classoid; /* OID of table containing the shared object */ + NameData provider; /* name of label provider */ #ifdef CATALOG_VARLEN /* variable-length fields start here */ - text provider BKI_FORCE_NOT_NULL; /* name of label provider */ text label BKI_FORCE_NOT_NULL; /* security label of the object */ #endif } FormData_pg_shseclabel; -- 2.40.0