Re: Transactional enum additions - was Re: Alter or rename enum value

> Got around to looking at this. Attached is a revised version that I think
> is in committable shape. The main non-cosmetic change is that the test
> for "type was created in same transaction as new value" now consists of
> comparing the xmins of the pg_type and pg_enum rows, without consulting
> GetCurrentTransactionId(). I did not like the original coding because
> it would pointlessly disallow references to enum values that were created
> in a parent transaction of the current subxact. This way is still leaving
> some subxact use-cases on the table, as noted in the code comments, but
> it's more flexible than before.

Thank you for picking this up. The patch looks good to me. I think
this is a useful to support adding values to the enum created in the
same transaction.

> + /*
> + * 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;

This looks like transaction internal logic inside enum.c to me. Maybe
it is because I am not much familiar with the internals. I couldn't
find a similar pattern anywhere else, but it might still be useful to
invent something like HeapTupleHeaderXminReallyCommitted().

One risk about this feature is that the future enum functions would
not check if the value is safe to return. Maybe we should append a
warning to the file header about this.