Hi, > I made a pass over these patches today and made a bunch of minor > corrections. New version attached. The two biggest things I changed > are (1) s/gzip_extractor/gzip_compressor/, because I feel like you > extract an archive like a tarfile, but that is not what is happening > here, this is not an archive and (2) I took a few bits of out of the > test case that didn't seem to be necessary. There wasn't any reason > that I could see why testing for PG_VERSION needed to be skipped when > the compression method is 'none', so my first thought was to just take > out the 'if' statement around that, but then after more thought that > test and the one for pg_verifybackup are certainly going to fail if > those files are not present, so why have an extra test? It might make > sense if we were only conditionally able to run pg_verifybackup and > wanted to have some test coverage even when we can't, but that's not > the case here, so I see no point.
Thanks. This makes sense. +#ifdef HAVE_LIBZ + /* + * If the user has requested a server compressed archive along with archive + * extraction at client then we need to decompress it. + */ + if (format == 'p' && compressmethod == COMPRESSION_GZIP && + compressloc == COMPRESS_LOCATION_SERVER) + streamer = bbstreamer_gzip_decompressor_new(streamer); +#endif I think it is not required to have HAVE_LIBZ check in pg_basebackup.c while creating a new gzip writer/decompressor. This check is already in place in bbstreamer_gzip_writer_new() and bbstreamer_gzip_decompressor_new() and it throws an error in case the build does not have required library support. I have removed this check from pg_basebackup.c and updated a delta patch. The patch can be applied on v5 patch. Thanks, Dipesh
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index 46ab60d..1f81bbf 100644 --- a/src/bin/pg_basebackup/pg_basebackup.c +++ b/src/bin/pg_basebackup/pg_basebackup.c @@ -1199,7 +1199,6 @@ CreateBackupStreamer(char *archive_name, char *spclocation, compressloc != COMPRESS_LOCATION_CLIENT) streamer = bbstreamer_plain_writer_new(archive_filename, archive_file); -#ifdef HAVE_LIBZ else if (compressmethod == COMPRESSION_GZIP) { strlcat(archive_filename, ".gz", sizeof(archive_filename)); @@ -1207,7 +1206,6 @@ CreateBackupStreamer(char *archive_name, char *spclocation, archive_file, compresslevel); } -#endif else { Assert(false); /* not reachable */ @@ -1256,7 +1254,6 @@ CreateBackupStreamer(char *archive_name, char *spclocation, else if (expect_unterminated_tarfile) streamer = bbstreamer_tar_terminator_new(streamer); -#ifdef HAVE_LIBZ /* * If the user has requested a server compressed archive along with archive * extraction at client then we need to decompress it. @@ -1264,7 +1261,6 @@ CreateBackupStreamer(char *archive_name, char *spclocation, if (format == 'p' && compressmethod == COMPRESSION_GZIP && compressloc == COMPRESS_LOCATION_SERVER) streamer = bbstreamer_gzip_decompressor_new(streamer); -#endif /* Return the results. */ *manifest_inject_streamer_p = manifest_inject_streamer;