Message-ID: <c75cf5ba-6cf8-a7eb-c9eb-87f7a56493c0@enterprisedb.com>
Date: Wed, 1 Mar 2023 17:20:05 +0100
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: Justin Pryzby <pryzby@telsasoft.com>, gkokolatos@pm.me
Cc: Michael Paquier <michael@paquier.xyz>, shiy.fnst@fujitsu.com,
 pgsql-hackers@lists.postgresql.org, Rachel Heaton <rachelmheaton@gmail.com>
References: 
 <OSZPR01MB631087FC3B53AD8B1E3439F7FDA39@OSZPR01MB6310.jpnprd01.prod.outlook.com>
 <SnTBJOQtJQf_mnXh6icTznqC05gnGqXKWDwjEA15uIx3zf0cJBwY1aK7ULztKoqlXrGM39lHtEpLGmS48A7585FXo1sO6W2YR5RtiDoWpDU=@pm.me>
 <535565cd-b69f-c064-a845-15c0fc44db4a@enterprisedb.com>
 <mYBKRYonRy5OiDDYxcxOOJ5xM9iLnuDsGP92zC7mjw8ew1wiccYZIpTvQEmtgRoAXRhaj91SAKwtFluUtRDZ3r6pRdGYLncWK7rXIQrHhFY=@pm.me>
 <22cb80ed-ec87-54c7-fd5b-cdcf79766bb1@enterprisedb.com>
 <8421f53d-c9f7-c02e-3278-9f9522a8e8f3@enterprisedb.com>
 <20230224015115.GS1653@telsasoft.com> <Y/g+gi/zQozPfmly@paquier.xyz>
 <ikuHRdKZjpeTCe1_5_z3wajRHSjmS0A7A9yrsQQKcqJdEjYYzXsCK-ZBNtC-x-0P5CYEMG6hLaggIxcLA1scy05qgTquWYOYNcqYzcOz13g=@pm.me>
 <20230225050214.GH1653@telsasoft.com> <20230225140553.GI1653@telsasoft.com>
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
In-Reply-To: <20230225140553.GI1653@telsasoft.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/c75cf5ba-6cf8-a7eb-c9eb-87f7a56493c0%40enterprisedb.com>
Precedence: bulk


On 2/25/23 15:05, Justin Pryzby wrote:
> On Fri, Feb 24, 2023 at 11:02:14PM -0600, Justin Pryzby wrote:
>> I have some fixes (attached) and questions while polishing the patch for
>> zstd compression.  The fixes are small and could be integrated with the
>> patch for zstd, but could be applied independently.
> 
> One more - WriteDataToArchiveGzip() says:
> 
> +       if (cs->compression_spec.level == 0)
> +           pg_fatal("requested to compress the archive yet no level was specified");
> 
> That was added at e9960732a.  
> 
> But if you specify gzip:0, the compression level is already enforced by
> validate_compress_specification(), before hitting gzip.c:
> 
> | pg_dump: error: invalid compression specification: compression algorithm "gzip" expects a compression level between 1 and 9 (default at -1)
> 
> 5e73a6048 intended that to work as before, and you *can* specify -Z0:
> 
>     The change is backward-compatible, hence specifying only an integer
>     leads to no compression for a level of 0 and gzip compression when the
>     level is greater than 0.
> 
>     $ time ./src/bin/pg_dump/pg_dump -h /tmp regression -t int8_tbl -Fp --compress 0 |file -
>     /dev/stdin: ASCII text
> 

FWIW I agree we should make this backwards-compatible - accept "0" and
treat it as no compression.

Georgios, can you prepare a patch doing that?


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