diff --git a/doc/src/sgml/ref/alter_type.sgml b/doc/src/sgml/ref/alter_type.sgml
index 7e2258e1e34..4027c1b8f7d 100644
--- a/doc/src/sgml/ref/alter_type.sgml
+++ b/doc/src/sgml/ref/alter_type.sgml
@@ -290,9 +290,8 @@ ALTER TYPE name RENAME VALUE Notes
- If ALTER TYPE ... ADD VALUE (the form that adds a new value to
- an enum type) is executed inside a transaction block, the new value cannot
- be used until after the transaction has been committed.
+ ALTER TYPE ... ADD VALUE (the form that adds a new value to an
+ enum type) cannot be executed inside a transaction block.
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 52408fc6b06..93dca7a72af 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -32,7 +32,6 @@
#include "access/xlogutils.h"
#include "catalog/catalog.h"
#include "catalog/namespace.h"
-#include "catalog/pg_enum.h"
#include "catalog/storage.h"
#include "commands/async.h"
#include "commands/tablecmds.h"
@@ -2129,7 +2128,6 @@ CommitTransaction(void)
AtCommit_Notify();
AtEOXact_GUC(true, 1);
AtEOXact_SPI(true);
- AtEOXact_Enum();
AtEOXact_on_commit_actions(true);
AtEOXact_Namespace(true, is_parallel_worker);
AtEOXact_SMgr();
@@ -2408,7 +2406,6 @@ PrepareTransaction(void)
/* PREPARE acts the same as COMMIT as far as GUC is concerned */
AtEOXact_GUC(true, 1);
AtEOXact_SPI(true);
- AtEOXact_Enum();
AtEOXact_on_commit_actions(true);
AtEOXact_Namespace(true, false);
AtEOXact_SMgr();
@@ -2611,7 +2608,6 @@ AbortTransaction(void)
AtEOXact_GUC(false, 1);
AtEOXact_SPI(false);
- AtEOXact_Enum();
AtEOXact_on_commit_actions(false);
AtEOXact_Namespace(false, is_parallel_worker);
AtEOXact_SMgr();
diff --git a/src/backend/catalog/pg_enum.c b/src/backend/catalog/pg_enum.c
index 0f7b36e11d8..fe61d4daccc 100644
--- a/src/backend/catalog/pg_enum.c
+++ b/src/backend/catalog/pg_enum.c
@@ -28,8 +28,6 @@
#include "utils/builtins.h"
#include "utils/catcache.h"
#include "utils/fmgroids.h"
-#include "utils/hsearch.h"
-#include "utils/memutils.h"
#include "utils/syscache.h"
#include "utils/tqual.h"
@@ -37,17 +35,6 @@
/* Potentially set by pg_upgrade_support functions */
Oid binary_upgrade_next_pg_enum_oid = InvalidOid;
-/*
- * Hash table of enum value OIDs created during the current transaction by
- * AddEnumLabel. We disallow using these values until the transaction is
- * committed; otherwise, they might get into indexes where we can't clean
- * them up, and then if the transaction rolls back we have a broken index.
- * (See comments for check_safe_enum_use() in enum.c.) Values created by
- * EnumValuesCreate are *not* blacklisted; we assume those are created during
- * CREATE TYPE, so they can't go away unless the enum type itself does.
- */
-static HTAB *enum_blacklist = NULL;
-
static void RenumberEnumType(Relation pg_enum, HeapTuple *existing, int nelems);
static int sort_order_cmp(const void *p1, const void *p2);
@@ -473,24 +460,6 @@ restart:
heap_freetuple(enum_tup);
heap_close(pg_enum, RowExclusiveLock);
-
- /* Set up the blacklist hash if not already done in this transaction */
- if (enum_blacklist == NULL)
- {
- HASHCTL hash_ctl;
-
- memset(&hash_ctl, 0, sizeof(hash_ctl));
- hash_ctl.keysize = sizeof(Oid);
- hash_ctl.entrysize = sizeof(Oid);
- hash_ctl.hcxt = TopTransactionContext;
- enum_blacklist = hash_create("Enum value blacklist",
- 32,
- &hash_ctl,
- HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
- }
-
- /* Add the new value to the blacklist */
- (void) hash_search(enum_blacklist, &newOid, HASH_ENTER, NULL);
}
@@ -578,39 +547,6 @@ RenameEnumLabel(Oid enumTypeOid,
}
-/*
- * Test if the given enum value is on the blacklist
- */
-bool
-EnumBlacklisted(Oid enum_id)
-{
- bool found;
-
- /* If we've made no blacklist table, all values are safe */
- if (enum_blacklist == NULL)
- return false;
-
- /* Else, is it in the table? */
- (void) hash_search(enum_blacklist, &enum_id, HASH_FIND, &found);
- return found;
-}
-
-
-/*
- * Clean up enum stuff after end of top-level transaction.
- */
-void
-AtEOXact_Enum(void)
-{
- /*
- * Reset the blacklist table, as all our enum values are now committed.
- * The memory will go away automatically when TopTransactionContext is
- * freed; it's sufficient to clear our pointer.
- */
- enum_blacklist = NULL;
-}
-
-
/*
* 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 7ed16aeff46..4c490ed5c1b 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -1222,10 +1222,10 @@ DefineEnum(CreateEnumStmt *stmt)
/*
* AlterEnum
- * Adds a new label to an existing enum.
+ * ALTER TYPE on an enum.
*/
ObjectAddress
-AlterEnum(AlterEnumStmt *stmt)
+AlterEnum(AlterEnumStmt *stmt, bool isTopLevel)
{
Oid enum_type_oid;
TypeName *typename;
@@ -1243,8 +1243,6 @@ AlterEnum(AlterEnumStmt *stmt)
/* Check it's an enum and check user has permission to ALTER the enum */
checkEnumOwner(tup);
- ReleaseSysCache(tup);
-
if (stmt->oldVal)
{
/* Rename an existing label */
@@ -1253,6 +1251,27 @@ AlterEnum(AlterEnumStmt *stmt)
else
{
/* Add a new label */
+
+ /*
+ * 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");
+
AddEnumLabel(enum_type_oid, stmt->newVal,
stmt->newValNeighbor, stmt->newValIsAfter,
stmt->skipIfNewValExists);
@@ -1262,6 +1281,8 @@ AlterEnum(AlterEnumStmt *stmt)
ObjectAddressSet(address, TypeRelationId, enum_type_oid);
+ ReleaseSysCache(tup);
+
return address;
}
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 5c69ecf0f75..82a707af7b8 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1412,7 +1412,7 @@ ProcessUtilitySlow(ParseState *pstate,
break;
case T_AlterEnumStmt: /* ALTER TYPE (enum) */
- address = AlterEnum((AlterEnumStmt *) parsetree);
+ address = AlterEnum((AlterEnumStmt *) parsetree, isTopLevel);
break;
case T_ViewStmt: /* CREATE VIEW */
diff --git a/src/backend/utils/adt/enum.c b/src/backend/utils/adt/enum.c
index c0124f497e4..048a08dd852 100644
--- a/src/backend/utils/adt/enum.c
+++ b/src/backend/utils/adt/enum.c
@@ -19,7 +19,6 @@
#include "catalog/indexing.h"
#include "catalog/pg_enum.h"
#include "libpq/pqformat.h"
-#include "storage/procarray.h"
#include "utils/array.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
@@ -32,79 +31,6 @@ static Oid enum_endpoint(Oid enumtypoid, ScanDirection direction);
static ArrayType *enum_range_internal(Oid enumtypoid, Oid lower, Oid upper);
-/*
- * Disallow use of an uncommitted pg_enum tuple.
- *
- * We need to make sure that uncommitted enum values don't get into indexes.
- * If they did, and if we then rolled back the pg_enum addition, we'd have
- * broken the index because value comparisons will not work reliably without
- * an underlying pg_enum entry. (Note that removal of the heap entry
- * containing an enum value is not sufficient to ensure that it doesn't appear
- * in upper levels of indexes.) To do this we prevent an uncommitted row from
- * being used for any SQL-level purpose. This is stronger than necessary,
- * since the value might not be getting inserted into a table or there might
- * be no index on its column, but it's easy to enforce centrally.
- *
- * However, it's okay to allow use of uncommitted values belonging to enum
- * types that were themselves created in the same transaction, because then
- * any such index would also be new and would go away altogether on rollback.
- * We don't implement that fully right now, but we do allow free use of enum
- * values created during CREATE TYPE AS ENUM, which are surely of the same
- * lifespan as the enum type. (This case is required by "pg_restore -1".)
- * Values added by ALTER TYPE ADD VALUE are currently restricted, but could
- * be allowed if the enum type could be proven to have been created earlier
- * in the same transaction. (Note that comparing tuple xmins would not work
- * for that, because the type tuple might have been updated in the current
- * transaction. Subtransactions also create hazards to be accounted for.)
- *
- * This function needs to be called (directly or indirectly) in any of the
- * functions below that could return an enum value to SQL operations.
- */
-static void
-check_safe_enum_use(HeapTuple enumval_tup)
-{
- TransactionId xmin;
- Form_pg_enum en;
-
- /*
- * If the row is hinted as committed, it's surely safe. This provides a
- * fast path for all normal use-cases.
- */
- if (HeapTupleHeaderXminCommitted(enumval_tup->t_data))
- return;
-
- /*
- * Usually, a row would get hinted as committed when it's read or loaded
- * into syscache; but just in case not, let's check the xmin directly.
- */
- xmin = HeapTupleHeaderGetXmin(enumval_tup->t_data);
- if (!TransactionIdIsInProgress(xmin) &&
- TransactionIdDidCommit(xmin))
- return;
-
- /*
- * Check if the enum value is blacklisted. If not, it's safe, because it
- * was made during CREATE TYPE AS ENUM and can't be shorter-lived than its
- * owning type. (This'd also be false for values made by other
- * transactions; but the previous tests should have handled all of those.)
- */
- if (!EnumBlacklisted(HeapTupleGetOid(enumval_tup)))
- return;
-
- /*
- * There might well be other tests we could do here to narrow down the
- * unsafe conditions, but for now just raise an exception.
- */
- en = (Form_pg_enum) GETSTRUCT(enumval_tup);
- ereport(ERROR,
- (errcode(ERRCODE_UNSAFE_NEW_ENUM_VALUE_USAGE),
- errmsg("unsafe use of new value \"%s\" of enum type %s",
- NameStr(en->enumlabel),
- format_type_be(en->enumtypid)),
- errhint("New enum values must be committed before they can be used.")));
-}
-
-
/* Basic I/O support */
Datum
@@ -133,9 +59,6 @@ enum_in(PG_FUNCTION_ARGS)
format_type_be(enumtypoid),
name)));
- /* check it's safe to use in SQL */
- check_safe_enum_use(tup);
-
/*
* This comes from pg_enum.oid and stores system oids in user tables. This
* oid must be preserved by binary upgrades.
@@ -201,9 +124,6 @@ enum_recv(PG_FUNCTION_ARGS)
format_type_be(enumtypoid),
name)));
- /* check it's safe to use in SQL */
- check_safe_enum_use(tup);
-
enumoid = HeapTupleGetOid(tup);
ReleaseSysCache(tup);
@@ -411,16 +331,9 @@ enum_endpoint(Oid enumtypoid, ScanDirection direction)
enum_tuple = systable_getnext_ordered(enum_scan, direction);
if (HeapTupleIsValid(enum_tuple))
- {
- /* check it's safe to use in SQL */
- check_safe_enum_use(enum_tuple);
minmax = HeapTupleGetOid(enum_tuple);
- }
else
- {
- /* should only happen with an empty enum */
minmax = InvalidOid;
- }
systable_endscan_ordered(enum_scan);
index_close(enum_idx, AccessShareLock);
@@ -581,9 +494,6 @@ enum_range_internal(Oid enumtypoid, Oid lower, Oid upper)
if (left_found)
{
- /* check it's safe to use in SQL */
- check_safe_enum_use(enum_tuple);
-
if (cnt >= max)
{
max *= 2;
diff --git a/src/backend/utils/errcodes.txt b/src/backend/utils/errcodes.txt
index 4f354717628..76fe79eac05 100644
--- a/src/backend/utils/errcodes.txt
+++ b/src/backend/utils/errcodes.txt
@@ -400,7 +400,6 @@ Section: Class 55 - Object Not In Prerequisite State
55006 E ERRCODE_OBJECT_IN_USE object_in_use
55P02 E ERRCODE_CANT_CHANGE_RUNTIME_PARAM cant_change_runtime_param
55P03 E ERRCODE_LOCK_NOT_AVAILABLE lock_not_available
-55P04 E ERRCODE_UNSAFE_NEW_ENUM_VALUE_USAGE unsafe_new_enum_value_usage
Section: Class 57 - Operator Intervention
diff --git a/src/include/catalog/pg_enum.h b/src/include/catalog/pg_enum.h
index dff3d2f481c..5938ba5cac3 100644
--- a/src/include/catalog/pg_enum.h
+++ b/src/include/catalog/pg_enum.h
@@ -69,7 +69,5 @@ extern void AddEnumLabel(Oid enumTypeOid, const char *newVal,
bool skipIfExists);
extern void RenameEnumLabel(Oid enumTypeOid,
const char *oldVal, const char *newVal);
-extern bool EnumBlacklisted(Oid enum_id);
-extern void AtEOXact_Enum(void);
#endif /* PG_ENUM_H */
diff --git a/src/include/commands/typecmds.h b/src/include/commands/typecmds.h
index 34f6fe328fe..8f3fc655363 100644
--- a/src/include/commands/typecmds.h
+++ b/src/include/commands/typecmds.h
@@ -26,7 +26,7 @@ extern void RemoveTypeById(Oid typeOid);
extern ObjectAddress DefineDomain(CreateDomainStmt *stmt);
extern ObjectAddress DefineEnum(CreateEnumStmt *stmt);
extern ObjectAddress DefineRange(CreateRangeStmt *stmt);
-extern ObjectAddress AlterEnum(AlterEnumStmt *stmt);
+extern ObjectAddress AlterEnum(AlterEnumStmt *stmt, bool isTopLevel);
extern ObjectAddress 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 4f839ce0279..a0b81608a1b 100644
--- a/src/test/regress/expected/enum.out
+++ b/src/test/regress/expected/enum.out
@@ -581,60 +581,19 @@ ERROR: enum label "green" already exists
-- check transactional behaviour of ALTER TYPE ... ADD VALUE
--
CREATE TYPE bogus AS ENUM('good');
--- check that we can add new values to existing enums in a transaction
--- but we can't use them
+-- check that we can't add new values to existing enums in a transaction
BEGIN;
-ALTER TYPE bogus ADD VALUE 'new';
-SAVEPOINT x;
-SELECT 'new'::bogus; -- unsafe
-ERROR: unsafe use of new value "new" of enum type bogus
-LINE 1: SELECT 'new'::bogus;
- ^
-HINT: New enum values must be committed before they can be used.
-ROLLBACK TO x;
-SELECT enum_first(null::bogus); -- safe
- enum_first
-------------
- good
-(1 row)
-
-SELECT enum_last(null::bogus); -- unsafe
-ERROR: unsafe use of new value "new" of enum type bogus
-HINT: New enum values must be committed before they can be used.
-ROLLBACK TO x;
-SELECT enum_range(null::bogus); -- unsafe
-ERROR: unsafe use of new value "new" of enum type bogus
-HINT: New enum values must be committed before they can be used.
-ROLLBACK TO x;
+ALTER TYPE bogus ADD VALUE 'bad';
+ERROR: ALTER TYPE ... ADD cannot run inside a transaction block
COMMIT;
-SELECT 'new'::bogus; -- now safe
- bogus
--------
- new
-(1 row)
-
-SELECT enumlabel, enumsortorder
-FROM pg_enum
-WHERE enumtypid = 'bogus'::regtype
-ORDER BY 2;
- enumlabel | enumsortorder
------------+---------------
- good | 1
- new | 2
-(2 rows)
-
-- check that we recognize the case where the enum already existed but was
--- modified in the current txn; this should not be considered safe
+-- modified in the current txn
BEGIN;
ALTER TYPE bogus RENAME TO bogon;
ALTER TYPE bogon ADD VALUE 'bad';
-SELECT 'bad'::bogon;
-ERROR: unsafe use of new value "bad" of enum type bogon
-LINE 1: SELECT 'bad'::bogon;
- ^
-HINT: New enum values must be committed before they can be used.
+ERROR: ALTER TYPE ... ADD cannot run inside a transaction block
ROLLBACK;
--- but a renamed value is safe to use later in same transaction
+-- but ALTER TYPE RENAME VALUE is safe in a transaction
BEGIN;
ALTER TYPE bogus RENAME VALUE 'good' to 'bad';
SELECT 'bad'::bogus;
@@ -645,27 +604,12 @@ SELECT 'bad'::bogus;
ROLLBACK;
DROP TYPE bogus;
--- check that values created during CREATE TYPE can be used in any case
+-- 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('good','bad','ugly');
-ALTER TYPE bogus RENAME TO bogon;
-select enum_range(null::bogon);
- enum_range
------------------
- {good,bad,ugly}
-(1 row)
-
-ROLLBACK;
--- ideally, we'd allow this usage; but it requires keeping track of whether
--- the enum type was created in the current transaction, which is expensive
-BEGIN;
-CREATE TYPE bogus AS ENUM('good');
-ALTER TYPE bogus RENAME TO bogon;
-ALTER TYPE bogon ADD VALUE 'bad';
-ALTER TYPE bogon ADD VALUE 'ugly';
-select enum_range(null::bogon); -- fails
-ERROR: unsafe use of new value "bad" of enum type bogon
-HINT: New enum values must be committed before they can be used.
+CREATE TYPE bogus AS ENUM();
+ALTER TYPE bogus ADD VALUE 'good';
+ALTER TYPE bogus ADD VALUE 'ugly';
ROLLBACK;
--
-- Cleanup
diff --git a/src/test/regress/sql/enum.sql b/src/test/regress/sql/enum.sql
index 6affd0d1ebe..7b68b2fe376 100644
--- a/src/test/regress/sql/enum.sql
+++ b/src/test/regress/sql/enum.sql
@@ -273,34 +273,19 @@ ALTER TYPE rainbow RENAME VALUE 'blue' TO 'green';
--
CREATE TYPE bogus AS ENUM('good');
--- check that we can add new values to existing enums in a transaction
--- but we can't use them
+-- check that we can't add new values to existing enums in a transaction
BEGIN;
-ALTER TYPE bogus ADD VALUE 'new';
-SAVEPOINT x;
-SELECT 'new'::bogus; -- unsafe
-ROLLBACK TO x;
-SELECT enum_first(null::bogus); -- safe
-SELECT enum_last(null::bogus); -- unsafe
-ROLLBACK TO x;
-SELECT enum_range(null::bogus); -- unsafe
-ROLLBACK TO x;
+ALTER TYPE bogus ADD VALUE 'bad';
COMMIT;
-SELECT 'new'::bogus; -- now safe
-SELECT enumlabel, enumsortorder
-FROM pg_enum
-WHERE enumtypid = 'bogus'::regtype
-ORDER BY 2;
-- check that we recognize the case where the enum already existed but was
--- modified in the current txn; this should not be considered safe
+-- modified in the current txn
BEGIN;
ALTER TYPE bogus RENAME TO bogon;
ALTER TYPE bogon ADD VALUE 'bad';
-SELECT 'bad'::bogon;
ROLLBACK;
--- but a renamed value is safe to use later in same transaction
+-- but ALTER TYPE RENAME VALUE is safe in a transaction
BEGIN;
ALTER TYPE bogus RENAME VALUE 'good' to 'bad';
SELECT 'bad'::bogus;
@@ -308,21 +293,12 @@ ROLLBACK;
DROP TYPE bogus;
--- check that values created during CREATE TYPE can be used in any case
+-- 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('good','bad','ugly');
-ALTER TYPE bogus RENAME TO bogon;
-select enum_range(null::bogon);
-ROLLBACK;
-
--- ideally, we'd allow this usage; but it requires keeping track of whether
--- the enum type was created in the current transaction, which is expensive
-BEGIN;
-CREATE TYPE bogus AS ENUM('good');
-ALTER TYPE bogus RENAME TO bogon;
-ALTER TYPE bogon ADD VALUE 'bad';
-ALTER TYPE bogon ADD VALUE 'ugly';
-select enum_range(null::bogon); -- fails
+CREATE TYPE bogus AS ENUM();
+ALTER TYPE bogus ADD VALUE 'good';
+ALTER TYPE bogus ADD VALUE 'ugly';
ROLLBACK;
--