Re: BUG #14825: enum type: unsafe use?

I wrote:
> Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
>> I see what you're saying, but my idea was slightly different. We would
>> only add to the hashtable I had in mind at the bottom AddEnumLabel().
>> Any other value, whether added in the current transaction or not, should
>> be safe, AIUI.

> Oh, I see: a list of enum values we need to blacklist, not whitelist.
> Yes, that's a significantly better idea than mine, because in common
> use-cases that would be empty or have a very small number of entries.

Oh, wait a minute ... that's not where the problem is, really.  We can
already tell reliably whether an enum value was created in the current
transaction: the is-it-committed check in check_safe_enum_use is
perfectly safe for that AFAICS.  The hard part of this is the part about
"was the enum type created in the current transaction?".  We could make
that reliable with the table I proposed of enum types created in the
current transaction, but the possible performance drag is a concern.

What I understand you to be proposing is to blacklist individual
enum values added by ALTER ADD VALUE, but *not* values created
aboriginally by CREATE TYPE AS ENUM.  (The latter are surely safe,
because the type must disappear if they do.)  However, that would
require dropping the second part of the current documentation promise:

   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.

We'd have to just say "it can't be used till the transaction has been
committed", full stop.  Otherwise we're right back into the problem of
needing to detect whether the enum type is new in transaction.

>> Maybe we should also keep a cache of whitelisted enums
>> created in the current transaction.

> What for?

I now realize that what you probably meant here was to track enum *types*,
not values, created in the current transaction.  But if we're doing that
then we don't really need the per-enum-value-blacklist part of it.

So I'm back to not being sure about the path forward.  Maybe it would be
all right to say "the value added by ADD VALUE can't be used in the same
transaction, period".  That's still a step forward compared to the pre-v10
prohibition on doing it at all.  I don't remember if there were use-cases
where we really needed the exception for new-in-transaction types.

			regards, tom lane