Remove heuristic same-transaction test from check_safe_enum_use().

The blacklist mechanism added by the preceding commit directly fixes
most of the practical cases that the same-transaction test was meant
to cover.  What remains is use-cases like

	begin;
	create type e as enum('x');
	alter type e add value 'y';
	-- use 'y' somehow
	commit;

However, because the same-transaction test is heuristic, it fails on
small variants of that, such as renaming the type or changing its
owner.  Rather than try to explain the behavior to users, let's
remove it and just have a rule that the newly added value can't be
used before being committed, full stop.  Perhaps later it will be
worth the implementation effort and overhead to have a more accurate
test for type-was-created-in-this-transaction.  We'll wait for some
field experience with v10 before deciding to do that.

Back-patch to v10.

Discussion: https://postgr.es/m/20170922185904.1448.16585@wrigleys.postgresql.org
This commit is contained in:
Tom Lane 2017-09-26 13:12:13 -04:00
parent 1635e80d30
commit 984c92074d
4 changed files with 24 additions and 49 deletions

View File

@ -292,8 +292,7 @@ ALTER TYPE <replaceable class="PARAMETER">name</replaceable> RENAME VALUE <repla
<para>
If <command>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, except in the case
that the enum type itself was created earlier in the same transaction.
be used until after the transaction has been committed.
</para>
<para>

View File

@ -48,7 +48,14 @@ static ArrayType *enum_range_internal(Oid enumtypoid, Oid lower, Oid upper);
* 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.
* (This case is required by pg_upgrade.)
* 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.
@ -58,7 +65,6 @@ check_safe_enum_use(HeapTuple enumval_tup)
{
TransactionId xmin;
Form_pg_enum en;
HeapTuple enumtyp_tup;
/*
* If the row is hinted as committed, it's surely safe. This provides a
@ -85,40 +91,11 @@ check_safe_enum_use(HeapTuple enumval_tup)
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));
if (!HeapTupleIsValid(enumtyp_tup))
elog(ERROR, "cache lookup failed for type %u", en->enumtypid);
/*
* We insist that the type have been created in the same (sub)transaction
* as the enum value. It would be safe to allow the type's originating
* xact to be a subcommitted child of the enum value's xact, but not vice
* versa (since we might now be in a subxact of the type's originating
* xact, which could roll back along with the enum value's subxact). The
* former case seems a sufficiently weird usage pattern as to not be worth
* spending code for, so we're left with a simple equality check.
*
* We also insist that the type's pg_type row not be HEAP_UPDATED. If it
* is, we can't tell whether the row was created or only modified in the
* apparent originating xact, so it might be older than that xact. (We do
* not worry whether the enum value is HEAP_UPDATED; if it is, we might
* think it's too new and throw an unnecessary error, but we won't allow
* an unsafe case.)
*/
if (xmin == HeapTupleHeaderGetXmin(enumtyp_tup->t_data) &&
!(enumtyp_tup->t_data->t_infomask & HEAP_UPDATED))
{
/* same (sub)transaction, so safe */
ReleaseSysCache(enumtyp_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",

View File

@ -656,18 +656,16 @@ select enum_range(null::bogon);
(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
-- 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 ADD VALUE 'bad';
ALTER TYPE bogus ADD VALUE 'ugly';
SELECT enum_range(null::bogus);
enum_range
-----------------
{good,bad,ugly}
(1 row)
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

View File

@ -315,13 +315,14 @@ 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
-- 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 ADD VALUE 'bad';
ALTER TYPE bogus ADD VALUE 'ugly';
SELECT enum_range(null::bogus);
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;
--