diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index 8418096c30..36de6d7e28 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); diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 491bd29a1c..a42b8e9b53 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -972,14 +972,7 @@ standard_ProcessUtility(Node *parsetree, case T_AlterEnumStmt: /* ALTER TYPE (enum) */ if (isCompleteQuery) EventTriggerDDLCommandStart(parsetree); - - /* - * We disallow this in transaction blocks, because we can't cope - * with enum OID values getting into indexes and then having their - * defining pg_enum entries go away. - */ - PreventTransactionChain(isTopLevel, "ALTER TYPE ... ADD"); - AlterEnum((AlterEnumStmt *) parsetree); + AlterEnum((AlterEnumStmt *) parsetree, isTopLevel); break; case T_ViewStmt: /* CREATE VIEW */ diff --git a/src/include/commands/typecmds.h b/src/include/commands/typecmds.h index 2351024c22..e87ca90089 100644 --- a/src/include/commands/typecmds.h +++ b/src/include/commands/typecmds.h @@ -26,7 +26,7 @@ extern void RemoveTypeById(Oid typeOid); extern void DefineDomain(CreateDomainStmt *stmt); extern void DefineEnum(CreateEnumStmt *stmt); extern void DefineRange(CreateRangeStmt *stmt); -extern void AlterEnum(AlterEnumStmt *stmt); +extern void AlterEnum(AlterEnumStmt *stmt, bool isTopLevel); extern Oid DefineCompositeType(RangeVar *typevar, List *coldeflist); extern Oid AssignTypeArrayOid(void); diff --git a/src/test/regress/expected/enum.out b/src/test/regress/expected/enum.out index ed729dddc3..36826428a0 100644 --- a/src/test/regress/expected/enum.out +++ b/src/test/regress/expected/enum.out @@ -556,6 +556,30 @@ ERROR: foreign key constraint "enumtest_bogus_child_parent_fkey" cannot be impl DETAIL: Key columns "parent" and "id" are of incompatible types: bogus and rainbow. DROP TYPE bogus; -- +-- check transactional behaviour of ALTER TYPE ... ADD VALUE +-- +CREATE TYPE bogus AS ENUM('good'); +-- check that we can't add new values to existing enums in a transaction +BEGIN; +ALTER TYPE bogus ADD VALUE 'bad'; +ERROR: ALTER TYPE ... ADD cannot run inside a transaction block +COMMIT; +-- check that we recognize the case where the enum already existed but was +-- modified in the current txn +BEGIN; +ALTER TYPE bogus RENAME TO bogon; +ALTER TYPE bogon ADD VALUE 'bad'; +ERROR: ALTER TYPE ... ADD cannot run inside a transaction block +ROLLBACK; +DROP TYPE bogus; +-- check that we *can* add new values to existing enums in a transaction, +-- if the type is new as well +BEGIN; +CREATE TYPE bogus AS ENUM(); +ALTER TYPE bogus ADD VALUE 'good'; +ALTER TYPE bogus ADD VALUE 'ugly'; +ROLLBACK; +-- -- Cleanup -- DROP TABLE enumtest_child; diff --git a/src/test/regress/sql/enum.sql b/src/test/regress/sql/enum.sql index 130a723f69..88a835e8aa 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 transactional behaviour of ALTER TYPE ... ADD VALUE +-- +CREATE TYPE bogus AS ENUM('good'); + +-- check that we can't add new values to existing enums in a transaction +BEGIN; +ALTER TYPE bogus ADD VALUE 'bad'; +COMMIT; + +-- check that we recognize the case where the enum already existed but was +-- modified in the current txn +BEGIN; +ALTER TYPE bogus RENAME TO bogon; +ALTER TYPE bogon ADD VALUE 'bad'; +ROLLBACK; + +DROP TYPE bogus; + +-- check that we *can* add new values to existing enums in a transaction, +-- if the type is new as well +BEGIN; +CREATE TYPE bogus AS ENUM(); +ALTER TYPE bogus ADD VALUE 'good'; +ALTER TYPE bogus ADD VALUE 'ugly'; +ROLLBACK; + -- -- Cleanup --