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

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>
Cc: balazs(at)obiserver(dot)hu, pgsql-bugs(at)postgresql(dot)org, pgsql-hackers(at)postgresql(dot)org
Subject: Re: BUG #14825: enum type: unsafe use?
Date: 2017-09-23 19:52:05
Message-ID: 19137.1506196325@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com> writes:
> On 09/23/2017 02:00 PM, Tom Lane wrote:
>> 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.

> Well, my idea was to have the test run like this:

> * is the value an old one? Test txnid of tuple. If yes it's ok
> * is the value one created by ALTER TYPE ADD VALUE? Test
> blacklist. If no, it's ok.
> * is the enum a new one? Test whitelist. If yes, it's ok.
> * anything else is not ok.

My point is that if you do 1 and 3, you don't need 2. Or if you do
2 and 3, you don't need 1. But in most cases, testing the tuple
hint bits is cheap, so you don't really want that option.

In any case, what I'm worried about is the amount of bookkeeping
overhead added by keeping a whitelist of enum-types-created-in-
current-transaction. That's less than trivial, especially since
you have to account correctly for subtransactions. And there are
common use-cases where that table will become large.

> If we just did the blacklist and stuck with our current heuristic test
> for enum being created in the current transaction, we'd still probably
> avoid 99% of the problems, including specifically the one that gave rise
> to the bug report.

True. But I'm not sure whether the heuristic test is adding anything
meaningful if we use a blacklist first. The case where it could help
is

begin;
create type t as enum();
alter type t add value 'v';
-- do something with 'v'
commit;

That perhaps is worth something, but if somebody is trying to build a new
enum type in pieces like that, doesn't it seem fairly likely that they
might throw in an ALTER OWNER or GRANT as well? My feeling is that the
lesson we need to learn is that the heuristic test isn't good enough.

regards, tom lane

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Andrew Dunstan 2017-09-23 21:45:46 Re: BUG #14825: enum type: unsafe use?
Previous Message Andrew Dunstan 2017-09-23 19:05:57 Re: BUG #14825: enum type: unsafe use?

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2017-09-23 20:26:23 Proposed fix for bug #14826 (the invalid empty array business)
Previous Message Fabien COELHO 2017-09-23 19:47:39 Re: Variable substitution in psql backtick expansion