On Mon, Mar 22, 2021 at 12:16 PM Robert Haas <robertmh...@gmail.com> wrote: > But, what about giving the default_toast_compression_method GUC an > assign hook that sets a global variable of type "char" to the > appropriate value? Then GetDefaultToastCompression() goes away > entirely. That might be worth exploring.
Actually, we can do even better. We should just make the values actually assigned to the GUC be TOAST_PGLZ_COMPRESSION etc. rather than TOAST_PGLZ_COMPRESSION_ID etc. Then a whole lot of complexity just goes away. I added some comments explaining why using TOAST_PGLZ_COMPRESSION is the wrong thing anyway. Then I got hacking and rearranged a few other things. So the attached patch does these thing: - Changes default_toast_compression to an enum, as in your patch, but now with values that are the same as what ultimately gets stored in attcompression. - Adds a comment warning against incautious use of TOAST_PGLZ_COMPRESSION_ID, etc. - Moves default_toast_compression_options to guc.c. - After doing the above two things, we can remove the #include of utils/guc.h into access/toast_compression.h, so the patch does that. - Moves NO_LZ4_SUPPORT, GetCompressionMethodName, and CompressionNameToMethod to guc.c. Making these inline functions doesn't save anything meaningful; it's more important not to export a bunch of random identifiers. - Removes an unnecessary cast to bool from the definition of CompressionMethodIsValid. I think this is significantly cleaner than what we have now, and I also prefer it to your proposal. Comments? -- Robert Haas EDB: http://www.enterprisedb.com
toast-compression-guc-rmh.patch
Description: Binary data
