Re: pg_upgrade fails to detect unsupported arrays and ranges

Daniel Gustafsson <daniel@yesql.se>

From: Daniel Gustafsson <daniel@yesql.se>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Tomas Vondra <tomas.vondra@enterprisedb.com>
Date: 2021-04-28T20:38:28Z
Lists: pgsql-hackers
> On 28 Apr 2021, at 17:09, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> [ blast-from-the-past department ]
> 
> I wrote:
>> Daniel Gustafsson <daniel@yesql.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/




Commits

  1. Improve wording of some pg_upgrade failure reports.

  2. Fix some more omissions in pg_upgrade's tests for non-upgradable types.

  3. Handle arrays and ranges in pg_upgrade's test for non-upgradable types.