Re: pg_upgrade and toasted pg_largeobject

I wrote:
> Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> writes:
>> I'm happy with the solution that pg_upgrade has a step in the check
>> stage that says "catalog XYZ has a toast table but shouldn't, aborting
>> the upgrade". (Well, not _happy_, but at least it's a lot easier to
>> diagnose).

> I think though that you're defining the problem too narrowly by putting
> forward a solution that would result in an error message like that.
> If we're going to do anything here at all, I think the code should emit
> a list of the names of relations that it was unable to match up, rather
> than trying (and likely failing) to be smart about why. To take just
> one reason why, the issue might be that there were too many rels in
> the new installation, not too few.

I took a break from release-note writing and hacked up something for
this; see attached patch series.

pg_upgrade-fix-0001.patch attempts to turn get_rel_infos()'s data
collection query into less of an unmaintainable mess. In the current
code, the "regular_heap" CTE doesn't fetch heap OIDs: it collects some
index OIDs as well. The "all_indexes" CTE doesn't fetch all index OIDs:
it only fetches OIDs for toast-table indexes (although this is far from
obvious to the naked eye, since it reads both the regular_heap and
toast_heap CTEs; it's only when you notice that it's subsequently ignoring
tables with zero reltoastrelid, and therefore that having read the
toast_heap CTE was totally useless, that you realize this). Also it
forgets to check indisready, so it's fortunate that it doesn't fetch
anything but toast-table indexes, which are unlikely to be in !indisready
state. Only the toast_heap CTE actually does what it says, though rather
inefficiently since it's uselessly looking for toast tables attached to
indexes. The comments that aren't outright wrong are variously
misleading, repetitive, badly placed, and/or ungrammatical. I'd like to
commit this even if we don't use the rest, because what's there now is not
a fit basis for further development.

pg_upgrade-fix-0002.patch expands the data collection query to collect
the OID of the table associated with each index, and the OID of the base
table for each toast table. This isn't needed for pg_upgrade's other
processing but we need it in order to usefully identify mismatched tables.

pg_upgrade-fix-0003.patch actually changes gen_db_file_maps() so that
it prints identifying information about each unmatched relation.

pg_upgrade-fix-0004.patch isn't meant to get committed; it's for testing
the previous patches. It hacks the pg_upgrade test script to cause
pg_largeobject in the source DB to acquire a toast table, thereby
replicating the situation Alvaro complained of. With that hack in place,
I get

...
Copying user relation files
No match found in new cluster for old relation with OID 13270 in database "postgres": "pg_toast.pg_toast_2613", toast table for "pg_catalog.pg_largeobject"
No match found in new cluster for old relation with OID 13272 in database "postgres": "pg_toast.pg_toast_2613_index", index on "pg_toast.pg_toast_2613", toast table for "pg_catalog.pg_largeobject"

Failed to match up old and new tables in database "postgres"
Failure, exiting
+ rm -rf /tmp/pg_upgrade_check-Lui8zH
make: *** [check] Error 1

which illustrates the output this will produce for a mismatch. (If anyone
wants to bikeshed the output wording, feel free.)

Any thoughts what to do with this? We could decide that it's a bug fix
and backpatch, or decide that it's a new feature and delay till 9.7,
or decide that it's a minor bug fix and add it to 9.6 only. I kinda lean
towards the last alternative.

regards, tom lane