Re: adding 'zstd' as a compression algorithm

On Fri, Feb 18, 2022 at 8:48 AM Robert Haas <robertmhaas@gmail.com> wrote:
> On Thu, Feb 17, 2022 at 8:36 PM Andres Freund <andres@anarazel.de> wrote:
> > On 2022-02-17 13:34:08 +0900, Michael Paquier wrote:
> > > %define needs to include HAVE_LIBZSTD, HAVE_ZSTD_H and USE_ZSTD, so
> > > this version fails the sanity check between pg_config.h.in and the
> > > MSVC scripts checking that all flags exist.
> >
> > Do we really need all three defines? How about using AC_CHECK_HEADER() instead
> > of AC_CHECK_HEADERS()? That wouldn't define HAVE_ZSTD_H. Cases where we error
> > out if a header isn't found make it a bit pointless to then still define
> > HAVE_*_H.  Plenty other cases in configure.ac just use AC_CHECK_HEADER.
>
> I have to admit to being somewhat confused by the apparent lack of
> consistency in the way we do configure checks. The ZSTD check we added
> here is just based on the LZ4 check just above it, which was the
> result of my commit of Dilip's patch to add LZ4 TOAST compression. So
> if we want to do something different we should change them both. But
> that just begs the question of why the LZ4 support looks the way it
> does, and to be honest I don't recall. The zlib and XSLT formulas just
> above are much simpler, but for some reason what we're doing here
> seems to be based on the more-complex formula we use for XML support
> instead of either of those.
>
> But having said that, the proposed patch adds no new call to
> AC_CHECK_HEADER(), and does add a new call to AC_CHECK_HEADERS(), so I
> don't understand the specifics of your complaint here.

Oh wait ... you want it the other way. Yeah, that seems harmless to
change. I wonder how many others there are that could be changed
similarly...

-- 
Robert Haas
EDB: http://www.enterprisedb.com