Date: Tue, 2 Sep 2025 09:43:40 -0500
From: Nathan Bossart <nathandbossart@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Nitin Motiani <nitinmotiani@google.com>,
	Hannu Krosing <hannuk@google.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: pg_upgrade: transfer pg_largeobject_metadata's files when
 possible
Message-ID: <aLcCnNkWtQZ5YQOF@nathan>
References: <aJ3_Gih_XW1_O2HF@nathan>
 <aLU7AhUFhsJEJ1xB@paquier.xyz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <aLU7AhUFhsJEJ1xB@paquier.xyz>
Archived-At: <https://www.postgresql.org/message-id/aLcCnNkWtQZ5YQOF%40nathan>
Precedence: bulk

On Mon, Sep 01, 2025 at 03:19:46PM +0900, Michael Paquier wrote:
> 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.

Do you think a new pg_upgrade test for security labels is worth the
trouble?  It seems doable, but it'd be an awfully expensive test for this.
On the other hand, I'm not sure there's any coverage for pg_upgrade with
security labels, so perhaps this is a good time to establish some tests.

> -        /*
> -         * 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?

No, that stuff is discarded for upgrades from those versions.  My intent
was to maintain readability by avoiding lots of version checks.  FWIW I
originally put all of the pg_large_object_metadata stuff in a separate
block, but that resulted in a lot of copy/pasted code.  I'm happy to adjust
it as you see fit.

-- 
nathan