Emre Hasegeli <e...@hasegeli.com> writes:
>> Bottom line here is that I'd rather commit ALTER TYPE RENAME VALUE with
>> no EXISTS features and then see it accrete those features together with
>> other types of RENAME, when and if there's a will to make that happen.
>
> This sounds like a good conclusion to me.
Works for me. I mainly added the IF [NOT] EXISTS support to be
consistent with ADD VALUE, and because I like idempotency, but I'm not
married to it.
Here is version 6 of the patch, which just adds RENAME VALUE with no IF
[NOT] EXISTS, rebased onto current master (particularly the
transactional ADD VALUE patch).
>From e4ee07d53bbab5ee434a12a2f30a9ac9bb5c89d3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilm...@ilmari.org>
Date: Tue, 6 Sep 2016 14:38:06 +0100
Subject: [PATCH 1/2] =?UTF-8?q?Add=20ALTER=20TYPE=20=E2=80=A6=20RENAME=20V?=
=?UTF-8?q?ALUE=20=E2=80=A6=20TO=20=E2=80=A6?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Unlike adding values to an enum, altering an existing value doesn't
affect the OID values or ordering, so can be done in a transaction
regardless of whether the enum was created in the same transaction.
---
doc/src/sgml/ref/alter_type.sgml | 18 +++++++-
src/backend/catalog/pg_enum.c | 91 ++++++++++++++++++++++++++++++++++++++
src/backend/commands/typecmds.c | 12 +++--
src/backend/parser/gram.y | 14 ++++++
src/include/catalog/pg_enum.h | 2 +
src/include/nodes/parsenodes.h | 1 +
src/test/regress/expected/enum.out | 58 ++++++++++++++++++++++++
src/test/regress/sql/enum.sql | 27 +++++++++++
8 files changed, 217 insertions(+), 6 deletions(-)
diff --git a/doc/src/sgml/ref/alter_type.sgml b/doc/src/sgml/ref/alter_type.sgml
index aec73f6..bdc2fa2 100644
--- a/doc/src/sgml/ref/alter_type.sgml
+++ b/doc/src/sgml/ref/alter_type.sgml
@@ -29,6 +29,7 @@ ALTER TYPE <replaceable class="PARAMETER">name</replaceable> RENAME ATTRIBUTE <r
ALTER TYPE <replaceable class="PARAMETER">name</replaceable> RENAME TO <replaceable class="PARAMETER">new_name</replaceable>
ALTER TYPE <replaceable class="PARAMETER">name</replaceable> SET SCHEMA <replaceable class="PARAMETER">new_schema</replaceable>
ALTER TYPE <replaceable class="PARAMETER">name</replaceable> ADD VALUE [ IF NOT EXISTS ] <replaceable class="PARAMETER">new_enum_value</replaceable> [ { BEFORE | AFTER } <replaceable class="PARAMETER">existing_enum_value</replaceable> ]
+ALTER TYPE <replaceable class="PARAMETER">name</replaceable> RENAME VALUE <replaceable class="PARAMETER">existing_enum_value</replaceable> TO <replaceable class="PARAMETER">new_enum_value</replaceable>
<phrase>where <replaceable class="PARAMETER">action</replaceable> is one of:</phrase>
@@ -123,6 +124,17 @@ ALTER TYPE <replaceable class="PARAMETER">name</replaceable> ADD VALUE [ IF NOT
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>RENAME VALUE TO</literal></term>
+ <listitem>
+ <para>
+ This form renames a value in an enum type.
+ The value's place in the enum's ordering is not affected.
+ An error will occur if the old value is not already present or the new value is.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><literal>CASCADE</literal></term>
<listitem>
@@ -241,7 +253,8 @@ ALTER TYPE <replaceable class="PARAMETER">name</replaceable> ADD VALUE [ IF NOT
<term><replaceable class="PARAMETER">new_enum_value</replaceable></term>
<listitem>
<para>
- 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 to rename an existing one to.
Like all enum literals, it needs to be quoted.
</para>
</listitem>
@@ -252,7 +265,8 @@ ALTER TYPE <replaceable class="PARAMETER">name</replaceable> ADD VALUE [ IF NOT
<listitem>
<para>
The existing enum value that the new value should be added immediately
- before or after in the enum type's sort ordering.
+ before or after in the enum type's sort ordering,
+ or renamed from.
Like all enum literals, it needs to be quoted.
</para>
</listitem>
diff --git a/src/backend/catalog/pg_enum.c b/src/backend/catalog/pg_enum.c
index c66f963..2e48dfe 100644
--- a/src/backend/catalog/pg_enum.c
+++ b/src/backend/catalog/pg_enum.c
@@ -465,6 +465,97 @@ AddEnumLabel(Oid enumTypeOid,
}
+/*
+ * RenameEnumLabel
+ * Rename a label in an enum set.
+ */
+void
+RenameEnumLabel(Oid enumTypeOid,
+ const char *oldVal,
+ const char *newVal)
+{
+ Relation pg_enum;
+ HeapTuple old_tup = NULL;
+ HeapTuple enum_tup;
+ CatCList *list;
+ int nelems;
+ int nbr_index;
+ Form_pg_enum nbr_en;
+ bool found_new = false;
+
+ /* 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. Without that, we couldn't be sure to get a consistent view
+ * of the enum members via the syscache. Note that this does not block
+ * other backends from inspecting the type; see comments for
+ * RenumberEnumType.
+ */
+ 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 this anyway, but we
+ * prefer a friendlier error message.
+ */
+ for (nbr_index = 0; nbr_index < nelems; nbr_index++)
+ {
+ enum_tup = &(list->members[nbr_index]->tuple);
+ nbr_en = (Form_pg_enum) GETSTRUCT(enum_tup);
+
+ if (strcmp(NameStr(nbr_en->enumlabel), oldVal) == 0)
+ old_tup = enum_tup;
+
+ if (strcmp(NameStr(nbr_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)));
+ }
+
+ enum_tup = heap_copytuple(old_tup);
+ nbr_en = (Form_pg_enum) GETSTRUCT(enum_tup);
+ ReleaseCatCacheList(list);
+
+ /* Update new pg_enum entry */
+ namestrcpy(&nbr_en->enumlabel, newVal);
+ simple_heap_update(pg_enum, &enum_tup->t_self, enum_tup);
+ CatalogUpdateIndexes(pg_enum, enum_tup);
+ heap_freetuple(enum_tup);
+
+ /* Make the updates visible */
+ CommandCounterIncrement();
+
+ 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 6cc7106..2807219 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -1241,10 +1241,14 @@ 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);
+ if (stmt->oldVal)
+ /* Update the existing label */
+ RenameEnumLabel(enum_type_oid, stmt->oldVal, stmt->newVal);
+ else
+ /* Add the new label */
+ AddEnumLabel(enum_type_oid, stmt->newVal,
+ stmt->newValNeighbor, stmt->newValIsAfter,
+ stmt->skipIfExists);
InvokeObjectPostAlterHook(TypeRelationId, enum_type_oid, 0);
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b69a77a..21fc2f1 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -5257,6 +5257,7 @@ enum_val_list: Sconst
{
AlterEnumStmt *n = makeNode(AlterEnumStmt);
n->typeName = $3;
+ n->oldVal = NULL;
n->newVal = $7;
n->newValNeighbor = NULL;
n->newValIsAfter = true;
@@ -5267,6 +5268,7 @@ enum_val_list: Sconst
{
AlterEnumStmt *n = makeNode(AlterEnumStmt);
n->typeName = $3;
+ n->oldVal = NULL;
n->newVal = $7;
n->newValNeighbor = $9;
n->newValIsAfter = false;
@@ -5277,12 +5279,24 @@ enum_val_list: Sconst
{
AlterEnumStmt *n = makeNode(AlterEnumStmt);
n->typeName = $3;
+ n->oldVal = NULL;
n->newVal = $7;
n->newValNeighbor = $9;
n->newValIsAfter = true;
n->skipIfExists = $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 = true;
+ n->skipIfExists = false;
+ $$ = (Node *) n;
+ }
;
opt_if_not_exists: IF_P NOT EXISTS { $$ = true; }
diff --git a/src/include/catalog/pg_enum.h b/src/include/catalog/pg_enum.h
index dd32443..b3e53ac 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 3716c2e..15380af 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2709,6 +2709,7 @@ typedef struct AlterEnumStmt
NodeTag type;
List *typeName; /* qualified name (list of Value strings) */
char *newVal; /* new enum value's name */
+ char *oldVal; /* old enum value's name when renaming */
char *newValNeighbor; /* neighboring enum value, if specified */
bool newValIsAfter; /* place new enum value after neighbor? */
bool skipIfExists; /* no error if label already exists */
diff --git a/src/test/regress/expected/enum.out b/src/test/regress/expected/enum.out
index d4a45a3..f288c79 100644
--- a/src/test/regress/expected/enum.out
+++ b/src/test/regress/expected/enum.out
@@ -556,6 +556,64 @@ 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 that we can rename 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 that renaming a value in a transaction works
+BEGIN;
+ALTER TYPE rainbow RENAME VALUE 'blue' TO 'bleu';
+COMMIT;
+SELECT enumlabel, enumsortorder
+FROM pg_enum
+WHERE enumtypid = 'rainbow'::regtype
+ORDER BY 2;
+ enumlabel | enumsortorder
+-----------+---------------
+ crimson | 1
+ orange | 2
+ yellow | 3
+ green | 4
+ bleu | 5
+ purple | 6
+(6 rows)
+
+-- check that rolling back a rename works
+BEGIN;
+ALTER TYPE rainbow RENAME VALUE 'bleu' TO 'blue';
+ROLLBACK;
+SELECT enumlabel, enumsortorder
+FROM pg_enum
+WHERE enumtypid = 'rainbow'::regtype
+ORDER BY 2;
+ enumlabel | enumsortorder
+-----------+---------------
+ crimson | 1
+ orange | 2
+ yellow | 3
+ green | 4
+ bleu | 5
+ purple | 6
+(6 rows)
+
--
-- 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 d25e8de..160cc3d 100644
--- a/src/test/regress/sql/enum.sql
+++ b/src/test/regress/sql/enum.sql
@@ -257,6 +257,33 @@ CREATE TYPE bogus AS ENUM('good', 'bad', 'ugly');
CREATE TABLE enumtest_bogus_child(parent bogus REFERENCES enumtest_parent);
DROP TYPE bogus;
+-- check that we can rename 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 that renaming a value in a transaction works
+BEGIN;
+ALTER TYPE rainbow RENAME VALUE 'blue' TO 'bleu';
+COMMIT;
+SELECT enumlabel, enumsortorder
+FROM pg_enum
+WHERE enumtypid = 'rainbow'::regtype
+ORDER BY 2;
+-- check that rolling back a rename works
+BEGIN;
+ALTER TYPE rainbow RENAME VALUE 'bleu' TO 'blue';
+ROLLBACK;
+SELECT enumlabel, enumsortorder
+FROM pg_enum
+WHERE enumtypid = 'rainbow'::regtype
+ORDER BY 2;
+
--
-- check transactional behaviour of ALTER TYPE ... ADD VALUE
--
--
2.9.3
--
"The surreality of the universe tends towards a maximum" -- Skud's Law
"Never formulate a law or axiom that you're not prepared to live with
the consequences of." -- Skud's Meta-Law
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
