Feedback-ID: i0fe9450f:Fastmail
Date: Thu, 26 Jan 2023 20:53:28 +0900
From: Michael Paquier <michael@paquier.xyz>
To: gkokolatos@pm.me
Cc: Justin Pryzby <pryzby@telsasoft.com>,
	Tomas Vondra <tomas.vondra@enterprisedb.com>,
	pgsql-hackers@lists.postgresql.org,
	Rachel Heaton <rachelmheaton@gmail.com>
Subject: Re: Add LZ4 compression in pg_dump
Message-ID: <Y9JpuLGng5zLO4Mx@paquier.xyz>
References: <7zu6GYFi72jByVk2SEe_nd8r1DHhwA35vSntAE9sAfiMX9Lfk6bwimkwmu7GDgYaEvL27SE4qF1uK4_LHWQM4H-SXOwkMAI-GV8PKGmED_E=@pm.me>
 <MMLKPM28EzV5-NC4Jd3YdWCmPlUvbpbyqrugtklWTHZwF24g5x2IJ1J1wTICSESs8CaIRdj-56eDIqC4cTpahm55PmTVRik2D77X0ZKGeVo=@pm.me>
 <20230123180014.GX13860@telsasoft.com>
 <YiPrsNILgul-hbeyD6tfNaRUK5JZEhaZE36dLe1YJvKN_i0Tm2GuHd246MqcfNGFsjSY-rNo9n54MFCrRiRHRDkYyjgE4e29V-diAN2EcNU=@pm.me>
 <20230125014203.GL13860@telsasoft.com>
 <-myGeDiZ-ePDRyi18026Iu2HTTfmwfjWaJn4OWJ2F3oglZUfJTOQ7FARPW7gYxE4vK3b0HpwYoiHR45nfOWReRvvJZfCBfJzPP5iJyn4muk=@pm.me>
 <20230125180020.GF22427@telsasoft.com>
 <Y9IUZzQsusY3VuyJ@paquier.xyz>
 <20230126062846.GO22427@telsasoft.com>
 <6YZqN2HEE4yJxxxXWiMl4QWwKzLon0SdKETTGVkT9zjWnXMftGZjLKHHjGFuwC_5I6xL2s8nMXdLQGA4vEFveYXSHbfD9K9pDQvEJF2y99o=@pm.me>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="X3+jOHMHubTMLmRx"
Content-Disposition: inline
In-Reply-To: <6YZqN2HEE4yJxxxXWiMl4QWwKzLon0SdKETTGVkT9zjWnXMftGZjLKHHjGFuwC_5I6xL2s8nMXdLQGA4vEFveYXSHbfD9K9pDQvEJF2y99o=@pm.me>
Archived-At: <https://www.postgresql.org/message-id/Y9JpuLGng5zLO4Mx%40paquier.xyz>
Precedence: bulk


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

On Thu, Jan 26, 2023 at 11:24:47AM +0000, gkokolatos@pm.me wrote:
> I gave this a little bit of thought. I think that ReadHead should not
> emit a warning, or at least not this warning as it is slightly misleading.
> It implies that it will automatically turn off data restoration, which is
> false. Further ahead, the code will fail with a conflicting error message
> stating that the compression is not available.
>=20
> Instead, it would be cleaner both for the user and the maintainer to
> move the check in RestoreArchive and make it the sole responsible for
> this logic.

-    pg_fatal("cannot restore from compressed archive (compression not supp=
orted in this installation)");
+    pg_fatal("cannot restore data from compressed archive (compression not=
 supported in this installation)");
Hmm.  I don't mind changing this part as you suggest.

-#ifndef HAVE_LIBZ
-       if (AH->compression_spec.algorithm =3D=3D PG_COMPRESSION_GZIP)
-               pg_fatal("archive is compressed, but this installation does=
 not support compression");
-#endif
However I think that we'd better keep the warning, as it can offer a
hint when using pg_restore -l not built with compression support if
looking at a dump that has been compressed.
--
Michael

--X3+jOHMHubTMLmRx
Content-Type: application/pgp-signature; name="signature.asc"

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmPSabgACgkQnvQgOdby
QH2y2g//bQ8Qnd47G28Obe7muFz3PzWPkVlI6Jl8zIUCgqn61xmGsVJzA8eYccgw
A62jYxcNRyh1ufNqxFGi6WK0cxYLk1GVSSmXeEM4e835kDrTS8dyPB3NToSXdfMp
FSrWvYnTKvRtdpJoSe9hqHYRmiPtF0TeYKLgLuK+5+fAEXmpTXlKUyM9TErxh7q/
DOZsjycFQk3/SHsLNvFXNDexiFwutKERzsToIie84COLJWzAv8DcZZKmhMVfjfhh
3j/J4LKL5h/t+XqwlxJq2TDg1AFtwmp2BRpXK0Ajps6+D1qSdg3649zN4GNUzIH2
YQH+eFVgqpn938nUcBeqdh81qVapRI07b0CO/C7BI//JlcfEuhcAdGhBGuwxjyeO
BbBcDekxrxXwSl9IM+0MhPPNHf1j4qWXCVw4GquzL6Fo0XgHSQgARB6jevO7Mpfi
Lj3xvEeLdHQCdFiPUDjjnbZC3TyX9JQQO4c6uMru1vQiXfypP/govYNOelITdOkw
blXxPZq2wE+jmoWGlg81OC5omRXtoSsywM6XV/6kSYcL5+YQl7m9dk/Tartj04zy
8goHju+DSFhgjdcrbyeNWq9tUW/DC5R/ETu24TZY3xqbb9ba3stgek1/p8HOORa8
5l0B75hRYlfNsNaLuKOAolZhsZi1gbADtjjm9fr8f5I0fr3DbrI=
=K02T
-----END PGP SIGNATURE-----

--X3+jOHMHubTMLmRx--