> That would require changing it from an AlterEnumStmt to a RenameStmt > instead. Those look to me like they're for renaming SQL objects, > i.e. things addressed by identifiers, whereas enum labels are just > strings. Looking at the implementation of a few of the functions called > by ExecRenameStmt(), they have very little in common with > RenameEnumLabel() logic-wise.
Yes, you are right that this is not an identifier like others. On the other hand, all RENAME xxx TO yyy statements are RenameStmt at the moment. I think we better leave the decision to the committer. >> What is "nbr"? Why not calling them something like "i" and "val"? > > This is the same naming as used in AddEnumLabel(), which I used as a > guide. I see. Judging from there, it should be shortcut for neighbour. We better use something else. >> Maybe we better release them before reporting error, too. I would >> release the list after the loop, close the heap before ereport(). > > As Tom said, this gets released automatically on error, and is again > similar to how AddEnumLabel() does it. I still don't see any reason not to ReleaseCatCacheList() after the loop instead of writing it 3 times. > Here is an updated patch. I don't know, if it is used by the committer or not, but "existance" is a typo on the commit message. Other than these, it looks good to me. I am marking it as Ready for Committer. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers