On Mon, Mar 22, 2021 at 03:47:58PM -0400, Robert Haas wrote: > 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.
I'm guessing Tom read this hunk as being changes to check_default_toast_compression() rather than removing the function ? - * Validate a new value for the default_toast_compression GUC. + * CompressionNameToMethod - Get compression method from compression name + * + * Search in the available built-in methods. If the compression not found + * in the built-in methods then return InvalidCompressionMethod. */ -bool -check_default_toast_compression(char **newval, void **extra, GucSource source) +char +CompressionNameToMethod(const char *compression) { - if (**newval == '\0') + if (strcmp(compression, "pglz") == 0) + return TOAST_PGLZ_COMPRESSION; + else if (strcmp(compression, "lz4") == 0) { - GUC_check_errdetail("%s cannot be empty.", - "default_toast_compression"); - return false; +#ifndef USE_LZ4 + NO_LZ4_SUPPORT(); +#endif + return TOAST_LZ4_COMPRESSION; -- Justin