transactional_enum-additions-v1x.patch

diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 71d4df9..13231bb 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -1237,23 +1237,13 @@ AlterEnum(AlterEnumStmt *stmt, bool isTopLevel)
 		elog(ERROR, "cache lookup failed for type %u", enum_type_oid);
 
 	/*
-	 * 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
-		PreventTransactionChain(isTopLevel, "ALTER TYPE ... ADD");
+	 * We used to prevent this from happening inside a transaction block in
+	 * situations where the new value could get into an upper level index page
+	 * and cause corruption on rollback. Now we instead detect use of the new
+	 * value in such situations and raise an exception, so that just adding to
+	 * an enum in a transaction block is now safe.
+	 * See enum.c:check_safe_enum_use()
+	 */
 
 	/* Check it's an enum and check user has permission to ALTER the enum */
 	checkEnumOwner(tup);
diff --git a/src/backend/utils/adt/enum.c b/src/backend/utils/adt/enum.c
index 135a544..7e0977e 100644
--- a/src/backend/utils/adt/enum.c
+++ b/src/backend/utils/adt/enum.c
@@ -16,6 +16,7 @@
 #include "access/genam.h"
 #include "access/heapam.h"
 #include "access/htup_details.h"
+#include "access/xact.h"
 #include "catalog/indexing.h"
 #include "catalog/pg_enum.h"
 #include "libpq/pqformat.h"
@@ -30,6 +31,60 @@
 static Oid	enum_endpoint(Oid enumtypoid, ScanDirection direction);
 static ArrayType *enum_range_internal(Oid enumtypoid, Oid lower, Oid upper);
 
+/*
+ * We need to make sure that new enum values don't get into the upper levels
+ * of indexes where we can't roll them back. To do that we prevent the use
+ * of enum values where the enum row is not committed, unless the type itself
+ * has been created in the current transaction. In the latter case any
+ * rollback would involve rolling back the whole index, since it couldn't
+ * exist before the enum type, and thus we couldn't get a corrupted index.
+ * 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. Narrowing down the unsafe conditions
+ * is left as an exercise for another day.
+ *
+ * This function needs to be called directly or indirectly in any of the
+ * functions below that could  produce an enum value, particulary, enum_in,
+ * enum_recv, enum_first, enum_last, and all the enum_range variants.
+ */
+static void
+check_safe_enum_use(char * name, Oid enumtypoid, HeapTuple enum_tup)
+{
+	/* First see if it's a new enum value - if not it's safe */
+	if (!HeapTupleHeaderXminCommitted(enum_tup->t_data))
+	{
+
+		/*
+		 * It is a new enum value, so now check to see if the whole enum is
+		 * new. If it is, we're safe.
+		 */
+		HeapTuple tup;
+
+		tup = SearchSysCache1(TYPEOID, ObjectIdGetDatum(enumtypoid));
+		Assert(HeapTupleIsValid(tup));
+
+		if (HeapTupleHeaderGetXmin(tup->t_data) ==GetCurrentTransactionId() &&
+			!(tup->t_data->t_infomask & HEAP_UPDATED))
+			/* safe, must have been created in this transaction */ ;
+		else
+			/*
+			 * We can't be in a top level transaction here, because the
+			 * enum row hasn't been committed. There might well be other
+			 * tests we could do here to narrow down the unsafe conditions,
+			 * but for now just raise an exception.
+			 */
+			ereport(ERROR,
+					(errcode(ERRCODE_UNSAFE_NEW_ENUM_VALUE_USAGE),
+					 errmsg("unsafe use of new value for enum %s: \"%s\"",
+						format_type_be(enumtypoid),
+							name),
+					 errhint("New enum values must be committed before they can be used.")));
+
+		ReleaseSysCache(tup);
+	}
+}
+
 
 /* Basic I/O support */
 
@@ -59,6 +114,8 @@ enum_in(PG_FUNCTION_ARGS)
 						format_type_be(enumtypoid),
 						name)));
 
