(2010/03/11 0:54), Robert Haas wrote: > On Wed, Mar 3, 2010 at 7:16 PM, Robert Haas<robertmh...@gmail.com> wrote: >> 2010/3/3 KaiGai Kohei<kai...@ak.jp.nec.com>: >>> (2010/03/03 22:42), Robert Haas wrote: >>>> 2010/3/3 KaiGai Kohei<kai...@ak.jp.nec.com>: >>>>> (2010/03/03 14:26), Robert Haas wrote: >>>>>> 2010/3/2 KaiGai Kohei<kai...@ak.jp.nec.com>: >>>>>>> Is it an expected behavior? >>>>>>> >>>>>>> postgres=> CREATE SEQUENCE s; >>>>>>> CREATE SEQUENCE >>>>>>> postgres=> ALTER TABLE s RENAME sequence_name TO abcd; >>>>>>> ALTER TABLE >>>>>>> >>>>>>> postgres=> CREATE TABLE t (a int primary key, b text); >>>>>>> NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index >>>>>>> "t_pkey" for table "t" >>>>>>> CREATE TABLE >>>>>>> postgres=> ALTER TABLE t_pkey RENAME a TO xyz; >>>>>>> ALTER TABLE >>>>>>> >>>>>>> The documentation says: >>>>>>> http://developer.postgresql.org/pgdocs/postgres/sql-altertable.html >>>>>>> >>>>>>> : >>>>>>> RENAME >>>>>>> The RENAME forms change the name of a table (or an index, >>>>>>> sequence, or view) or >>>>>>> the name of an individual column in a table. There is no effect >>>>>>> on the stored data. >>>>>>> >>>>>>> It seems to me the renameatt() should check relkind of the specified >>>>>>> relation, and >>>>>>> raise an error if relkind != RELKIND_RELATION. >>>>>> >>>>>> Are we talking about renameatt() or RenameRelation()? Letting >>>>>> RenameRelation() rename whatever seems fairly harmless; renameatt(), >>>>>> on the other hand, should probably refuse to allow this: >>>>>> >>>>>> CREATE SEQUENCE foo; >>>>>> ALTER TABLE foo RENAME COLUMN is_cycled TO bob; >>>>>> >>>>>> ...because that's just weird. Tables, indexes, and views make sense, >>>>>> but the attributes of a sequence should be nailed down I think; >>>>>> they're basically system properties. >>>>> >>>>> I'm talking about renameatt(), not RenameRelation(). >>>> >>>> OK. Your original example was misleading because you had renameatt() >>>> in the subject line but the actual SQL commands were renaming a whole >>>> relation (which is a reasonable thing to do). >>>> >>>>> If our perspective is these are a type of system properties, we should >>>>> be able to reference these attributes with same name, so it is not >>>>> harmless >>>>> to allow renaming these attributes. >>>>> >>>>> I also agree that it makes sense to allow renaming attributes of tables >>>>> and views. But I don't know whether it makes sense to allow it on indexs, >>>>> like sequence and toast relations. >>>> >>>> I would think not. >>> >>> OK, the attached patch forbid renameatt() on relations expect for tables >>> and views. >> >> OK, I will review it. > > I don't think we should apply this as-is. After some reflection, I > don't believe we should reject attribute renames on indices or > composite types. The former is maybe useless but seems harmless, and > the latter seems affirmatively useful.
Indeed, it is useful to allow renaming attribute of composite types. However, it is also useless to allow to rename attribute of sequences, but harmless, like renames on indexes. It seems to me it is fair enough to allow renaming attributes of tables, views and composite types... > Also, I think that the comment about "this would normally be done in > utility.c" is no longer true and should be removed while we're > cleaning house. Yes, when we rename attribute of the relations, it originally checks ownership of the relation twice in ExecRenameStmt() and renameatt(). It should be reworked in the next. Thanks, -- KaiGai Kohei <kai...@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
