Message-ID: <0a8a1003-42a3-b92e-00e5-6e04f2735efa@enterprisedb.com>
Date: Wed, 29 Mar 2023 00:02:00 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: Add LZ4 compression in pg_dump
Content-Language: en-US
To: gkokolatos@pm.me
Cc: Justin Pryzby <pryzby@telsasoft.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>
References: 
 <7zu6GYFi72jByVk2SEe_nd8r1DHhwA35vSntAE9sAfiMX9Lfk6bwimkwmu7GDgYaEvL27SE4qF1uK4_LHWQM4H-SXOwkMAI-GV8PKGmED_E=@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>
 <ZBjg4D/X05X+JbcJ@telsasoft.com>
 <3beb0dd7-c927-3eca-689f-716e255d5b27@enterprisedb.com>
 <JuDkpaGyAgfhoxfL71N4Bg1vGqzKW7bfardMjsfLjcqyQPqf-V-AW13jSDtCLQRYEzasmzwRJQED1dkx6_6uY-ocGJS54rNpPqM4iC36pSw=@pm.me>
 <bFjRZQCZego2j9Lsvunr4XtBEKRmCdxATU486xblkV4_qyePJoXoK1uItH48NzgeHki-w-3IlXPVNre4qDF_h6NRbf2VMhhP4v6FFWMK_-g=@pm.me>
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
In-Reply-To: 
 <bFjRZQCZego2j9Lsvunr4XtBEKRmCdxATU486xblkV4_qyePJoXoK1uItH48NzgeHki-w-3IlXPVNre4qDF_h6NRbf2VMhhP4v6FFWMK_-g=@pm.me>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/0a8a1003-42a3-b92e-00e5-6e04f2735efa%40enterprisedb.com>
Precedence: bulk

On 3/28/23 18:07, gkokolatos@pm.me wrote:
> 
> ------- Original Message -------
> On Friday, March 24th, 2023 at 10:30 AM, gkokolatos@pm.me <gkokolatos@pm.me> wrote:
> 
>>
>> ------- Original Message -------
>> On Thursday, March 23rd, 2023 at 6:10 PM, Tomas Vondra tomas.vondra@enterprisedb.com wrote:
>>
>>> This leaves the empty-data issue (which we have a fix for) and the
>>> switch to LZ4F. And then the zstd part.
>>
>> Please expect promptly a patch for the switch to frames.
> 
> Please find the expected patch attached. Note that the bulk of the
> patch is code unification, variable renaming to something more
> appropriate, and comment addition. These are changes that are not
> strictly necessary to switch to LZ4F. I do believe that are
> essential for code hygiene after the switch and they do belong
> on the same commit. 
> 


I think the patch is fine, but I'm wondering if the renames shouldn't go
a bit further. It removes references to LZ4File struct, but there's a
bunch of functions with LZ4File_ prefix. Why not to simply use LZ4_
prefix? We don't have GzipFile either.

Sure, it might be a bit confusing because lz4.h uses LZ4_ prefix, but
then we probably should not define LZ4_compressor_init ...

Also, maybe the comments shouldn't use "File API" when compress_io.c
calls that "Compressed stream API".


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company