Allow adding values to an enum type created in the current transaction.

Normally it is unsafe to allow ALTER TYPE ADD VALUE in a transaction block,
because instances of the value could be added to indexes later in the same
transaction, and then they would still be accessible even if the
transaction rolls back.  However, we can allow this if the enum type itself
was created in the current transaction, because then any such indexes would
have to go away entirely on rollback.

The reason for allowing this is to support pg_upgrade's new usage of
pg_restore --single-transaction: in --binary-upgrade mode, pg_dump emits
enum types as a succession of ALTER TYPE ADD VALUE commands so that it can
preserve the values' OIDs.  The support is a bit limited, so we'll leave
it undocumented.

Andres Freund

1 files changed, 21 insertions, 2 deletions

diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 8418096c308..36de6d7e28b 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -1169,7 +1169,7 @@ DefineEnum(CreateEnumStmt *stmt)
  *		Adds a new label to an existing enum.
  */
 void
-AlterEnum(AlterEnumStmt *stmt)
+AlterEnum(AlterEnumStmt *stmt, bool isTopLevel)
 {
 	Oid			enum_type_oid;
 	TypeName   *typename;
@@ -1183,12 +1183,31 @@ AlterEnum(AlterEnumStmt *stmt)
 	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");
+
 	/* 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->newValNeighbor, stmt->newValIsAfter,
 				 stmt->skipIfExists);
 
 	ReleaseSysCache(tup);