Re: [PATCH] Alter or rename enum value

Emre Hasegeli <emre@hasegeli.com>

From: Emre Hasegeli <emre@hasegeli.com>
To: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Tom Lane <tgl@sss.pgh.pa.us>, Andrew Dunstan <andrew@dunslane.net>, Matthias Kurz <m.kurz@irregular.at>, Jim Nasby <Jim.Nasby@bluetreble.com>, "David G. Johnston" <david.g.johnston@gmail.com>
Date: 2016-08-24T14:47:19Z
Lists: pgsql-hackers
> 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.


Commits

  1. Relax transactional restrictions on ALTER TYPE ... ADD VALUE (redux).

  2. Partially restore comments discussing enum renumbering hazards.

  3. Allow adding values to an enum type created in the current transaction.