From 0a482f6d7434964ce51f66c260bde4a74dac4da0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Thu, 24 Mar 2016 17:50:58 +0000
Subject: [PATCH] Add ALTER TYPE ... ALTER VALUE '...' TO '...'

Unlike adding values, altering a value can be done in a transaction,
since it doesn't affect the OID values or ordering, so is safe to
rollback.

TODO: documentation, IF (NOT) EXISTS support(?)
---
 src/backend/catalog/pg_enum.c      | 88 ++++++++++++++++++++++++++++++++++++++
 src/backend/commands/typecmds.c    | 50 ++++++++++++----------
 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 ++++++++++++
 7 files changed, 218 insertions(+), 22 deletions(-)

diff --git a/src/backend/catalog/pg_enum.c b/src/backend/catalog/pg_enum.c
index af89daa..1079e6c 100644
--- a/src/backend/catalog/pg_enum.c
+++ b/src/backend/catalog/pg_enum.c
@@ -464,6 +464,94 @@ restart:
 
 
 /*
+ * RenameEnumLabel
+ *		Add a new label to the enum set. By default it goes at
+ *		the end, but the user can choose to place it before or
+ *		after any existing set member.
+ */
+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 227d382..63f030b 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -1236,32 +1236,38 @@ AlterEnum(AlterEnumStmt *stmt, bool isTopLevel)
 	if (!HeapTupleIsValid(tup))
 		elog(ERROR, "cache lookup failed for type %u", enum_type_oid);
 
-	/*
-	 * Ordinarily we disallow adding values within transaction blocks, because
-	 * we can't cope with enum OID values getting into indexes and then having
-	 * their defining pg_enum entries go away.  However, it's okay if the enum
-	 * type was created in the current transaction, since then there can be no
-	 * such indexes that wouldn't themselves go away on rollback.  (We support
-	 * this case because pg_dump --binary-upgrade needs it.)  We test this by
-	 * seeing if the pg_type row has xmin == current XID and is not
-	 * HEAP_UPDATED.  If it is HEAP_UPDATED, we can't be sure whether the type
-	 * was created or only modified in this xact.  So we are disallowing some
-	 * cases that could theoretically be safe; but fortunately pg_dump only
-	 * needs the simplest case.
-	 */
-	if (HeapTupleHeaderGetXmin(tup->t_data) == GetCurrentTransactionId() &&
-		!(tup->t_data->t_infomask & HEAP_UPDATED))
-		 /* safe to do inside transaction block */ ;
-	else
-		PreventTransactionChain(isTopLevel, "ALTER TYPE ... ADD");
+	if (!stmt->oldVal) {
+		/*
+		 * Ordinarily we disallow adding values within transaction blocks, because
+		 * we can't cope with enum OID values getting into indexes and then having
+		 * their defining pg_enum entries go away.  However, it's okay if the enum
+		 * type was created in the current transaction, since then there can be no
+		 * such indexes that wouldn't themselves go away on rollback.  (We support
+		 * this case because pg_dump --binary-upgrade needs it.)  We test this by
+		 * seeing if the pg_type row has xmin == current XID and is not
+		 * HEAP_UPDATED.  If it is HEAP_UPDATED, we can't be sure whether the type
+		 * was created or only modified in this xact.  So we are disallowing some
+		 * cases that could theoretically be safe; but fortunately pg_dump only
+		 * needs the simplest case.
+		 */
+		if (HeapTupleHeaderGetXmin(tup->t_data) == GetCurrentTransactionId() &&
+			!(tup->t_data->t_infomask & HEAP_UPDATED))
+			/* safe to do inside transaction block */ ;
+		else
+			PreventTransactionChain(isTopLevel, "ALTER TYPE ... ADD");
+	}
 
 	/* 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 1273352..b7fea7a 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -5253,6 +5253,7 @@ AlterEnumStmt:
 			{
 				AlterEnumStmt *n = makeNode(AlterEnumStmt);
 				n->typeName = $3;
+				n->oldVal = NULL;
 				n->newVal = $7;
 				n->newValNeighbor = NULL;
 				n->newValIsAfter = true;
@@ -5263,6 +5264,7 @@ AlterEnumStmt:
 			{
 				AlterEnumStmt *n = makeNode(AlterEnumStmt);
 				n->typeName = $3;
+				n->oldVal = NULL;
 				n->newVal = $7;
 				n->newValNeighbor = $9;
 				n->newValIsAfter = false;
@@ -5273,12 +5275,24 @@ AlterEnumStmt:
 			{
 				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 ALTER 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..1a06482 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 8b958b4..0fc65d4 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2693,6 +2693,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 1a61a5b..39bf239 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 ALTER 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 ALTER VALUE 'red' TO 'crimson';
+ERROR:  "red" is not an existing enum label
+-- check that renaming to an existent value fails
+ALTER TYPE rainbow ALTER VALUE 'blue' TO 'green';
+ERROR:  enum label "green" already exists
+-- check that renaming a type in a transaction works
+BEGIN;
+ALTER TYPE rainbow ALTER 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 ALTER 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 88a835e..0863df8 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 ALTER 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 ALTER VALUE 'red' TO 'crimson';
+-- check that renaming to an existent value fails
+ALTER TYPE rainbow ALTER VALUE 'blue' TO 'green';
+-- check that renaming a type in a transaction works
+BEGIN;
+ALTER TYPE rainbow ALTER 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 ALTER 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.8.0.rc3

