Re: pg_upgrade fails to detect unsupported arrays and ranges

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>
Subject: Re: pg_upgrade fails to detect unsupported arrays and ranges
Date: 2021-04-28 20:38:28
Message-ID: 7062C2A2-BF72-41EC-A058-9A31F8AAC550@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 28 Apr 2021, at 17:09, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> [ blast-from-the-past department ]
>
> I wrote:
>> Daniel Gustafsson <daniel(at)yesql(dot)se> writes:
>>> I can see the appeal of
>>> including it before the wrap, even though I personally would've held off.
>
>> Nah, I'm not gonna risk it at this stage. I concur with your point
>> that this is an ancient bug, and one that is unlikely to bite many
>> people. I'll push it Wednesday or so.
>
> I happened across a couple of further pg_upgrade oversights in the
> same vein as 29aeda6e4 et al:

Nice find, this makes a lot of sense.

> ..the same OID was 12022 in v13, 11551 in v11, etc. So if you
> had a column of type pg_enum, you'd likely get no-such-type-OID
> failures when reading the values after an upgrade. I don't see
> much use-case for doing such a thing, so it seems saner to just
> block off the possibility rather than trying to support it.

Agreed. Having implemented basically this for Greenplum I think it’s wise to
avoid it unless we really have to, it gets very complicated once the layers of
worms are peeled back.

> The attached proposed patch fixes these cases too. I generalized
> the recursive query a little more so that it could start from an
> arbitrary query yielding pg_type OIDs, rather than just one type
> name; otherwise it's pretty straightforward.
>
> Barring objections I'll apply and back-patch this soon.

Patch LGTM on reading, +1 on applying. Being on parental leave I don’t have my
dev env ready to go so I didn’t perform testing; sorry about that.

> + pg_fatal("Your installation contains system-defined composite type(s) in user tables.\n"
> + "These type OIDs are not stable across PostgreSQL versions,\n"
> + "so this cluster cannot currently be upgraded. You can\n"
> + "remove the problem tables and restart the upgrade.\n"
> + "A list of the problem columns is in the file:\n"

Would it be helpful to inform the user that they can alter/drop just the
problematic columns as a potentially less scary alternative to dropping the
entire table?

> - * The type of interest might be wrapped in a domain, array,
> + * The types of interest might be wrapped in a domain, array,

Shouldn't this be "type(s)” as in the other changes here?

--
Daniel Gustafsson https://vmware.com/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2021-04-28 20:41:25 Re: Replication slot stats misgivings
Previous Message Tom Lane 2021-04-28 19:51:27 Re: Better sanity checking for message length words