Allow more cases to pass the unsafe-use-of-new-enum-value restriction.

Up to now we've rejected cases like

BEGIN;
CREATE TYPE rainbow AS ENUM ();
ALTER TYPE rainbow ADD VALUE 'red';
-- use the value 'red', perhaps in a constraint or index
COMMIT;

The concern is that the uncommitted enum value 'red' might get into
an index and then break the index if we roll back the ALTER ADD.
If the ALTER is in the same transaction as the CREATE then it's really
perfectly safe, but we weren't taking the trouble to identify that.

pg_dump in binary-upgrade mode will emit enum definitions that look
like the above, which up to now didn't fall foul of the unsafe-usage
check because we processed each restore command as a separate
transaction.  However an upcoming patch proposes to bundle the restore
commands into large transactions to reduce XID consumption during
pg_upgrade, and that makes this behavior a problem.

To fix, remember the OIDs of enum types created in the current
transaction, and allow use of enum values that are added to one later
in the same transaction.  To do this fully correctly in the presence
of subtransactions, we'd have to track subtransaction nesting level of
the CREATE and do maintenance work at every subsequent subtransaction
exit.  That seems expensive, and we don't need it to satisfy pg_dump's
usage.  Hence, apply the additional optimization only when the CREATE
and ALTER are at outermost transaction level.

Patch by me, reviewed by Andrew Dunstan

Discussion: https://postgr.es/m/1548468.1711220438@sss.pgh.pa.us

1 files changed, 10 insertions, 9 deletions

diff --git a/src/backend/utils/adt/enum.c b/src/backend/utils/adt/enum.c
index f649ff2c564..814c7fb4e3e 100644
--- a/src/backend/utils/adt/enum.c
+++ b/src/backend/utils/adt/enum.c
@@ -49,11 +49,12 @@ static ArrayType *enum_range_internal(Oid enumtypoid, Oid lower, Oid upper);
  * 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.)
+ * Values added by ALTER TYPE ADD VALUE are also allowed if the enum type
+ * is known to have been created earlier in the same transaction.  (Note that
+ * we have to track that explicitly; comparing tuple xmins is insufficient,
+ * because the type tuple might have been updated in the current transaction.
+ * Subtransactions also create hazards to be accounted for; currently,
+ * pg_enum.c only handles ADD VALUE at the outermost transaction level.)
  *
  * This function needs to be called (directly or indirectly) in any of the
  * functions below that could return an enum value to SQL operations.
@@ -81,10 +82,10 @@ check_safe_enum_use(HeapTuple enumval_tup)
 		return;
 
 	/*
-	 * Check if the enum value is uncommitted.  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.)
+	 * Check if the enum value is listed as uncommitted.  If not, it's safe,
+	 * because it 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 (!EnumUncommitted(en->oid))
 		return;