Date: Sat, 20 Mar 2021 10:39:34 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: Dilip Kumar <dilipbalaut@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Robert Haas <robertmhaas@gmail.com>,
	Alvaro Herrera <alvherre@alvh.no-ip.org>,
	Andres Freund <andres@anarazel.de>,
	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>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: [HACKERS] Custom compression methods
Message-ID: <20210320153933.GT11765@telsasoft.com>
References: <20210319222242.GA23930@alvherre.pgsql>
 <CA+TgmoYQ8hn1apJXbmxutYv-sHkTOUdPxdFJUzvZ7uT4cUT+cQ@mail.gmail.com>
 <CA+TgmobGm9umTgno4O4UqSAoGhOPOAA55uiKE8N_wehnHfg4Tw@mail.gmail.com>
 <4158590.1616185209@sss.pgh.pa.us>
 <20210320000625.GP11765@telsasoft.com>
 <20210320074420.GR11765@telsasoft.com>
 <CAFiTN-uX=hLscjL=0raA6dYcoiZB=9YqL3DmX15CMvZiu+WB+w@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CAFiTN-uX=hLscjL=0raA6dYcoiZB=9YqL3DmX15CMvZiu+WB+w@mail.gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20210320153933.GT11765%40telsasoft.com>
Precedence: bulk

On Sat, Mar 20, 2021 at 04:37:53PM +0530, Dilip Kumar wrote:
> - 0006 is fine but not sure what is the advantage over what we have today?

The advantage is that it's dozens of lines shorter, and automatically includes
a HINT.
 SET default_toast_compression = 'I do not exist compression';                                                                                                                                                                     
 ERROR:  invalid value for parameter "default_toast_compression": "I do not exist compression"                                                                                                                                     
-DETAIL:  Compression method "I do not exist compression" does not exist.                                                                                                                                                          
+HINT:  Available values: pglz, lz4.                                                                                                                                                                                               

If we use a GUC hook, I think it should be to special case lz4 to say:
"..must be enabled when PG was built".

> - And, 0008 and 0009, I think my
> 0001-Fixup-dump-toast-compression-method.patch[1] is doing this in a
> much simpler way, please have a look and let me know if you think that
> has any problems and we need to do the way you are doing here?

I tested and saw that your patch doesn't output "SET default_toast_compression"
in non-text dumps (pg_dump -Fc).  Also, I think the internal newline should be
removed:

ALTER TABLE public.t ALTER COLUMN b
SET COMPRESSION lz4;

-- 
Justin