OBJECT_ATTRIBUTE is useless (or: ALTER TYPE vs ALTER TABLE for composites)

Started by Alvaro Herreraalmost 11 years ago6 messages
#1Alvaro Herrera
alvherre@2ndquadrant.com
1 attachment(s)

I found that the OBJECT_ATTRIBUTE symbol is useless. I can just remove
it and replace it with OBJECT_COLUMN, and everything continues to work;
no test fails that I can find.

I thought we had a prohibition against ALTER TABLE when used on
composites, but it's not as severe as I thought. The following commands
fail in master:

ALTER TABLE comptype RENAME TO comptype2; -- HINT: Use ALTER TYPE
ALTER TABLE comptype SET SCHEMA sch; -- HINT: Use ALTER TYPE

However, the following command works in master:

ALTER TABLE comptype RENAME COLUMN a TO b;
and has the same effect as this:
ALTER TYPE comptype RENAME ATTRIBUTE a TO b;

The RENAME ATTTRIBUTE case in RenameStmt is the only thing currently
using OBJECT_ATTRIBUTE; therefore, since in precisely that case we do
not prohibit using ALTER TABLE, we can just remove OBJECT_ATTRIBUTE
completely. That leads to the attached patch, which changes no test
result at all.

This symbol was added in

commit e440e12c562432a2a695b8054964fb34e3bd823e
Author: Peter Eisentraut <peter_e@gmx.net>
Date: Sun Sep 26 14:41:03 2010 +0300

Add ALTER TYPE ... ADD/DROP/ALTER/RENAME ATTRIBUTE

Like with tables, this also requires allowing the existence of
composite types with zero attributes.

reviewed by KaiGai Kohei

Thoughts?

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

Attachments:

