Re: pg_dump versus enum types, round N+1

From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: pg_dump versus enum types, round N+1
Date: 2024-03-23 22:10:35
Message-ID: CAD5tBc+W3o_CGg25JYu5txHJyz81Zht8u4YyoSiYKob-0QSM3g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, Mar 23, 2024 at 3:00 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> I have a patch in the queue [1] that among other things tries to
> reduce the number of XIDs consumed during pg_upgrade by making
> pg_restore group its commands into batches of a thousand or so
> per transaction. This had been passing tests, so I was quite
> surprised when the cfbot started to show it as falling over.
> Investigation showed that it is now failing because 6185c9737
> added these objects to the regression tests and didn't drop them:
>
> CREATE TYPE rainbow AS ENUM ('red', 'orange', 'yellow', 'green', 'blue',
> 'purple');
> CREATE DOMAIN rgb AS rainbow CHECK (VALUE IN ('red', 'green', 'blue'));
>
> In binary-upgrade mode, pg_dump dumps the enum type like this:
>
> CREATE TYPE public.rainbow AS ENUM (
> );
> ALTER TYPE public.rainbow ADD VALUE 'red';
> ALTER TYPE public.rainbow ADD VALUE 'orange';
> ALTER TYPE public.rainbow ADD VALUE 'yellow';
> ALTER TYPE public.rainbow ADD VALUE 'green';
> ALTER TYPE public.rainbow ADD VALUE 'blue';
> ALTER TYPE public.rainbow ADD VALUE 'purple';
>
> and then, if we're within the same transaction, creation of the domain
> falls over with
>
> ERROR: unsafe use of new value "red" of enum type rainbow
> HINT: New enum values must be committed before they can be used.
>
> So I'm glad we found that sooner not later, but something needs
> to be done about it if [1] is to get committed. It doesn't seem
> particularly hard to fix though: we just have to track the enum
> type OIDs made in the current transaction, using largely the same
> approach as is already used in pg_enum.c to track enum value OIDs.
> enum.sql contains a comment opining that this is too expensive,
> but I don't see why it is as long as we don't bother to track
> commands that are nested within subtransactions. That would be a bit
> complicated to do correctly, but pg_dump/pg_restore doesn't need it.
>
> Hence, I propose the attached.
>
>
>

Makes sense, Nice clear comments.

cheers

andrew

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Korotkov 2024-03-24 01:12:11 Re: POC: Lock updated tuples in tuple_update() and tuple_delete()
Previous Message James Coleman 2024-03-23 21:43:07 Re: [DOCS] HOT - correct claim about indexes not referencing old line pointers