Re: ALTER TYPE RENAME

Petr Jelinek <pjmodos(at)pjmodos(dot)net> writes:
> I noticed we don't have ALTER TYPE foo RENAME TO bar command which would
> be handy for me especially for enum types.
> So I wrote this little patch (including very brief doc) which adds above
> syntax. It basically just does some checks and calls existing TypeRename
> function which is used for renaming table rowtype now.
> I hope I haven't missed anything, but I am unsure about two things.
> First, this patch allows renaming base types which I don't know if it's
> desired. And second we might want to throw error when renaming rowtype
> (there is check in AlterTypeOwner for this but not in AlterTypeNamespace
> so I don't know).

Hm, it's definitely a bug/oversight that AlterTypeNamespace doesn't make
a similar test for that. I think you should have to use ALTER TABLE to
muck with a table's rowtype. Your patch as proposed would allow people
to get into a state where a table's rowtype is named differently than
the table, which is a horrid idea. (For one thing, a dump/restore would
fail to preserve such a state.)

As for the patch at hand, I think it's very poor style to have functions
named both RenameType and TypeRename, with no obvious indication of
which does what. The newly added function should be RenameType, since
that parallels the other callees of ExecRenameStmt, but I'd be inclined
to rename the existing function to RenameTypeInternal, as contemplated
in its header comment. (BTW, your patch omits to fix its header
comment, which is rendered an outright lie by the patch.)

regards, tom lane