Re: [HACKERS] Custom compression methods

On Mon, Mar 22, 2021 at 03:47:58PM -0400, Robert Haas wrote:
> On Mon, Mar 22, 2021 at 2:10 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > Robert Haas <robertmhaas(at)gmail(dot)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