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-21T13:49:16Z
Lists: pgsql-hackers
> Here is v4, which changes the command from ALTER VALUE to RENAME VALUE,
> for consistency with RENAME ATTRIBUTE.

It looks like we always use "ALTER ... RENAME ... old_name TO
new_name" syntax, so it is better that way.  I have noticed that all
the other RENAMEs go through ExecRenameStmt().  We better do the same.

> +    int         nbr_index;
> +    Form_pg_enum nbr_en;

What is "nbr"?  Why not calling them something like "i" and "val"?

> +   /* Locate the element to rename and check if the new label is already in

The project style for multi-line commands is to leave the first line
of the comment block empty.

> +        if (strcmp(NameStr(nbr_en->enumlabel), oldVal) == 0)

I found namestrcmp() for this.

> +    }
> +    if (!old_tup)

I would leave a space in between.

> +        ReleaseCatCacheList(list);
> +        heap_close(pg_enum, RowExclusiveLock);

Maybe we better release them before reporting error, too.  I would
release the list after the loop, close the heap before ereport().


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.