On Mon, Mar 22, 2021 at 2:10 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > Robert Haas <robertmh...@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