remove-objattr.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index d899dd7..1da38c0 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -1591,7 +1591,6 @@ check_object_ownership(Oid roleid, ObjectType objtype, ObjectAddress address,
 			break;
 		case OBJECT_TYPE:
 		case OBJECT_DOMAIN:
-		case OBJECT_ATTRIBUTE:
 		case OBJECT_DOMCONSTRAINT:
 			if (!pg_type_ownercheck(address.objectId, roleid))
 				aclcheck_error_type(ACLCHECK_NOT_OWNER, address.objectId);
diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c
index 78b54b4..7f85fa4 100644
--- a/src/backend/commands/alter.c
+++ b/src/backend/commands/alter.c
@@ -330,7 +330,6 @@ ExecRenameStmt(RenameStmt *stmt)
 			return RenameRelation(stmt);
 
 		case OBJECT_COLUMN:
-		case OBJECT_ATTRIBUTE:
 			return renameatt(stmt);
 
 		case OBJECT_RULE:
diff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c
index dcf5b98..d14346f 100644
--- a/src/backend/commands/event_trigger.c
+++ b/src/backend/commands/event_trigger.c
@@ -1056,7 +1056,6 @@ EventTriggerSupportsObjectType(ObjectType obtype)
 			/* no support for event triggers on event triggers */
 			return false;
 		case OBJECT_AGGREGATE:
-		case OBJECT_ATTRIBUTE:
 		case OBJECT_CAST:
 		case OBJECT_COLUMN:
 		case OBJECT_COLLATION:
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 6c21002..8ad933d 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -7848,7 +7848,7 @@ RenameStmt: ALTER AGGREGATE func_name aggr_args RENAME TO name
 			| ALTER TYPE_P any_name RENAME ATTRIBUTE name TO name opt_drop_behavior
 				{
 					RenameStmt *n = makeNode(RenameStmt);
-					n->renameType = OBJECT_ATTRIBUTE;
+					n->renameType = OBJECT_COLUMN;
 					n->relationType = OBJECT_TYPE;
 					n->relation = makeRangeVarFromAnyName($3, @3, yyscanner);
 					n->subname = $6;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 6d26986..2b6fc3e 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1615,9 +1615,6 @@ AlterObjectTypeCommandTag(ObjectType objtype)
 		case OBJECT_AGGREGATE:
 			tag = "ALTER AGGREGATE";
 			break;
-		case OBJECT_ATTRIBUTE:
-			tag = "ALTER TYPE";
-			break;
 		case OBJECT_CAST:
 			tag = "ALTER CAST";
 			break;
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index ac13302..09607eb 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1212,7 +1212,6 @@ typedef struct SetOperationStmt
 typedef enum ObjectType
 {
 	OBJECT_AGGREGATE,
-	OBJECT_ATTRIBUTE,			/* type's attribute, when distinct from column */
 	OBJECT_CAST,
 	OBJECT_COLUMN,
 	OBJECT_COLLATION,
#2Kouhei Kaigai
kaigai@ak.jp.nec.com
In reply to: Alvaro Herrera (#1)
Re: OBJECT_ATTRIBUTE is useless (or: ALTER TYPE vs ALTER TABLE for composites)

Please see check_object_ownership(). It checks relation's ownership
if OBJECT_COLUMN, however, type's ownership is the correct check if
OBJECT_ATTRIBUTE.

--
NEC OSS Promotion Center / PG-Strom Project
KaiGai Kohei <kaigai@ak.jp.nec.com>

-----Original Message-----
From: pgsql-hackers-owner@postgresql.org
[mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Alvaro Herrera
Sent: Tuesday, February 24, 2015 4:02 AM
To: Pg Hackers; Peter Eisentraut
Subject: [HACKERS] OBJECT_ATTRIBUTE is useless (or: ALTER TYPE vs ALTER TABLE
for composites)

I found that the OBJECT_ATTRIBUTE symbol is useless. I can just remove
it and replace it with OBJECT_COLUMN, and everything continues to work;
no test fails that I can find.

I thought we had a prohibition against ALTER TABLE when used on
composites, but it's not as severe as I thought. The following commands
fail in master:

ALTER TABLE comptype RENAME TO comptype2; -- HINT: Use ALTER TYPE
ALTER TABLE comptype SET SCHEMA sch; -- HINT: Use ALTER TYPE

However, the following command works in master:

ALTER TABLE comptype RENAME COLUMN a TO b;
and has the same effect as this:
ALTER TYPE comptype RENAME ATTRIBUTE a TO b;

The RENAME ATTTRIBUTE case in RenameStmt is the only thing currently
using OBJECT_ATTRIBUTE; therefore, since in precisely that case we do
not prohibit using ALTER TABLE, we can just remove OBJECT_ATTRIBUTE
completely. That leads to the attached patch, which changes no test
result at all.

This symbol was added in

commit e440e12c562432a2a695b8054964fb34e3bd823e
Author: Peter Eisentraut <peter_e@gmx.net>
Date: Sun Sep 26 14:41:03 2010 +0300

Add ALTER TYPE ... ADD/DROP/ALTER/RENAME ATTRIBUTE

Like with tables, this also requires allowing the existence of
composite types with zero attributes.

reviewed by KaiGai Kohei

Thoughts?

--
Álvaro Herrera http://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

#3Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Kouhei Kaigai (#2)
Re: OBJECT_ATTRIBUTE is useless (or: ALTER TYPE vs ALTER TABLE for composites)

Kouhei Kaigai wrote:

Please see check_object_ownership(). It checks relation's ownership
if OBJECT_COLUMN, however, type's ownership is the correct check if
OBJECT_ATTRIBUTE.

Hmm. Is there any case where the two are different?

--
�lvaro Herrera http://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

#4Kouhei Kaigai
kaigai@ak.jp.nec.com
In reply to: Alvaro Herrera (#3)
Re: OBJECT_ATTRIBUTE is useless (or: ALTER TYPE vs ALTER TABLE for composites)

Kouhei Kaigai wrote:

Please see check_object_ownership(). It checks relation's ownership
if OBJECT_COLUMN, however, type's ownership is the correct check if
OBJECT_ATTRIBUTE.

Hmm. Is there any case where the two are different?

AlterObjectTypeCommandTag()?

OBJECT_ATTRIBUTE makes "ALTER TYPE" tag, but "ALTER COLUMN" tag is
made with OBJECT_COLUMN.

Above two cases are all I could found.

Thanks,
--
NEC OSS Promotion Center / PG-Strom Project
KaiGai Kohei <kaigai@ak.jp.nec.com>

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

#5Peter Eisentraut
peter_e@gmx.net
In reply to: Alvaro Herrera (#1)
Re: OBJECT_ATTRIBUTE is useless (or: ALTER TYPE vs ALTER TABLE for composites)

On 2/23/15 2:01 PM, Alvaro Herrera wrote:

I found that the OBJECT_ATTRIBUTE symbol is useless. I can just remove
it and replace it with OBJECT_COLUMN, and everything continues to work;
no test fails that I can find.

It appears that it would change the command tag from ALTER TYPE to ALTER
TABLE.

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

#6Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Peter Eisentraut (#5)
Re: OBJECT_ATTRIBUTE is useless (or: ALTER TYPE vs ALTER TABLE for composites)

Peter Eisentraut wrote:

On 2/23/15 2:01 PM, Alvaro Herrera wrote:

I found that the OBJECT_ATTRIBUTE symbol is useless. I can just remove
it and replace it with OBJECT_COLUMN, and everything continues to work;
no test fails that I can find.

It appears that it would change the command tag from ALTER TYPE to ALTER
TABLE.

Ah, that it does. I guess I unconsciously believed that command tags
would be part of regression tests expected output, but clearly they are
not.

I don't think this change is all that terrible, but I've seen past
discussions about changing command type and they aren't pretty, so I
will drop this.

--
�lvaro Herrera http://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