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


Reply via email to