On Wed, Apr 12, 2023 at 07:53:53PM -0500, Justin Pryzby wrote: > I doubt it - in the !HAVE_LIBZ case, it's currently an "if" statement > with nothing but a comment, which isn't a problem. > > I think the only issue with an empty "if" is when you have no braces, > like: > > if (...) > #if ... > something; > #endif > > // problem here //
(My apologies for the late reply.) Still it could be easily messed up, and that's not a style that really exists in the tree, either, because there are always #else blocks set up in such cases. Another part that makes me a bit uncomfortable is that we would still call twice parse_compress_specification(), something that should not happen but we are doing so on HEAD because the default compression_algorithm_str is "none" and we want to enforce "gzip" for custom and directory formats when building with zlib. What about just moving this block a bit up, just before the compression spec parsing, then? If we set compression_algorithm_str, the specification is compiled with the expected default, once instead of twice. -- Michael
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 058244cd17..e4f32d170f 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -721,6 +721,21 @@ main(int argc, char **argv) if (archiveFormat == archNull) plainText = 1; + /* + * Custom and directory formats are compressed by default with gzip when + * available, not the others. If gzip is not available, no compression + * is done by default. + */ + if ((archiveFormat == archCustom || archiveFormat == archDirectory) && + !user_compression_defined) + { +#ifdef HAVE_LIBZ + compression_algorithm_str = "gzip"; +#else + compression_algorithm_str = "none"; +#endif + } + /* * Compression options */ @@ -749,21 +764,6 @@ main(int argc, char **argv) pg_log_warning("compression option \"%s\" is not currently supported by pg_dump", "workers"); - /* - * Custom and directory formats are compressed by default with gzip when - * available, not the others. - */ - if ((archiveFormat == archCustom || archiveFormat == archDirectory) && - !user_compression_defined) - { -#ifdef HAVE_LIBZ - parse_compress_specification(PG_COMPRESSION_GZIP, NULL, - &compression_spec); -#else - /* Nothing to do in the default case */ -#endif - } - /* * If emitting an archive format, we always want to emit a DATABASE item, * in case --create is specified at pg_restore time.
signature.asc
Description: PGP signature