From: Michael Paquier <michael@paquier.xyz>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: gkokolatos@pm.me, shiy.fnst@fujitsu.com, pgsql-hackers@lists.postgresql.org, Rachel Heaton <rachelmheaton@gmail.com>
Subject: Re: Add LZ4 compression in pg_dump
Date: Thu, 13 Apr 2023 07:23:48 +0900
Message-ID: <ZDcvdPVVi22mXPfR@paquier.xyz> (raw)
In-Reply-To: <ZDYHP4SvY2eejTj/@telsasoft.com>
References: <qRJGr1JjK3B_pbOxyGqI0iGJWEcRxnq9c7_mIqTY1CIIvYMY0hamPIrvjuS-6gyjr_ItOG8SLRacF1oj7d8R4QKHfZp85YTHI9B-U6LSLAQ=@pm.me>
<qSzzmYsXqUf1gDvFspOEbqoE8U_F8IsphDG8c25VZ5KTXimQVYfLvnCcuU7_X2raa2SzZinDF0NpDIKSOALRUYP8pu_NL5QJwIDdR_RT-4U=@pm.me>
<8ffd14fe-0278-7f8d-78d4-472816d06268@enterprisedb.com>
<ZDX+J72FYmJhH+fC@telsasoft.com>
<ZDYEPDbFAua+4hlr@paquier.xyz>
<ZDYHP4SvY2eejTj/@telsasoft.com>
On Tue, Apr 11, 2023 at 08:19:59PM -0500, Justin Pryzby wrote:
> On Wed, Apr 12, 2023 at 10:07:08AM +0900, Michael Paquier wrote:
>> Yes, this comment gives no value as it stands. I would be tempted to
>> follow the suggestion to group the whole code block in a single ifdef,
>> including the check, and remove this comment. Like the attached
>> perhaps?
>
> +1
Let me try this one again, as the previous patch would cause a warning
under --without:-zlib as user_compression_defined would be unused. We
could do something like the attached instead. It means doing twice
parse_compress_specification() for the non-zlib path, still we are
already doing so for the zlib path.
If there are other ideas, feel free.
--
Michael
Attachments:
[text/x-diff] pgdump-comment-2.patch (1005B, ../ZDcvdPVVi22mXPfR@paquier.xyz/2-pgdump-comment-2.patch)
download | inline diff:diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.cindex 967ced4eed..596f64ed2c 100644--- a/src/bin/pg_dump/pg_dump.c+++ b/src/bin/pg_dump/pg_dump.c@@ -751,17 +751,21 @@ main(int argc, char **argv)
/*
* Custom and directory formats are compressed by default with gzip when
- * available, not the others.+ * available if no compression specification has been specified, not the+ * others.
*/
- if ((archiveFormat == archCustom || archiveFormat == archDirectory) &&- !user_compression_defined)+ if (!user_compression_defined)
{
+ if (archiveFormat == archCustom || archiveFormat == archDirectory)+ {
#ifdef HAVE_LIBZ
- parse_compress_specification(PG_COMPRESSION_GZIP, NULL,- &compression_spec);+ parse_compress_specification(PG_COMPRESSION_GZIP, NULL,+ &compression_spec);
#else
- /* Nothing to do in the default case */+ parse_compress_specification(PG_COMPRESSION_NONE, NULL,+ &compression_spec);
#endif
+ }
}
/*
[application/pgp-signature] signature.asc (832B, ../ZDcvdPVVi22mXPfR@paquier.xyz/3-signature.asc)
download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: michael@paquier.xyz, pryzby@telsasoft.com, tomas.vondra@enterprisedb.com, rachelmheaton@gmail.com
Subject: Re: Add LZ4 compression in pg_dump
In-Reply-To: <ZDcvdPVVi22mXPfR@paquier.xyz>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox