pg_get_object_address() doesn't support composites

Started by Jim Nasbyalmost 9 years ago5 messages
#1Jim Nasby
Jim.Nasby@BlueTreble.com

See below. ISTM that pg_get_object_address should support everything
pg_identify_object_as_address can output, no?

I'm guessing the answer here is to have pg_identify_object_as_address
complain if you ask it for something that's not mapable.

~@decina.local/5621# CREATE TYPE comp AS (a int, b int);
CREATE TYPE
~@decina.local/5621# select * from pg_identify_object_as_address(1259,'comp'::regclass, 0);
type | object_names | object_args
----------------+---------------+-------------
composite type | {public,comp} | {}
(1 row)

~@decina.local/5621# select * from pg_get_object_address('composite type', '{public,comp}', '{}');
ERROR: unsupported object type "composite type"
~@decina.local/5621#

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Jim Nasby (#1)
Re: pg_get_object_address() doesn't support composites

Jim Nasby wrote:

See below. ISTM that pg_get_object_address should support everything
pg_identify_object_as_address can output, no?

I'm guessing the answer here is to have pg_identify_object_as_address
complain if you ask it for something that's not mapable.

Yes, I think we should just reject the case in
pg_identify_object_as_address. Note that you can refer to the type
using it pg_type entry:

alvherre=# select * from pg_identify_object_as_address(1247,'comp'::regtype, 0);
type │ object_names │ object_args
──────┼───────────────┼─────────────
type │ {public.comp} │ {}
(1 fila)

alvherre=# select * from pg_get_object_address('type', '{public.comp}', '{}');
classid │ objid │ subobjid
─────────┼───────┼──────────
1247 │ 16400 │ 0
(1 fila)

Trying to accept it using its pg_class entry would require adding an
OBJECT_COMPOSITE_TYPE member to the ObjectType enum. I considered it
back then, and eventually decided that it was not worth the trouble.

Another way to think about this problem is an approach Peter E suggested
not long ago, which was to change the objname/objargs representation
more completely.

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Jim Nasby
Jim.Nasby@BlueTreble.com
In reply to: Alvaro Herrera (#2)
Re: pg_get_object_address() doesn't support composites

On 2/17/17 9:53 PM, Alvaro Herrera wrote:

Another way to think about this problem is an approach Peter E suggested
not long ago, which was to change the objname/objargs representation
more completely.

Hrm, I didn't see that. What was the idea?

BTW, I do find it odd (and might eventually find it irritating) that
some objname's squash schema and name into a single element. Not sure
that's worth fixing at this point, though.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Jim Nasby
Jim.Nasby@BlueTreble.com
In reply to: Alvaro Herrera (#2)
1 attachment(s)
Re: pg_get_object_address() doesn't support composites

On 2/17/17 9:53 PM, Alvaro Herrera wrote:

Jim Nasby wrote:

See below. ISTM that pg_get_object_address should support everything
pg_identify_object_as_address can output, no?

I'm guessing the answer here is to have pg_identify_object_as_address
complain if you ask it for something that's not mapable.

Yes, I think we should just reject the case in
pg_identify_object_as_address.

Attached patch does that, and tests for it. Note that there were some
unsupported types that were not being tested before. I've added a
comment requesting people update the test if they add more types.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)

Attachments:

object_address_unsuported.patchtext/plain; charset=UTF-8; name=object_address_unsuported.patch; x-mac-creator=0; x-mac-type=0Download
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index 2a38792ed6..27ac6ca79a 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -488,7 +488,8 @@ static const ObjectPropertyType ObjectProperty[] =
  * do not have corresponding values in the output enum.  The user of this map
  * must be careful to test for invalid values being returned.
  *
- * To ease maintenance, this follows the order of getObjectTypeDescription.
+ * To ease maintenance, this follows the order of getObjectTypeDescription. If
+ * you add anything here please update test/regress/sql/object_address.sql.
  */
 static const struct object_type_map
 {
@@ -3634,6 +3635,7 @@ pg_identify_object_as_address(PG_FUNCTION_ARGS)
 	Oid			objid = PG_GETARG_OID(1);
 	int32		subobjid = PG_GETARG_INT32(2);
 	ObjectAddress address;
+	char	   *type;
 	char	   *identity;
 	List	   *names;
 	List	   *args;
@@ -3646,6 +3648,13 @@ pg_identify_object_as_address(PG_FUNCTION_ARGS)
 	address.objectId = objid;
 	address.objectSubId = subobjid;
 
+	/* Verify pg_get_object_address() would be able to do something with this type */
+	type = getObjectTypeDescription(&address);
+	if (read_objtype_from_string(type) < 0)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("unsupported object type \"%s\"", type)));
+
 	/*
 	 * Construct a tuple descriptor for the result row.  This must match this
 	 * function's pg_proc entry!
@@ -3661,7 +3670,7 @@ pg_identify_object_as_address(PG_FUNCTION_ARGS)
 	tupdesc = BlessTupleDesc(tupdesc);
 
 	/* object type */
-	values[0] = CStringGetTextDatum(getObjectTypeDescription(&address));
+	values[0] = CStringGetTextDatum(type);
 	nulls[0] = false;
 
 	/* object identity */
diff --git a/src/test/regress/expected/object_address.out b/src/test/regress/expected/object_address.out
index ec5ada97ad..4e99068425 100644
--- a/src/test/regress/expected/object_address.out
+++ b/src/test/regress/expected/object_address.out
@@ -50,8 +50,9 @@ DO $$
 DECLARE
 	objtype text;
 BEGIN
-	FOR objtype IN VALUES ('toast table'), ('index column'), ('sequence column'),
-		('toast table column'), ('view column'), ('materialized view column')
+	FOR objtype IN VALUES ('toast table'), ('composite type'), ('index column'),
+		('sequence column'), ('toast table column'), ('view column'),
+		('materialized view column'), ('composite type column')
 	LOOP
 		BEGIN
 			PERFORM pg_get_object_address(objtype, '{one}', '{}');
@@ -62,11 +63,52 @@ BEGIN
 END;
 $$;
 WARNING:  error for toast table: unsupported object type "toast table"
+WARNING:  error for composite type: unsupported object type "composite type"
 WARNING:  error for index column: unsupported object type "index column"
 WARNING:  error for sequence column: unsupported object type "sequence column"
 WARNING:  error for toast table column: unsupported object type "toast table column"
 WARNING:  error for view column: unsupported object type "view column"
 WARNING:  error for materialized view column: unsupported object type "materialized view column"
+WARNING:  error for composite type column: unsupported object type "composite type column"
+DO $$
+DECLARE
+	toastid oid;
+	classid oid;
+	objid oid;
+	objsubid int;
+	objtype text;
+BEGIN
+	SELECT INTO STRICT toastid
+			reltoastrelid
+		FROM pg_class
+		WHERE oid = 'addr_nsp.gentable'::regclass
+	;
+	FOR classid, objid, objsubid, objtype IN VALUES
+		(1259, toastid, 0, 'toast table'),
+		(1259, 'addr_nsp.gencomptype'::regclass, 0, 'composite type'),
+		(1259, 'addr_nsp.gentable_pkey'::regclass, 1, 'index column'),
+		(1259, 'addr_nsp.gentable_a_seq'::regclass, 1, 'sequence column'),
+		(1259, toastid, 1, 'toast table column'),
+		(1259, 'addr_nsp.genview'::regclass, 1, 'view column'),
+		(1259, 'addr_nsp.genmatview'::regclass, 1, 'materialized view column'),
+		(1259, 'addr_nsp.gencomptype'::regclass, 0, 'composite type column')
+	LOOP
+		BEGIN
+			PERFORM pg_identify_object_as_address(classid, objid, objsubid);
+		EXCEPTION WHEN invalid_parameter_value THEN
+			RAISE WARNING 'error for %: %', objtype, sqlerrm;
+		END;
+	END LOOP;
+END;
+$$;
+WARNING:  error for toast table: unsupported object type "toast table"
+WARNING:  error for composite type: unsupported object type "composite type"
+WARNING:  error for index column: unsupported object type "index column"
+WARNING:  error for sequence column: unsupported object type "sequence column"
+WARNING:  error for toast table column: unsupported object type "toast table column"
+WARNING:  error for view column: unsupported object type "view column"
+WARNING:  error for materialized view column: unsupported object type "materialized view column"
+WARNING:  error for composite type column: unsupported object type "composite type"
 DO $$
 DECLARE
 	objtype text;
diff --git a/src/test/regress/sql/object_address.sql b/src/test/regress/sql/object_address.sql
index e658ea346d..818b92c656 100644
--- a/src/test/regress/sql/object_address.sql
+++ b/src/test/regress/sql/object_address.sql
@@ -52,8 +52,9 @@ DO $$
 DECLARE
 	objtype text;
 BEGIN
-	FOR objtype IN VALUES ('toast table'), ('index column'), ('sequence column'),
-		('toast table column'), ('view column'), ('materialized view column')
+	FOR objtype IN VALUES ('toast table'), ('composite type'), ('index column'),
+		('sequence column'), ('toast table column'), ('view column'),
+		('materialized view column'), ('composite type column')
 	LOOP
 		BEGIN
 			PERFORM pg_get_object_address(objtype, '{one}', '{}');
@@ -66,6 +67,38 @@ $$;
 
 DO $$
 DECLARE
+	toastid oid;
+	classid oid;
+	objid oid;
+	objsubid int;
+	objtype text;
+BEGIN
+	SELECT INTO STRICT toastid
+			reltoastrelid
+		FROM pg_class
+		WHERE oid = 'addr_nsp.gentable'::regclass
+	;
+	FOR classid, objid, objsubid, objtype IN VALUES
+		(1259, toastid, 0, 'toast table'),
+		(1259, 'addr_nsp.gencomptype'::regclass, 0, 'composite type'),
+		(1259, 'addr_nsp.gentable_pkey'::regclass, 1, 'index column'),
+		(1259, 'addr_nsp.gentable_a_seq'::regclass, 1, 'sequence column'),
+		(1259, toastid, 1, 'toast table column'),
+		(1259, 'addr_nsp.genview'::regclass, 1, 'view column'),
+		(1259, 'addr_nsp.genmatview'::regclass, 1, 'materialized view column'),
+		(1259, 'addr_nsp.gencomptype'::regclass, 0, 'composite type column')
+	LOOP
+		BEGIN
+			PERFORM pg_identify_object_as_address(classid, objid, objsubid);
+		EXCEPTION WHEN invalid_parameter_value THEN
+			RAISE WARNING 'error for %: %', objtype, sqlerrm;
+		END;
+	END LOOP;
+END;
+$$;
+
+DO $$
+DECLARE
 	objtype text;
 	names	text[];
 	args	text[];
#5Jim Nasby
Jim.Nasby@BlueTreble.com
In reply to: Jim Nasby (#4)
Re: pg_get_object_address() doesn't support composites

On 2/18/17 4:26 PM, Jim Nasby wrote:

On 2/17/17 9:53 PM, Alvaro Herrera wrote:

Jim Nasby wrote:

See below. ISTM that pg_get_object_address should support everything
pg_identify_object_as_address can output, no?

I'm guessing the answer here is to have pg_identify_object_as_address
complain if you ask it for something that's not mapable.

Yes, I think we should just reject the case in
pg_identify_object_as_address.

Attached patch does that, and tests for it. Note that there were some
unsupported types that were not being tested before. I've added a
comment requesting people update the test if they add more types.

While testing a view on pg_depend, I discovered this has the unfortunate
side-effect of meaning you can no longer use
pg_identify_object_as_address against pg_depend.ref*. Using it against
pg_depend was already problematic though, because it throws an error on
the pinned objects if you try and hand it classid, objid or objsubid. So
maybe it's OK.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers