Feedback-ID: i0fe9450f:Fastmail
Date: Thu, 4 Sep 2025 13:59:36 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Nathan Bossart <nathandbossart@gmail.com>
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: <aLkcuKPYaLJ4RipR@paquier.xyz>
References: <aJ3_Gih_XW1_O2HF@nathan>
 <aLU7AhUFhsJEJ1xB@paquier.xyz>
 <aLcCnNkWtQZ5YQOF@nathan>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="HghOGTkopIgjDG0O"
Content-Disposition: inline
In-Reply-To: <aLcCnNkWtQZ5YQOF@nathan>
Archived-At: 
 <https://www.postgresql.org/message-id/aLkcuKPYaLJ4RipR%40paquier.xyz>
Precedence: bulk


--HghOGTkopIgjDG0O
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Sep 02, 2025 at 09:43:40AM -0500, Nathan Bossart wrote:
> 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.

I would argue in favor of these additions.  Security labels are not
the most popular thing ever, AFAIK, but your patch makes the need more
relevant to have.  The cheapest approach would be to add a LO creation
pattern in src/bin/pg_dump/t/002_pg_dump.pl, with an EXTRA_INSTALL
pointing at src/test/modules/dummy_seclabel/ to be able to create the
security label (we already do that in pg_upgrade and pg_basebackup so
the trick works).  That should be enough to make sure that the binary
upgrade dumps have the seclabel data included.  It's a bit funky, I
agree.  So if you think that this is not worth the test cycles, I
won't push hard on this point, either.

> 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 adju=
st
> it as you see fit.

+        if (fout->remoteVersion >=3D 160000)
+            ArchiveEntry(fout, nilCatalogId, createDumpId(),
+                         ARCHIVE_OPTS(.tag =3D "pg_largeobject_metadata",
+                                      .description =3D "pg_largeobject_met=
adata",
+                                      .section =3D SECTION_PRE_DATA,
+                                      .createStmt =3D lomOutQry->data));

So it's filtered out depending on the old cluster version here.  I
have managed to miss this part.  Some tests with LOs and across
multiple backend versions (pg_dump -C --binary-upgrade) are showing me
that I am wrong and that you are right, with diffs showing up properly
in the binary upgrade dumps for pg_largeobject_metadata.  This logic
looks OK to me, even if it's a waste to fill lomOutQry when upgrading
=66rom an instance in the 9.3~15 range, discarding ArchiveEntry() at the
end.  It's not a big deal.
--
Michael

--HghOGTkopIgjDG0O
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmi5HLgACgkQnvQgOdby
QH173xAAqrxfhgXRVVKBcVQ7MFZJT0eGy3W7pBBQ0LSgaEEsO2GC7VqPTP+BVS9B
lYtukLIa1c53WsTHI+u7SnwtMeSqHgjGxLrF3nl/fEAIARBfR0jsxLqrMVWMCHI1
xtIYMh7bgscvYc1V86r21CMD0ncSopuTw/IQimoqDIEqZGLN1RCqkNQdxNy3+nOv
XfELv8yeGKY5Cj9BZkFIZRDkXf6wKi6GwDo4HXOV7u5GNRU3N0147h4IM9ZUF7i0
IDg6X3JykCcXdZqTYcA/TfeNzIJaA4QSfdV3rFCSYWCNSxTgk41725wlOcuuNpM6
JBoDGBunTe5px+Nn7r7TOEr4+KA9mjWYYka++LNfvAUwcHOa6ya7ilqV7WmqeVWm
Dww+kkt86Hpy1KntdDDdShIbmAZu8WqyBuBka2NOypSl83Y5+yCEDrxnlY8eqc0q
pvXszEYkhXLgRythUYpsPKxRyfnA4kypV3LP2n+SYB4HyradWy144S9EhPb1sQJr
3scjOMNRHv3N62biv6KN5Swi4SuU6nUCBTOaAiLgycmnktXea5sDGsKHb+haU/G6
WTAA9bGT7Ja0SWbboacdXBovsoqiva16O9miPPzca1hoT3HXLIpI0jI1fgsxu7uZ
rcnihNg4v4Og46McjYSVZl0+u7c/RQPXxsZHGB8HnLSMS8j0cS8=
=Lmvo
-----END PGP SIGNATURE-----

--HghOGTkopIgjDG0O--