Date: Mon, 20 Mar 2023 17:40:32 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: gkokolatos@pm.me
Cc: Tomas Vondra <tomas.vondra@enterprisedb.com>,
	Alexander Lakhin <exclusion@gmail.com>, shiy.fnst@fujitsu.com,
	Michael Paquier <michael@paquier.xyz>,
	pgsql-hackers@lists.postgresql.org,
	Rachel Heaton <rachelmheaton@gmail.com>
Subject: Re: Add LZ4 compression in pg_dump
Message-ID: <ZBjg4D/X05X+JbcJ@telsasoft.com>
References: 
 <7zu6GYFi72jByVk2SEe_nd8r1DHhwA35vSntAE9sAfiMX9Lfk6bwimkwmu7GDgYaEvL27SE4qF1uK4_LHWQM4H-SXOwkMAI-GV8PKGmED_E=@pm.me>
 <8421f53d-c9f7-c02e-3278-9f9522a8e8f3@enterprisedb.com>
 <33496f7c-3449-1426-d568-63f6bca2ac1f@gmail.com>
 <1aBKUlb8BGVUUTkRJwEmU3ud1NyeHVCoC7bpOKAuRYlHHJ7hRiDVXYgad-wmjx1K3pykd_ctyY4Lqy9xA7JGLXOm1Eul-imdZ_7TVr5LY8U=@pm.me>
 <3da84c90-7e12-75ee-a582-025bc68dd098@enterprisedb.com>
 <5qAu7vrMV3hEodlIC82b0JICWTwEUSRoIDWlIaczNBIEVVfDH2lccxHNIQ726e4nX_Qqjc4lPc5_z9RtV28NMdVv7BNiADF4JB-FDahmlVk=@pm.me>
 <d8315656-3b35-7d9a-c799-8d48fdccfe45@enterprisedb.com>
 <JduQzRIUMhF5NDvC_klfyHTDhzb5Py60xH9ICRUtluHQ4dD02nw-BH7WfX7DQG6gaFWl1Q919RvX3Xvm_5mau_0YKRQeUs6ezUSFDlFVPsE=@pm.me>
 <32ba9612-1982-4b2d-135a-5d1222987082@enterprisedb.com>
 <XPMIIorJ5munrlLRZGquartiN5tuapdjiBhaPZ6iEmJccRiKWCIHSowSLPhaGj4gLpqvaS1pQn7PGkA_WNwvuUTVdl-lrU_TzHA81LqujJ4=@pm.me>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <XPMIIorJ5munrlLRZGquartiN5tuapdjiBhaPZ6iEmJccRiKWCIHSowSLPhaGj4gLpqvaS1pQn7PGkA_WNwvuUTVdl-lrU_TzHA81LqujJ4=@pm.me>
Archived-At: 
 <https://www.postgresql.org/message-id/ZBjg4D/X05X%2BJbcJ%40telsasoft.com>
Precedence: bulk

On Fri, Mar 17, 2023 at 03:43:58PM +0000, gkokolatos@pm.me wrote:
> From a174cdff4ec8aad59f5bcc7e8d52218a14fe56fc Mon Sep 17 00:00:00 2001
> From: Georgios Kokolatos <gkokolatos@pm.me>
> Date: Fri, 17 Mar 2023 14:45:58 +0000
> Subject: [PATCH v3 1/3] Improve type handling in pg_dump's compress file API

> -int
> +bool
>  EndCompressFileHandle(CompressFileHandle *CFH)
>  {
> -	int			ret = 0;
> +	bool		ret = 0;

Should say "= false" ?

>  	/*
>  	 * Write 'size' bytes of data into the file from 'ptr'.
> +	 *
> +	 * Returns true on success and false on error.
> +	 */
> +       bool            (*write_func) (const void *ptr, size_t size,

> -        * Get a pointer to a string that describes an error that occurred during a
> -        * compress file handle operation.
> +        * Get a pointer to a string that describes an error that occurred during
> +        * a compress file handle operation.
>          */
>         const char *(*get_error_func) (CompressFileHandle *CFH);

This should mention that the error accessible in error_func() applies (only) to
write_func() ?

As long as this touches pg_backup_directory.c you could update the
header comment to refer to "compressed extensions", not just .gz.

I noticed that EndCompressorLZ4() tests "if (LZ4cs)", but that should
always be true.

I was able to convert the zstd patch to this new API with no issue.

-- 
Justin