diff --git a/doc/src/sgml/ref/alter_type.sgml b/doc/src/sgml/ref/alter_type.sgml index d65f70f6741..446e08b1759 100644 --- a/doc/src/sgml/ref/alter_type.sgml +++ b/doc/src/sgml/ref/alter_type.sgml @@ -294,8 +294,6 @@ ALTER TYPE name RENAME VALUE diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 93dca7a72af..52408fc6b06 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -32,6 +32,7 @@ #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" @@ -2128,6 +2129,7 @@ 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(); @@ -2406,6 +2408,7 @@ 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(); @@ -2608,6 +2611,7 @@ 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 fe61d4daccc..0f7b36e11d8 100644 --- a/src/backend/catalog/pg_enum.c +++ b/src/backend/catalog/pg_enum.c @@ -28,6 +28,8 @@ #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" @@ -35,6 +37,17 @@ /* 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); @@ -460,6 +473,24 @@ 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); } @@ -547,6 +578,39 @@ 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/utils/adt/enum.c b/src/backend/utils/adt/enum.c index 973397cc85b..401e7299fa4 100644 --- a/src/backend/utils/adt/enum.c +++ b/src/backend/utils/adt/enum.c @@ -76,6 +76,15 @@ check_safe_enum_use(HeapTuple enumval_tup) 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; + /* It is a new enum value, so check to see if the whole enum is new */ en = (Form_pg_enum) GETSTRUCT(enumval_tup); enumtyp_tup = SearchSysCache1(TYPEOID, ObjectIdGetDatum(en->enumtypid)); diff --git a/src/include/catalog/pg_enum.h b/src/include/catalog/pg_enum.h index 5938ba5cac3..dff3d2f481c 100644 --- a/src/include/catalog/pg_enum.h +++ b/src/include/catalog/pg_enum.h @@ -69,5 +69,7 @@ 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/test/regress/expected/enum.out b/src/test/regress/expected/enum.out index 0e6030443f9..6bbe488736d 100644 --- a/src/test/regress/expected/enum.out +++ b/src/test/regress/expected/enum.out @@ -633,8 +633,29 @@ 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. +ROLLBACK; +-- but a renamed value is safe to use later in same transaction +BEGIN; +ALTER TYPE bogus RENAME VALUE 'good' to 'bad'; +SELECT 'bad'::bogus; + bogus +------- + bad +(1 row) + ROLLBACK; DROP TYPE bogus; +-- check that values created during CREATE TYPE can be used in any case +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; -- check that we can add new values to existing enums in a transaction -- and use them, if the type is new as well BEGIN; diff --git a/src/test/regress/sql/enum.sql b/src/test/regress/sql/enum.sql index d7e87143a01..eb464a72c5c 100644 --- a/src/test/regress/sql/enum.sql +++ b/src/test/regress/sql/enum.sql @@ -300,8 +300,21 @@ ALTER TYPE bogon ADD VALUE 'bad'; SELECT 'bad'::bogon; ROLLBACK; +-- but a renamed value is safe to use later in same transaction +BEGIN; +ALTER TYPE bogus RENAME VALUE 'good' to 'bad'; +SELECT 'bad'::bogus; +ROLLBACK; + DROP TYPE bogus; +-- check that values created during CREATE TYPE can be used in any case +BEGIN; +CREATE TYPE bogus AS ENUM('good','bad','ugly'); +ALTER TYPE bogus RENAME TO bogon; +select enum_range(null::bogon); +ROLLBACK; + -- check that we can add new values to existing enums in a transaction -- and use them, if the type is new as well BEGIN;