From b59fec684164076ce71e25ab0fd987d7ae07b257 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Tue, 16 Jan 2018 15:20:14 -0500 Subject: [PATCH v4 5/5] Get more precise return values from get_object_type() Make get_object_type() distinguish different kinds of relations, to allow callers to give more precise error messages. Add some tests. --- src/backend/catalog/objectaddress.c | 14 +++++++++++--- src/backend/commands/alter.c | 6 +++--- src/include/catalog/objectaddress.h | 2 +- src/test/regress/expected/alter_table.out | 16 ++++++++++++++++ src/test/regress/sql/alter_table.sql | 21 +++++++++++++++++++++ 5 files changed, 52 insertions(+), 7 deletions(-) diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c index d6043f5b66..e5a1fafef1 100644 --- a/src/backend/catalog/objectaddress.c +++ b/src/backend/catalog/objectaddress.c @@ -338,7 +338,7 @@ static const ObjectPropertyType ObjectProperty[] = Anum_pg_class_relnamespace, Anum_pg_class_relowner, Anum_pg_class_relacl, - OBJECT_RELATION, + OBJECT_TABLE, true }, { @@ -2540,11 +2540,19 @@ get_object_attnum_acl(Oid class_id) } ObjectType -get_object_type(Oid class_id) +get_object_type(Oid class_id, Oid object_id) { const ObjectPropertyType *prop = get_object_property_data(class_id); - return prop->objtype; + if (prop->objtype == OBJECT_TABLE) + /* + * If the property data says it's a table, dig a little deeper to get + * the real relation kind, so that callers can produce more precise + * error messages. + */ + return relkind_get_objtype(get_rel_relkind(object_id)); + else + return prop->objtype; } bool diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c index 2d88f4c649..0d63866fb0 100644 --- a/src/backend/commands/alter.c +++ b/src/backend/commands/alter.c @@ -171,7 +171,7 @@ AlterObjectRename_internal(Relation rel, Oid objectId, const char *new_name) AttrNumber Anum_name = get_object_attnum_name(classId); AttrNumber Anum_namespace = get_object_attnum_namespace(classId); AttrNumber Anum_owner = get_object_attnum_owner(classId); - ObjectType objtype = get_object_type(classId); + ObjectType objtype = get_object_type(classId, objectId); HeapTuple oldtup; HeapTuple newtup; Datum datum; @@ -663,7 +663,7 @@ AlterObjectNamespace_internal(Relation rel, Oid objid, Oid nspOid) AttrNumber Anum_name = get_object_attnum_name(classId); AttrNumber Anum_namespace = get_object_attnum_namespace(classId); AttrNumber Anum_owner = get_object_attnum_owner(classId); - ObjectType objtype = get_object_type(classId); + ObjectType objtype = get_object_type(classId, objid); Oid oldNspOid; Datum name, namespace; @@ -942,7 +942,7 @@ AlterObjectOwner_internal(Relation rel, Oid objectId, Oid new_ownerId) /* Superusers can bypass permission checks */ if (!superuser()) { - ObjectType objtype = get_object_type(classId); + ObjectType objtype = get_object_type(classId, objectId); /* must be owner */ if (!has_privs_of_role(GetUserId(), old_ownerId)) diff --git a/src/include/catalog/objectaddress.h b/src/include/catalog/objectaddress.h index 43819304eb..0de73240f9 100644 --- a/src/include/catalog/objectaddress.h +++ b/src/include/catalog/objectaddress.h @@ -62,7 +62,7 @@ extern AttrNumber get_object_attnum_name(Oid class_id); extern AttrNumber get_object_attnum_namespace(Oid class_id); extern AttrNumber get_object_attnum_owner(Oid class_id); extern AttrNumber get_object_attnum_acl(Oid class_id); -extern ObjectType get_object_type(Oid class_id); +extern ObjectType get_object_type(Oid class_id, Oid object_id); extern bool get_object_namensp_unique(Oid class_id); extern HeapTuple get_catalog_object_by_oid(Relation catalog, diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index b998dab6ab..f5b37b8891 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -1,5 +1,12 @@ -- -- ALTER_TABLE +-- +-- Clean up in case a prior regression run failed +SET client_min_messages TO 'warning'; +DROP ROLE IF EXISTS regress_alter_user1; +RESET client_min_messages; +CREATE USER regress_alter_user1; +-- -- add attribute -- CREATE TABLE tmp (initial int4); @@ -209,9 +216,17 @@ ALTER INDEX IF EXISTS __tmp_onek_unique1 RENAME TO onek_unique1; NOTICE: relation "__tmp_onek_unique1" does not exist, skipping ALTER INDEX onek_unique1 RENAME TO tmp_onek_unique1; ALTER INDEX tmp_onek_unique1 RENAME TO onek_unique1; +SET ROLE regress_alter_user1; +ALTER INDEX onek_unique1 RENAME TO fail; -- permission denied +ERROR: must be owner of index onek_unique1 +RESET ROLE; -- renaming views CREATE VIEW tmp_view (unique1) AS SELECT unique1 FROM tenk1; ALTER TABLE tmp_view RENAME TO tmp_view_new; +SET ROLE regress_alter_user1; +ALTER VIEW tmp_view_new RENAME TO fail; -- permission denied +ERROR: must be owner of view tmp_view_new +RESET ROLE; -- hack to ensure we get an indexscan here set enable_seqscan to off; set enable_bitmapscan to off; @@ -3822,3 +3837,4 @@ ALTER TABLE tmp ALTER COLUMN i SET (n_distinct = 1, n_distinct_inherited = 2); ALTER TABLE tmp ALTER COLUMN i RESET (n_distinct_inherited); ANALYZE tmp; DROP TABLE tmp; +DROP USER regress_alter_user1; diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 02a33ca7c4..3bb74151f2 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -1,5 +1,15 @@ -- -- ALTER_TABLE +-- + +-- Clean up in case a prior regression run failed +SET client_min_messages TO 'warning'; +DROP ROLE IF EXISTS regress_alter_user1; +RESET client_min_messages; + +CREATE USER regress_alter_user1; + +-- -- add attribute -- @@ -209,10 +219,19 @@ CREATE TABLE tmp_array (id int); ALTER INDEX onek_unique1 RENAME TO tmp_onek_unique1; ALTER INDEX tmp_onek_unique1 RENAME TO onek_unique1; + +SET ROLE regress_alter_user1; +ALTER INDEX onek_unique1 RENAME TO fail; -- permission denied +RESET ROLE; + -- renaming views CREATE VIEW tmp_view (unique1) AS SELECT unique1 FROM tenk1; ALTER TABLE tmp_view RENAME TO tmp_view_new; +SET ROLE regress_alter_user1; +ALTER VIEW tmp_view_new RENAME TO fail; -- permission denied +RESET ROLE; + -- hack to ensure we get an indexscan here set enable_seqscan to off; set enable_bitmapscan to off; @@ -2530,3 +2549,5 @@ CREATE TABLE tmp(i integer); ALTER TABLE tmp ALTER COLUMN i RESET (n_distinct_inherited); ANALYZE tmp; DROP TABLE tmp; + +DROP USER regress_alter_user1; -- 2.15.1