Re: Add LZ4 compression in pg_dump

On 3/28/23 18:07, gkokolatos(at)pm(dot)me wrote:
>
> ------- Original Message -------
> On Friday, March 24th, 2023 at 10:30 AM, gkokolatos(at)pm(dot)me <gkokolatos(at)pm(dot)me> wrote:
>
>>
>> ------- Original Message -------
>> On Thursday, March 23rd, 2023 at 6:10 PM, Tomas Vondra tomas(dot)vondra(at)enterprisedb(dot)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