<title>Notes</title>
<para>
- <command>ALTER TYPE ... ADD VALUE</command> (the form that adds a new value to an
- enum type) cannot be executed inside a transaction block.
+ If <command>ALTER TYPE ... ADD VALUE</command> (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.
</para>
<para>
#include "access/session.h"
#include "access/xact.h"
#include "access/xlog.h"
+#include "catalog/pg_enum.h"
#include "catalog/index.h"
#include "catalog/namespace.h"
#include "commands/async.h"
#define PARALLEL_KEY_SESSION_DSM UINT64CONST(0xFFFFFFFFFFFF000A)
#define PARALLEL_KEY_REINDEX_STATE UINT64CONST(0xFFFFFFFFFFFF000B)
#define PARALLEL_KEY_RELMAPPER_STATE UINT64CONST(0xFFFFFFFFFFFF000C)
+#define PARALLEL_KEY_ENUMBLACKLIST UINT64CONST(0xFFFFFFFFFFFF000D)
/* Fixed-size parallel state. */
typedef struct FixedParallelState
Size tstatelen = 0;
Size reindexlen = 0;
Size relmapperlen = 0;
+ Size enumblacklistlen = 0;
Size segsize = 0;
int i;
FixedParallelState *fps;
shm_toc_estimate_chunk(&pcxt->estimator, reindexlen);
relmapperlen = EstimateRelationMapSpace();
shm_toc_estimate_chunk(&pcxt->estimator, relmapperlen);
+ enumblacklistlen = EstimateEnumBlacklistSpace();
+ shm_toc_estimate_chunk(&pcxt->estimator, enumblacklistlen);
/* If you add more chunks here, you probably need to add keys. */
- shm_toc_estimate_keys(&pcxt->estimator, 9);
+ shm_toc_estimate_keys(&pcxt->estimator, 10);
/* Estimate space need for error queues. */
StaticAssertStmt(BUFFERALIGN(PARALLEL_ERROR_QUEUE_SIZE) ==
char *error_queue_space;
char *session_dsm_handle_space;
char *entrypointstate;
+ char *enumblacklistspace;
Size lnamelen;
/* Serialize shared libraries we have loaded. */
shm_toc_insert(pcxt->toc, PARALLEL_KEY_RELMAPPER_STATE,
relmapperspace);
+ /* Serialize enum blacklist state. */
+ enumblacklistspace = shm_toc_allocate(pcxt->toc, enumblacklistlen);
+ SerializeEnumBlacklist(enumblacklistspace, enumblacklistlen);
+ shm_toc_insert(pcxt->toc, PARALLEL_KEY_ENUMBLACKLIST,
+ enumblacklistspace);
+
/* Allocate space for worker information. */
pcxt->worker = palloc0(sizeof(ParallelWorkerInfo) * pcxt->nworkers);
char *tstatespace;
char *reindexspace;
char *relmapperspace;
+ char *enumblacklistspace;
StringInfoData msgbuf;
char *session_dsm_handle_space;
relmapperspace = shm_toc_lookup(toc, PARALLEL_KEY_RELMAPPER_STATE, false);
RestoreRelationMap(relmapperspace);
+ /* Restore enum blacklist. */
+ enumblacklistspace = shm_toc_lookup(toc, PARALLEL_KEY_ENUMBLACKLIST,
+ false);
+ RestoreEnumBlacklist(enumblacklistspace);
+
/*
* We've initialized all of our state now; nothing should change
* hereafter.
#include "access/xloginsert.h"
#include "access/xlogutils.h"
#include "catalog/namespace.h"
+#include "catalog/pg_enum.h"
#include "catalog/storage.h"
#include "commands/async.h"
#include "commands/tablecmds.h"
AtCommit_Notify();
AtEOXact_GUC(true, 1);
AtEOXact_SPI(true);
+ AtEOXact_Enum();
AtEOXact_on_commit_actions(true);
AtEOXact_Namespace(true, is_parallel_worker);
AtEOXact_SMgr();
/* 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();
AtEOXact_GUC(false, 1);
AtEOXact_SPI(false);
+ AtEOXact_Enum();
AtEOXact_on_commit_actions(false);
AtEOXact_Namespace(false, is_parallel_worker);
AtEOXact_SMgr();
#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"
/* 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);
heap_close(pg_enum, RowExclusiveLock);
}
+/*
+ * Initialize the enum blacklist for this transaction.
+ */
+static void
+init_enum_blacklist(void)
+{
+ 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);
+}
/*
* AddEnumLabel
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)
+ init_enum_blacklist();
+
+ /* Add the new value to the blacklist */
+ (void) hash_search(enum_blacklist, &newOid, HASH_ENTER, NULL);
}
}
+/*
+ * 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.
else
return 0;
}
+
+Size
+EstimateEnumBlacklistSpace(void)
+{
+ size_t entries;
+
+ if (enum_blacklist)
+ entries = hash_get_num_entries(enum_blacklist);
+ else
+ entries = 0;
+
+ /* Add one for the terminator. */
+ return sizeof(Oid) * (entries + 1);
+}
+
+void
+SerializeEnumBlacklist(void *space, Size size)
+{
+ Oid *serialized = (Oid *) space;
+
+ /*
+ * Make sure the hash table hasn't changed in size since the caller
+ * reserved the space.
+ */
+ Assert(size == EstimateEnumBlacklistSpace());
+
+ /* Write out all the values from the hash table, if there is one. */
+ if (enum_blacklist)
+ {
+ HASH_SEQ_STATUS status;
+ Oid *value;
+
+ hash_seq_init(&status, enum_blacklist);
+ while ((value = (Oid *) hash_seq_search(&status)))
+ *serialized++ = *value;
+ }
+
+ /* Write out the terminator. */
+ *serialized = InvalidOid;
+
+ /*
+ * Make sure the amount of space we actually used matches what was
+ * estimated.
+ */
+ Assert((char *) (serialized + 1) == ((char *) space) + size);
+}
+
+void
+RestoreEnumBlacklist(void *space)
+{
+ Oid *serialized = (Oid *) space;
+
+ Assert(!enum_blacklist);
+
+ /*
+ * As a special case, if the list is empty then don't even bother to
+ * create the hash table. This is the usual case, since enum alteration
+ * is expected to be rare.
+ */
+ if (!OidIsValid(*serialized))
+ return;
+
+ /* Read all the values into a new hash table. */
+ init_enum_blacklist();
+ do
+ {
+ hash_search(enum_blacklist, serialized++, HASH_ENTER, NULL);
+ } while (OidIsValid(*serialized));
+}
/*
* AlterEnum
- * ALTER TYPE on an enum.
+ * Adds a new label to an existing enum.
*/
ObjectAddress
-AlterEnum(AlterEnumStmt *stmt, bool isTopLevel)
+AlterEnum(AlterEnumStmt *stmt)
{
Oid enum_type_oid;
TypeName *typename;
/* 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 */
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
- PreventInTransactionBlock(isTopLevel, "ALTER TYPE ... ADD");
-
AddEnumLabel(enum_type_oid, stmt->newVal,
stmt->newValNeighbor, stmt->newValIsAfter,
stmt->skipIfNewValExists);
ObjectAddressSet(address, TypeRelationId, enum_type_oid);
- ReleaseSysCache(tup);
-
return address;
}
break;
case T_AlterEnumStmt: /* ALTER TYPE (enum) */
- address = AlterEnum((AlterEnumStmt *) parsetree, isTopLevel);
+ address = AlterEnum((AlterEnumStmt *) parsetree);
break;
case T_ViewStmt: /* CREATE VIEW */
#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"
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
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.
format_type_be(enumtypoid),
name)));
+ /* check it's safe to use in SQL */
+ check_safe_enum_use(tup);
+
enumoid = HeapTupleGetOid(tup);
ReleaseSysCache(tup);
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);
if (left_found)
{
+ /* check it's safe to use in SQL */
+ check_safe_enum_use(enum_tuple);
+
if (cnt >= max)
{
max *= 2;
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
bool skipIfExists);
extern void RenameEnumLabel(Oid enumTypeOid,
const char *oldVal, const char *newVal);
+extern bool EnumBlacklisted(Oid enum_id);
+extern Size EstimateEnumBlacklistSpace(void);
+extern void SerializeEnumBlacklist(void *space, Size size);
+extern void RestoreEnumBlacklist(void *space);
+extern void AtEOXact_Enum(void);
#endif /* PG_ENUM_H */
extern ObjectAddress DefineDomain(CreateDomainStmt *stmt);
extern ObjectAddress DefineEnum(CreateEnumStmt *stmt);
extern ObjectAddress DefineRange(CreateRangeStmt *stmt);
-extern ObjectAddress AlterEnum(AlterEnumStmt *stmt, bool isTopLevel);
+extern ObjectAddress AlterEnum(AlterEnumStmt *stmt);
extern ObjectAddress DefineCompositeType(RangeVar *typevar, List *coldeflist);
extern Oid AssignTypeArrayOid(void);
-- 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
+-- check that we can add new values to existing enums in a transaction
+-- but we can't use them
BEGIN;
-ALTER TYPE bogus ADD VALUE 'bad';
-ERROR: ALTER TYPE ... ADD cannot run inside a transaction block
+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;
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
+-- modified in the current txn; this should not be considered safe
BEGIN;
ALTER TYPE bogus RENAME TO bogon;
ALTER TYPE bogon ADD VALUE 'bad';
-ERROR: ALTER TYPE ... ADD cannot run inside a transaction block
+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.
ROLLBACK;
--- but ALTER TYPE RENAME VALUE is safe in a transaction
+-- 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 we *can* add new values to existing enums in a transaction,
--- if the type is new as well
+-- 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;
+-- 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();
-ALTER TYPE bogus ADD VALUE 'good';
-ALTER TYPE bogus ADD VALUE 'ugly';
+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.
ROLLBACK;
--
-- Cleanup
--
CREATE TYPE bogus AS ENUM('good');
--- check that we can't add new values to existing enums in a transaction
+-- check that we can add new values to existing enums in a transaction
+-- but we can't use them
BEGIN;
-ALTER TYPE bogus ADD VALUE 'bad';
+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;
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
+-- modified in the current txn; this should not be considered safe
BEGIN;
ALTER TYPE bogus RENAME TO bogon;
ALTER TYPE bogon ADD VALUE 'bad';
+SELECT 'bad'::bogon;
ROLLBACK;
--- but ALTER TYPE RENAME VALUE is safe in a transaction
+-- but a renamed value is safe to use later in same transaction
BEGIN;
ALTER TYPE bogus RENAME VALUE 'good' to 'bad';
SELECT 'bad'::bogus;
DROP TYPE bogus;
--- check that we *can* add new values to existing enums in a transaction,
--- if the type is new as well
+-- 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;
+
+-- 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();
-ALTER TYPE bogus ADD VALUE 'good';
-ALTER TYPE bogus ADD VALUE 'ugly';
+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
ROLLBACK;
--
projects / postgresql / commitdiff