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