+	check_safe_enum_use(name, enumtypoid, tup);
+
 	/*
 	 * This comes from pg_enum.oid and stores system oids in user tables. This
 	 * oid must be preserved by binary upgrades.
@@ -124,6 +181,8 @@ enum_recv(PG_FUNCTION_ARGS)
 						format_type_be(enumtypoid),
 						name)));
 
+	check_safe_enum_use(name, enumtypoid, tup);
+
 	enumoid = HeapTupleGetOid(tup);
 
 	ReleaseSysCache(tup);
@@ -327,9 +386,18 @@ enum_endpoint(Oid enumtypoid, ScanDirection direction)
 
 	enum_tuple = systable_getnext_ordered(enum_scan, direction);
 	if (HeapTupleIsValid(enum_tuple))
+	{
+		Form_pg_enum en;
+
+		en = (Form_pg_enum) GETSTRUCT(enum_tuple);
+		check_safe_enum_use(NameStr(en->enumlabel), enumtypoid, 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);
@@ -490,6 +558,11 @@ enum_range_internal(Oid enumtypoid, Oid lower, Oid upper)
 
 		if (left_found)
 		{
+			Form_pg_enum en;
+
+			en = (Form_pg_enum) GETSTRUCT(enum_tuple);
+			check_safe_enum_use(NameStr(en->enumlabel), enumtypoid, enum_tuple);
+
 			if (cnt >= max)
 			{
 				max *= 2;
diff --git a/src/backend/utils/errcodes.txt b/src/backend/utils/errcodes.txt
index be924d5..c6495d2 100644
--- a/src/backend/utils/errcodes.txt
+++ b/src/backend/utils/errcodes.txt
@@ -199,6 +199,7 @@ Section: Class 22 - Data Exception
 22P03    E    ERRCODE_INVALID_BINARY_REPRESENTATION                          invalid_binary_representation
 22P04    E    ERRCODE_BAD_COPY_FILE_FORMAT                                   bad_copy_file_format
 22P05    E    ERRCODE_UNTRANSLATABLE_CHARACTER                               untranslatable_character
+22P07    E    ERRCODE_UNSAFE_NEW_ENUM_VALUE_USAGE                            unsafe_new_enum_value_usage
 2200L    E    ERRCODE_NOT_AN_XML_DOCUMENT                                    not_an_xml_document
 2200M    E    ERRCODE_INVALID_XML_DOCUMENT                                   invalid_xml_document
 2200N    E    ERRCODE_INVALID_XML_CONTENT                                    invalid_xml_content
diff --git a/src/test/regress/expected/enum.out b/src/test/regress/expected/enum.out
index 1a61a5b..a89fc30 100644
--- a/src/test/regress/expected/enum.out
+++ b/src/test/regress/expected/enum.out
@@ -560,25 +560,55 @@ DROP TYPE bogus;
 -- 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
+SAVEPOINT x;
+SELECT 'bad'::bogus;
+ERROR:  unsafe use of new value for enum bogus: "bad"
+LINE 1: SELECT 'bad'::bogus;
+               ^
+HINT:  New enum values must be committed before they can be used.
+ROLLBACK TO x;
+SELECT enum_first(null::bogus) as should_suceed;
+ should_suceed 
+---------------
+ good
+(1 row)
+
+SELECT enum_last(null::bogus);
+ERROR:  unsafe use of new value for enum bogus: "bad"
+HINT:  New enum values must be committed before they can be used.
+ROLLBACK TO x;
+SELECT enum_range(null::bogus);
+ERROR:  unsafe use of new value for enum bogus: "bad"
+HINT:  New enum values must be committed before they can be used.
 COMMIT;
 -- check that we recognize the case where the enum already existed but was
 -- modified in the current txn
 BEGIN;
 ALTER TYPE bogus RENAME TO bogon;
-ALTER TYPE bogon ADD VALUE 'bad';
-ERROR:  ALTER TYPE ... ADD cannot run inside a transaction block
+ALTER TYPE bogon ADD VALUE 'alsobad';
+SELECT 'alsobad'::bogon;
+ERROR:  unsafe use of new value for enum bogon: "alsobad"
+LINE 1: SELECT 'alsobad'::bogon;
+               ^
+HINT:  New enum values must be committed before they can be used.
 ROLLBACK;
 DROP TYPE bogus;
 -- check that we *can* add new values to existing enums in a transaction,
--- if the type is new as well
+-- and to use them, if the type is new as well
 BEGIN;
 CREATE TYPE bogus AS ENUM();
 ALTER TYPE bogus ADD VALUE 'good';
 ALTER TYPE bogus ADD VALUE 'ugly';
+SELECT enum_range(null::bogus);
+ enum_range  
+-------------
+ {good,ugly}
+(1 row)
+
 ROLLBACK;
 --
 -- Cleanup
diff --git a/src/test/regress/sql/enum.sql b/src/test/regress/sql/enum.sql
index 88a835e..6203c44 100644
--- a/src/test/regress/sql/enum.sql
+++ b/src/test/regress/sql/enum.sql
@@ -262,26 +262,36 @@ DROP TYPE bogus;
 --
 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';
+SAVEPOINT x;
+SELECT 'bad'::bogus;
+ROLLBACK TO x;
+SELECT enum_first(null::bogus) as should_suceed;
+SELECT enum_last(null::bogus);
+ROLLBACK TO x;
+SELECT enum_range(null::bogus);
 COMMIT;
 
 -- check that we recognize the case where the enum already existed but was
 -- modified in the current txn
 BEGIN;
 ALTER TYPE bogus RENAME TO bogon;
-ALTER TYPE bogon ADD VALUE 'bad';
+ALTER TYPE bogon ADD VALUE 'alsobad';
+SELECT 'alsobad'::bogon;
 ROLLBACK;
 
 DROP TYPE bogus;
 
 -- check that we *can* add new values to existing enums in a transaction,
--- if the type is new as well
+-- and to use them, if the type is new as well
 BEGIN;
 CREATE TYPE bogus AS ENUM();
 ALTER TYPE bogus ADD VALUE 'good';
 ALTER TYPE bogus ADD VALUE 'ugly';
+SELECT enum_range(null::bogus);
 ROLLBACK;
 
 --