Thread

[PATCH v4 5/5] Get more precise return values from get_object_type()

Peter Eisentraut <peter_e@gmx.net> — 2018-01-16T20:20:14Z

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


--------------9542E740EF1A89D80A2224F4--