diff options
-rw-r--r-- | doc/src/sgml/ref/alter_type.sgml | 2 | ||||
-rw-r--r-- | src/backend/access/transam/xact.c | 4 | ||||
-rw-r--r-- | src/backend/catalog/pg_enum.c | 64 | ||||
-rw-r--r-- | src/backend/utils/adt/enum.c | 9 | ||||
-rw-r--r-- | src/include/catalog/pg_enum.h | 2 | ||||
-rw-r--r-- | src/test/regress/expected/enum.out | 21 | ||||
-rw-r--r-- | src/test/regress/sql/enum.sql | 13 |
7 files changed, 113 insertions, 2 deletions
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 <replaceable class="PARAMETER">name</replaceable> RENAME VALUE <repla an enum type) is executed inside a transaction block, the new value cannot be used until after the transaction has been committed, except in the case that the enum type itself was created earlier in the same transaction. - Likewise, when a pre-existing enum value is renamed, the transaction must - be committed before the renamed value can be used. </para> <para> 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); } @@ -548,6 +579,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 @@ -634,7 +634,28 @@ 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; |