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;
