From: Tom Lane Date: Wed, 7 Sep 2016 20:11:56 +0000 (-0400) Subject: Support renaming an existing value of an enum type. X-Git-Tag: REL_10_BETA1~1759 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=0ab9c56d0fe3acc9d4717a9cbac6ef3369275b90;p=thirdparty%2Fpostgresql.git Support renaming an existing value of an enum type. 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: --- diff --git a/doc/src/sgml/ref/alter_type.sgml b/doc/src/sgml/ref/alter_type.sgml index aec73f62852..fdb4f3367d5 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 c66f9632c29..1f0ffcfa159 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 6cc7106467d..41fd2dae7f8 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 be2207e3188..4f39dad66b4 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 c4ec4077a60..4800165a919 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 b69a77a588f..1526c73a1c5 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 dd32443b91e..901d3adbb9a 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 3716c2eef96..8d3dcf4d4c1 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 d4a45a306bc..514d1d01a10 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 d25e8dedb6c..d7e87143a01 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 --