From 0ab9c56d0fe3acc9d4717a9cbac6ef3369275b90 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 7 Sep 2016 16:11:56 -0400 Subject: [PATCH] Support renaming an existing value of an enum type. MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Not much to be said about this patch: it does what it says on the tin. In passing, rename AlterEnumStmt.skipIfExists to skipIfNewValExists to clarify what it actually does. In the discussion of this patch we considered supporting other similar options, such as IF EXISTS on the type as a whole or IF NOT EXISTS on the target name. This patch doesn't actually add any such feature, but it might happen later. Dagfinn Ilmari Mannsåker, reviewed by Emre Hasegeli Discussion: --- doc/src/sgml/ref/alter_type.sgml | 68 ++++++++++++++++++------ src/backend/catalog/pg_enum.c | 85 ++++++++++++++++++++++++++++++ src/backend/commands/typecmds.c | 20 ++++--- src/backend/nodes/copyfuncs.c | 3 +- src/backend/nodes/equalfuncs.c | 3 +- src/backend/parser/gram.y | 20 +++++-- src/include/catalog/pg_enum.h | 2 + src/include/nodes/parsenodes.h | 3 +- src/test/regress/expected/enum.out | 22 ++++++++ src/test/regress/sql/enum.sql | 11 ++++ 10 files changed, 208 insertions(+), 29 deletions(-) diff --git a/doc/src/sgml/ref/alter_type.sgml b/doc/src/sgml/ref/alter_type.sgml index aec73f6285..fdb4f3367d 100644 --- a/doc/src/sgml/ref/alter_type.sgml +++ b/doc/src/sgml/ref/alter_type.sgml @@ -28,7 +28,8 @@ ALTER TYPE name OWNER TO { name RENAME ATTRIBUTE attribute_name TO new_attribute_name [ CASCADE | RESTRICT ] ALTER TYPE name RENAME TO new_name ALTER TYPE name SET SCHEMA new_schema -ALTER TYPE name ADD VALUE [ IF NOT EXISTS ] new_enum_value [ { BEFORE | AFTER } existing_enum_value ] +ALTER TYPE name ADD VALUE [ IF NOT EXISTS ] new_enum_value [ { BEFORE | AFTER } neighbor_enum_value ] +ALTER TYPE name RENAME VALUE existing_enum_value TO new_enum_value where action is one of: @@ -124,21 +125,13 @@ ALTER TYPE name ADD VALUE [ IF NOT - CASCADE + RENAME VALUE - Automatically propagate the operation to typed tables of the - type being altered, and their descendants. - - - - - - RESTRICT - - - Refuse the operation if the type being altered is the type of a - typed table. This is the default. + This form renames a value of an enum type. + The value's place in the enum's ordering is not affected. + An error will occur if the specified value is not present or the new + name is already present. @@ -241,14 +234,15 @@ ALTER TYPE name ADD VALUE [ IF NOT new_enum_value - The new value to be added to an enum type's list of values. + The new value to be added to an enum type's list of values, + or the new name to be given to an existing value. Like all enum literals, it needs to be quoted. - existing_enum_value + neighbor_enum_value The existing enum value that the new value should be added immediately @@ -258,6 +252,36 @@ ALTER TYPE name ADD VALUE [ IF NOT + + existing_enum_value + + + The existing enum value that should be renamed. + Like all enum literals, it needs to be quoted. + + + + + + CASCADE + + + Automatically propagate the operation to typed tables of the + type being altered, and their descendants. + + + + + + RESTRICT + + + Refuse the operation if the type being altered is the type of a + typed table. This is the default. + + + + @@ -270,6 +294,8 @@ ALTER TYPE name ADD VALUE [ IF NOT an enum type) is executed inside a transaction block, the new value cannot be used until after the transaction has been committed, except in the case that the enum type itself was created earlier in the same transaction. + Likewise, when a pre-existing enum value is renamed, the transaction must + be committed before the renamed value can be used. @@ -323,7 +349,15 @@ ALTER TYPE compfoo ADD ATTRIBUTE f3 int; To add a new value to an enum type in a particular sort position: ALTER TYPE colors ADD VALUE 'orange' AFTER 'red'; - + + + + + To rename an enum value: + +ALTER TYPE colors RENAME VALUE 'purple' TO 'mauve'; + + diff --git a/src/backend/catalog/pg_enum.c b/src/backend/catalog/pg_enum.c index c66f9632c2..1f0ffcfa15 100644 --- a/src/backend/catalog/pg_enum.c +++ b/src/backend/catalog/pg_enum.c @@ -465,6 +465,91 @@ restart: } +/* + * RenameEnumLabel + * Rename a label in an enum set. + */ +void +RenameEnumLabel(Oid enumTypeOid, + const char *oldVal, + const char *newVal) +{ + Relation pg_enum; + HeapTuple enum_tup; + Form_pg_enum en; + CatCList *list; + int nelems; + HeapTuple old_tup; + bool found_new; + int i; + + /* check length of new label is ok */ + if (strlen(newVal) > (NAMEDATALEN - 1)) + ereport(ERROR, + (errcode(ERRCODE_INVALID_NAME), + errmsg("invalid enum label \"%s\"", newVal), + errdetail("Labels must be %d characters or less.", + NAMEDATALEN - 1))); + + /* + * Acquire a lock on the enum type, which we won't release until commit. + * This ensures that two backends aren't concurrently modifying the same + * enum type. Since we are not changing the type's sort order, this is + * probably not really necessary, but there seems no reason not to take + * the lock to be sure. + */ + LockDatabaseObject(TypeRelationId, enumTypeOid, 0, ExclusiveLock); + + pg_enum = heap_open(EnumRelationId, RowExclusiveLock); + + /* Get the list of existing members of the enum */ + list = SearchSysCacheList1(ENUMTYPOIDNAME, + ObjectIdGetDatum(enumTypeOid)); + nelems = list->n_members; + + /* + * Locate the element to rename and check if the new label is already in + * use. (The unique index on pg_enum would catch that anyway, but we + * prefer a friendlier error message.) + */ + old_tup = NULL; + found_new = false; + for (i = 0; i < nelems; i++) + { + enum_tup = &(list->members[i]->tuple); + en = (Form_pg_enum) GETSTRUCT(enum_tup); + if (strcmp(NameStr(en->enumlabel), oldVal) == 0) + old_tup = enum_tup; + if (strcmp(NameStr(en->enumlabel), newVal) == 0) + found_new = true; + } + if (!old_tup) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("\"%s\" is not an existing enum label", + oldVal))); + if (found_new) + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("enum label \"%s\" already exists", + newVal))); + + /* OK, make a writable copy of old tuple */ + enum_tup = heap_copytuple(old_tup); + en = (Form_pg_enum) GETSTRUCT(enum_tup); + + ReleaseCatCacheList(list); + + /* Update the pg_enum entry */ + namestrcpy(&en->enumlabel, newVal); + simple_heap_update(pg_enum, &enum_tup->t_self, enum_tup); + CatalogUpdateIndexes(pg_enum, enum_tup); + heap_freetuple(enum_tup); + + heap_close(pg_enum, RowExclusiveLock); +} + + /* * RenumberEnumType * Renumber existing enum elements to have sort positions 1..n. diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index 6cc7106467..41fd2dae7f 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -1241,17 +1241,25 @@ AlterEnum(AlterEnumStmt *stmt) /* Check it's an enum and check user has permission to ALTER the enum */ checkEnumOwner(tup); - /* Add the new label */ - AddEnumLabel(enum_type_oid, stmt->newVal, - stmt->newValNeighbor, stmt->newValIsAfter, - stmt->skipIfExists); + ReleaseSysCache(tup); + + if (stmt->oldVal) + { + /* Rename an existing label */ + RenameEnumLabel(enum_type_oid, stmt->oldVal, stmt->newVal); + } + else + { + /* Add a new label */ + AddEnumLabel(enum_type_oid, stmt->newVal, + stmt->newValNeighbor, stmt->newValIsAfter, + stmt->skipIfNewValExists); + } InvokeObjectPostAlterHook(TypeRelationId, enum_type_oid, 0); ObjectAddressSet(address, TypeRelationId, enum_type_oid); - ReleaseSysCache(tup); - return address; } diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index be2207e318..4f39dad66b 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3375,10 +3375,11 @@ _copyAlterEnumStmt(const AlterEnumStmt *from) AlterEnumStmt *newnode = makeNode(AlterEnumStmt); COPY_NODE_FIELD(typeName); + COPY_STRING_FIELD(oldVal); COPY_STRING_FIELD(newVal); COPY_STRING_FIELD(newValNeighbor); COPY_SCALAR_FIELD(newValIsAfter); - COPY_SCALAR_FIELD(skipIfExists); + COPY_SCALAR_FIELD(skipIfNewValExists); return newnode; } diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index c4ec4077a6..4800165a91 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -1465,10 +1465,11 @@ static bool _equalAlterEnumStmt(const AlterEnumStmt *a, const AlterEnumStmt *b) { COMPARE_NODE_FIELD(typeName); + COMPARE_STRING_FIELD(oldVal); COMPARE_STRING_FIELD(newVal); COMPARE_STRING_FIELD(newValNeighbor); COMPARE_SCALAR_FIELD(newValIsAfter); - COMPARE_SCALAR_FIELD(skipIfExists); + COMPARE_SCALAR_FIELD(skipIfNewValExists); return true; } diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index b69a77a588..1526c73a1c 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -5257,30 +5257,44 @@ AlterEnumStmt: { AlterEnumStmt *n = makeNode(AlterEnumStmt); n->typeName = $3; + n->oldVal = NULL; n->newVal = $7; n->newValNeighbor = NULL; n->newValIsAfter = true; - n->skipIfExists = $6; + n->skipIfNewValExists = $6; $$ = (Node *) n; } | ALTER TYPE_P any_name ADD_P VALUE_P opt_if_not_exists Sconst BEFORE Sconst { AlterEnumStmt *n = makeNode(AlterEnumStmt); n->typeName = $3; + n->oldVal = NULL; n->newVal = $7; n->newValNeighbor = $9; n->newValIsAfter = false; - n->skipIfExists = $6; + n->skipIfNewValExists = $6; $$ = (Node *) n; } | ALTER TYPE_P any_name ADD_P VALUE_P opt_if_not_exists Sconst AFTER Sconst { AlterEnumStmt *n = makeNode(AlterEnumStmt); n->typeName = $3; + n->oldVal = NULL; n->newVal = $7; n->newValNeighbor = $9; n->newValIsAfter = true; - n->skipIfExists = $6; + n->skipIfNewValExists = $6; + $$ = (Node *) n; + } + | ALTER TYPE_P any_name RENAME VALUE_P Sconst TO Sconst + { + AlterEnumStmt *n = makeNode(AlterEnumStmt); + n->typeName = $3; + n->oldVal = $6; + n->newVal = $8; + n->newValNeighbor = NULL; + n->newValIsAfter = false; + n->skipIfNewValExists = false; $$ = (Node *) n; } ; diff --git a/src/include/catalog/pg_enum.h b/src/include/catalog/pg_enum.h index dd32443b91..901d3adbb9 100644 --- a/src/include/catalog/pg_enum.h +++ b/src/include/catalog/pg_enum.h @@ -67,5 +67,7 @@ extern void EnumValuesDelete(Oid enumTypeOid); extern void AddEnumLabel(Oid enumTypeOid, const char *newVal, const char *neighbor, bool newValIsAfter, bool skipIfExists); +extern void RenameEnumLabel(Oid enumTypeOid, + const char *oldVal, const char *newVal); #endif /* PG_ENUM_H */ diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 3716c2eef9..8d3dcf4d4c 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2708,10 +2708,11 @@ typedef struct AlterEnumStmt { NodeTag type; List *typeName; /* qualified name (list of Value strings) */ + char *oldVal; /* old enum value's name, if renaming */ char *newVal; /* new enum value's name */ char *newValNeighbor; /* neighboring enum value, if specified */ bool newValIsAfter; /* place new enum value after neighbor? */ - bool skipIfExists; /* no error if label already exists */ + bool skipIfNewValExists; /* no error if new already exists? */ } AlterEnumStmt; /* ---------------------- diff --git a/src/test/regress/expected/enum.out b/src/test/regress/expected/enum.out index d4a45a306b..514d1d01a1 100644 --- a/src/test/regress/expected/enum.out +++ b/src/test/regress/expected/enum.out @@ -556,6 +556,28 @@ CREATE TABLE enumtest_bogus_child(parent bogus REFERENCES enumtest_parent); ERROR: foreign key constraint "enumtest_bogus_child_parent_fkey" cannot be implemented DETAIL: Key columns "parent" and "id" are of incompatible types: bogus and rainbow. DROP TYPE bogus; +-- check renaming a value +ALTER TYPE rainbow RENAME VALUE 'red' TO 'crimson'; +SELECT enumlabel, enumsortorder +FROM pg_enum +WHERE enumtypid = 'rainbow'::regtype +ORDER BY 2; + enumlabel | enumsortorder +-----------+--------------- + crimson | 1 + orange | 2 + yellow | 3 + green | 4 + blue | 5 + purple | 6 +(6 rows) + +-- check that renaming a non-existent value fails +ALTER TYPE rainbow RENAME VALUE 'red' TO 'crimson'; +ERROR: "red" is not an existing enum label +-- check that renaming to an existent value fails +ALTER TYPE rainbow RENAME VALUE 'blue' TO 'green'; +ERROR: enum label "green" already exists -- -- check transactional behaviour of ALTER TYPE ... ADD VALUE -- diff --git a/src/test/regress/sql/enum.sql b/src/test/regress/sql/enum.sql index d25e8dedb6..d7e87143a0 100644 --- a/src/test/regress/sql/enum.sql +++ b/src/test/regress/sql/enum.sql @@ -257,6 +257,17 @@ CREATE TYPE bogus AS ENUM('good', 'bad', 'ugly'); CREATE TABLE enumtest_bogus_child(parent bogus REFERENCES enumtest_parent); DROP TYPE bogus; +-- check renaming a value +ALTER TYPE rainbow RENAME VALUE 'red' TO 'crimson'; +SELECT enumlabel, enumsortorder +FROM pg_enum +WHERE enumtypid = 'rainbow'::regtype +ORDER BY 2; +-- check that renaming a non-existent value fails +ALTER TYPE rainbow RENAME VALUE 'red' TO 'crimson'; +-- check that renaming to an existent value fails +ALTER TYPE rainbow RENAME VALUE 'blue' TO 'green'; + -- -- check transactional behaviour of ALTER TYPE ... ADD VALUE -- -- 2.40.0