Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible

On Thu, Aug 14, 2025 at 10:22:02AM -0500, Nathan Bossart wrote:
> Here is a patch. For background, the reason this is limited to upgrades
> from v16 and newer is because the aclitem data type (needed by
> pg_largeobject_metadata.lomacl) changed its storage format in v16 (see
> commit 7b378237aa). Note that the patch is essentially a revert of commit
> 12a53c732c, but there are enough differences that it should be considered a
> fresh effort.

Noted.

> Something I hadn't anticipated is that we need to take special care to
> transfer the relfilenode of pg_largeobject_metadata and its index, as was
> done for pg_largeobject in commits d498e052b4 and bbe08b8869. In fact, the
> majority of the patch is dedicated to that.
>
> My testing showed some decent, but not earth-shattering performance
> improvements from this patch. For upgrades with many large objects with
> NULL lomacl/lomowner columns, pg_upgrade was 50% faster. With non-NULL
> lomacl/lomowner, that dropped to 25%. When each large object had a
> comment, there was no change. I'm assuming that its rare to have lots of
> large objects with comments or security labels, so I don't see any need to
> expend energy trying to optimize that use-case.

I highly doubt that there are a lot of comments assigned to LOs, so
these numbers are pretty cool IMO. Security labels are a pain to test
in the upgrade path, or test_dummy_label could be extended with a new
TAP test and a pg_upgrade command.. There is some coverage with
comments on LOs in src/bin/pg_dump's 002, so that would be enough for
the comment part, at least.

> I am a bit concerned that we'll forget to add checks for new types of
> dependencies similar to comments and security labels. If we do, pg_upgrade
> should just fail to restore the schema, and fixing the code should be easy
> enough. Also, we'll need to remember to revisit this code if there's
> another storage format change for one of pg_largeobject_metadata's columns,
> but that seems unlikely to happen anytime soon. On the whole, I'm not too
> worried about either of these points.

This part does not worry me much, TBH. This stuff would require
dump/restore support and the pg_dump test suite would catch that for
commands with --binary-upgrade. So it should be hard to miss.

- /*
- * pg_largeobject
- */
if (fout->remoteVersion >= 90300)
appendPQExpBuffer(loFrozenQry, "SELECT relfrozenxid, relminmxid, relfilenode, oid\n"
"FROM pg_catalog.pg_class\n"
- "WHERE oid IN (%u, %u);\n",
- LargeObjectRelationId, LargeObjectLOidPNIndexId);
+ "WHERE oid IN (%u, %u, %u, %u);\n",
+ LargeObjectRelationId, LargeObjectLOidPNIndexId,
+ LargeObjectMetadataRelationId, LargeObjectMetadataOidIndexId);
[...]
appendPQExpBufferStr(loHorizonQry, "\n-- For binary upgrade, set pg_largeobject relfrozenxid and relminmxid\n");
+ appendPQExpBufferStr(lomHorizonQry, "\n-- For binary upgrade, set pg_largeobject_metadata relfrozenxid and relminmxid\n");
appendPQExpBufferStr(loOutQry, "\n-- For binary upgrade, preserve pg_largeobject and index relfilenodes\n");
+ appendPQExpBufferStr(lomOutQry, "\n-- For binary upgrade, preserve pg_largeobject_metadata and index relfilenodes\n");

Is all that really required when upgrading from a cluster in the
9.3~15 range?
--
Michael