On Mon, Mar 22, 2021 at 2:10 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
> > I think this is significantly cleaner than what we have now, and I
> > also prefer it to your proposal.
>
> +1 in general. However, I suspect that you did not try to compile
> this without --with-lz4, because if you had you'd have noticed the
> other uses of NO_LZ4_SUPPORT() that you broke. I think you need
> to leave that macro where it is.
You're correct that I hadn't tried this without --with-lz4, but I did
grep for other uses of NO_LZ4_SUPPORT() and found none. I also just
tried it without --with-lz4 just now, and it worked fine.
> Also, it's not nice for GUC check
> functions to throw ereport(ERROR); we prefer the caller to be able
> to decide if it's a hard error or not. That usage should be using
> GUC_check_errdetail() or a cousin, so it can't share the macro anyway.
I agree that these are valid points about GUC check functions in
general, but the patch I sent adds 0 GUC check functions and removes
1, and it didn't do the stuff you describe here anyway.
Are you sure you're looking at the patch I sent,
toast-compression-guc-rmh.patch? I can't help wondering if you applied
it to a dirty source tree or got the wrong file or something, because
otherwise I don't understand why you're seeing things that I'm not
seeing.
--
Robert Haas
EDB: http://www.enterprisedb.com
