Re: adding 'zstd' as a compression algorithm

On Thu, Feb 17, 2022 at 8:36 PM Andres Freund <andres(at)anarazel(dot)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.

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