Horton

Re: [HACKERS] Custom compression methods

Justin Pryzby <pryzby@telsasoft.com>

View thread View on postgr.es
From: Justin Pryzby <pryzby@telsasoft.com>
To: Dilip Kumar <dilipbalaut@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Tomas Vondra <tomas.vondra@2ndquadrant.com>, Alexander Korotkov <a.korotkov@postgrespro.ru>, David Steele <david@pgmasters.net>, Ildus Kurbangaliev <i.kurbangaliev@gmail.com>, Dmitry Dolgov <9erthalion6@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2021-03-09T07:16:55Z
Lists: pgsql-hackers 
On Mon, Mar 08, 2021 at 03:32:39PM +0530, Dilip Kumar wrote:
> On Sun, Mar 7, 2021 at 1:27 AM Justin Pryzby <pryzby@telsasoft.com> wrote:
> >
> > On Sat, Mar 06, 2021 at 08:59:16PM +0530, Dilip Kumar wrote:
> > > - Alter table set compression, will not rewrite the old data, so only
> > > the new tuple will be compressed with the new compression method.
> > > - No preserve.
> >
> > In this patch, SET default_toast_compression=lz4 "works" even if without-lz4,
> > but then CREATE TABLE fails.  You should either allow table creation (as
> > above), or check in check_default_toast_compression() if lz4 is enabled.
> > Its comment about "catalog access" is incorrect now.
> 
> As of now I have made GUC behavior similar to the CREATE TABLE, in
> both case it will throw an error if it is not compiled with lz4
> method.

In the latest patch, CREATE TABLE (t text COMPRESS lz4) fails if --without-lz4.
I think that's the right choice, since otherwise we should also allow ALTER SET
COMPRESSION lz4, which feels wrong.

This comment and associated conditional is still wrong, since there's no
catalog access anymore:

+        * If we aren't inside a transaction, or not connected to a database, we
+        * cannot do the catalog access necessary to verify the method.  Must
+        * accept the value on faith.

This shouldn't refer to "access method" (probably originally my error):

+                        * When source == PGC_S_TEST, don't throw a hard error for a
+                        * nonexistent table access method, only a NOTICE. See comments in
+                        * guc.h.

I'm not sure why that function is now in guc.c ?

Now I think this comment should just say /* GUC */
+/* Compile-time default */
+char   *default_toast_compression = DEFAULT_TOAST_COMPRESSION;

It sounds like after that, you should merge that part into 0001.

In 0001, configure.ac is missing this :
AC_MSG_RESULT([$with_lz4])

Also, Thomas updated the mac CI to installed LZ4.
However it fails to find the library.  Maybe configure.ac needs to use
pkg-config.  Or maybe the mac build needs to use this - we're not sure.
    --with-includes=/usr/local/opt --with-libraries=/usr/local/opt

-- 
Justin

Commits

  1. docs: Update TOAST storage docs for configurable compression.

  2. Further tweaking of pg_dump's handling of default_toast_compression.

  3. Fix interaction of TOAST compression with expression indexes.

  4. Tidy up more loose ends related to configurable TOAST compression.

  5. Short-circuit slice requests that are for more than the object's size.

  6. Mostly-cosmetic adjustments of TOAST-related macros.

  7. Remove useless configure probe for <lz4/lz4.h>.

  8. Error on invalid TOAST compression in CREATE or ALTER TABLE.

  9. docs: Fix omissions related to configurable TOAST compression.

  10. More code cleanup for configurable TOAST compression.

  11. Bring configure support for LZ4 up to snuff.

  12. Make compression.sql regression test independent of default.

  13. Use valid compression method in brin_form_tuple

  14. Fix up pg_dump's handling of per-attribute compression options.

  15. Allow configurable LZ4 TOAST compression.

  16. Fix inconsistencies in the code

  17. Mostly-cosmetic improvements in memory chunk header alignment coding.

  18. Allow numeric to use a more compact, 2-byte header in many cases.