Feedback-ID: i0fe9450f:Fastmail
Date: Fri, 5 Dec 2025 11:23:31 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Euler Taveira <euler@eulerto.com>
Cc: Aleksander Alekseev <aleksander@tigerdata.com>,
	pgsql-hackers <pgsql-hackers@postgresql.org>,
	Peter Eisentraut <peter@eisentraut.org>
Subject: Re: change default default_toast_compression to lz4?
Message-ID: <aTJCI6Ej8I0iuZmD@paquier.xyz>
References: <435df33a-129e-4f0c-a803-f3935c5a5ecb@eisentraut.org>
 <aSaDfIrZIhZwAe42@paquier.xyz>
 <c822b84c-b104-4fb4-92f3-3008cce50bf9@app.fastmail.com>
 <CAJ7c6TOgh15gX1ZXVEHVOBahR30pVU=HLL-30u1Rt38uxDKVkQ@mail.gmail.com>
 <c269e7e1-2ef0-4fb2-8ecd-18ab2db13e3b@app.fastmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="goeXiaWaDqVmFeKm"
Content-Disposition: inline
In-Reply-To: <c269e7e1-2ef0-4fb2-8ecd-18ab2db13e3b@app.fastmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/aTJCI6Ej8I0iuZmD%40paquier.xyz>
Precedence: bulk


--goeXiaWaDqVmFeKm
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Thu, Dec 04, 2025 at 05:11:15PM -0300, Euler Taveira wrote:
> Here it is v2. It enforces the lz4 dependency. It works the same as other
> dependencies (readline, icu, zlib); error if the dependency could not be found.
> Regarding meson, I'm confused. If any of the referred dependencies (icu,
> readline, zlib) is not found, there is no hard error. Instead, the feature is
> disabled. I searched for a discussion about this decision but couldn't find.
> For this patch, I decided to use the same pattern (no error) but I added a
> warning message (similar to zlib).

Another thing to be careful of is that this would immediately break
the CI task CompilerWarnings for mingw:
[02:03:19.626] checking whether to build with LZ4 support... yes
[02:03:19.626] checking for liblz4... no
[02:03:19.694] configure: error: Package requirements (liblz4) were
not met:

So this had better be adjusted in one go, in the shape of a tweak in
mingw_cross_warning_script with the addition of a --without-lz4, same
way as for ICU.

> Should it report an error (like autoconf) instead of silently disable the
> feature that is a requirement? If yes, then we should add error messages for
> these 3 dependencies.

Hmm.  It seems to me that we should just set not_found_dep if lz4
cannot be found, leaving meson_options.txt as it is currently, no?
One could always force meson's hand with -Dlz4=disabled.  As a whole,
I'd value more consistency with how icu is handled if we want to force
LZ4 across the board in the backend.  The case with zlib is different
in the backend: we only use it on a wanted-basis for compression
specifications in server-side compressed base backups, so it seems to
me that there's a case for being a tad more aggressive with LZ4 as it
relates to TOAST compression, forcing an error rather than ignoring it
silently if it cannot be found.
--
Michael

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmkyQiMACgkQnvQgOdby
QH2Q1A//SxyUwTvRPpkA/1yI8cMC7WrAQ8JrZrYxj+QWcHgZ/c3CJlPek6PBPfJM
1wJJELy1/ab/oEVgU1iUAOxUXOc0KDDbzBc19pvn4dC1oHtairH1NUj2EKQl1l8b
JLIhSqqZFjDacc2GziP2Ib5ZNQqyPRUPLZvDFa2XAd6t1deLzvDKBMgIcV/PtsVu
Ord5GEJylKXYBbDvb80jBxjs9WBv7lBtT1YaNmF0ohg2fCj11R7uerneZSg2r4pP
rnwleUeFkUa17JuGE5w87QwbrAY9wiONI7+eZVNMYpdcO602KJ0ZzYocRY8Q9KzK
pJerWDbJeGDmx4kbWZ2Gp7wUZ0qyXOzYnuGo4hdijOAqbH3mPS6qC4k6H2VrcJoK
0AgTJMslOzkxe+k8dCluMnaqbFnPJBAHpsqINaeFXd9MIPr/PO4EaAlwcmNbirZf
LVZX4HRHH2G5tRbTz0HJnJSVcJ548ElrzQcXTMPWAocpzu1P3PgC3UaJgcNfzo19
P4gaxw6L8Alkq3R+xf+d18TUzZiCThVjkOiKrNmjR4eGrHIfhs6/hKK4XLNzR+bX
/UB+SsNaVDPxeVBO1cRSHX4zZ5iByQRyLL0sBSFGmdPeekFcRiLYb97/l7/GeuNF
bpjSc1o58rl9eA0eDD4VT6RL301JEoxb3bmVsT+li8klGCRFhSI=
=HN/0
-----END PGP SIGNATURE-----

--goeXiaWaDqVmFeKm--