On Fri, Feb 24, 2023 at 11:02:14PM -0600, Justin Pryzby wrote:
> I have some fixes (attached) and questions while polishing the patch for
> zstd compression. The fixes are small and could be integrated with the
> patch for zstd, but could be applied independently.
One more - WriteDataToArchiveGzip() says:
+ if (cs->compression_spec.level == 0)
+ pg_fatal("requested to compress the archive yet no level was
specified");
That was added at e9960732a.
But if you specify gzip:0, the compression level is already enforced by
validate_compress_specification(), before hitting gzip.c:
| pg_dump: error: invalid compression specification: compression algorithm
"gzip" expects a compression level between 1 and 9 (default at -1)
5e73a6048 intended that to work as before, and you *can* specify -Z0:
The change is backward-compatible, hence specifying only an integer
leads to no compression for a level of 0 and gzip compression when the
level is greater than 0.
$ time ./src/bin/pg_dump/pg_dump -h /tmp regression -t int8_tbl -Fp
--compress 0 |file -
/dev/stdin: ASCII text
Right now, I think that pg_fatal in gzip.c is dead code - that was first
added in the patch version sent on 21 Dec 2022.
--
Justin
>From 07b446803ec89ccd0954583640f314fa7f77048f Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Fri, 24 Feb 2023 14:07:09 -0600
Subject: [PATCH] f!fixes for LZ4
---
doc/src/sgml/ref/pg_dump.sgml | 4 ++--
src/bin/pg_dump/compress_gzip.c | 7 -------
src/bin/pg_dump/compress_io.c | 2 +-
src/bin/pg_dump/compress_io.h | 4 ++--
src/bin/pg_dump/compress_lz4.c | 4 ++--
src/bin/pg_dump/pg_backup_archiver.c | 4 ++--
src/bin/pg_dump/pg_dump.c | 2 --
src/bin/pg_dump/t/002_pg_dump.pl | 6 +++---
8 files changed, 12 insertions(+), 21 deletions(-)
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 49d218905fb..6fbe49f7ede 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -331,7 +331,7 @@ PostgreSQL documentation
can read. A directory format archive can be manipulated with
standard Unix tools; for example, files in an uncompressed archive
can be compressed with the <application>gzip</application> or
- <application>lz4</application>tool.
+ <application>lz4</application> tools.
This format is compressed by default using <literal>gzip</literal>
and also supports parallel dumps.
</para>
@@ -655,7 +655,7 @@ PostgreSQL documentation
<para>
Specify the compression method and/or the compression level to use.
The compression method can be set to <literal>gzip</literal> or
- <literal>lz4</literal> or <literal>none</literal> for no compression.
+ <literal>lz4</literal>, or <literal>none</literal> for no compression.
A compression detail string can optionally be specified. If the
detail string is an integer, it specifies the compression level.
Otherwise, it should be a comma-separated list of items, each of the
diff --git a/src/bin/pg_dump/compress_gzip.c b/src/bin/pg_dump/compress_gzip.c
index 0af65afeb4e..af68fd27a12 100644
--- a/src/bin/pg_dump/compress_gzip.c
+++ b/src/bin/pg_dump/compress_gzip.c
@@ -123,13 +123,6 @@ WriteDataToArchiveGzip(ArchiveHandle *AH, CompressorState *cs,
gzipcs->outbuf = pg_malloc(ZLIB_OUT_SIZE + 1);
gzipcs->outsize = ZLIB_OUT_SIZE;
- /*
- * A level of zero simply copies the input one block at the time. This
- * is probably not what the user wanted when calling this interface.
- */
- if (cs->compression_spec.level == 0)
- pg_fatal("requested to compress the archive yet no level was specified");
-
if (deflateInit(zp, cs->compression_spec.level) != Z_OK)
pg_fatal("could not initialize compression library: %s", zp->msg);
diff --git a/src/bin/pg_dump/compress_io.c b/src/bin/pg_dump/compress_io.c
index ce06f1eac9c..9239dbb2755 100644
--- a/src/bin/pg_dump/compress_io.c
+++ b/src/bin/pg_dump/compress_io.c
@@ -83,7 +83,7 @@
* used by the caller in an error message.
*/
char *
-supports_compression(const pg_compress_specification compression_spec)
+pgdump_supports_compression(const pg_compress_specification compression_spec)
{
const pg_compress_algorithm algorithm = compression_spec.algorithm;
bool supported = false;
diff --git a/src/bin/pg_dump/compress_io.h b/src/bin/pg_dump/compress_io.h
index bbde2693915..46815fa2ebe 100644
--- a/src/bin/pg_dump/compress_io.h
+++ b/src/bin/pg_dump/compress_io.h
@@ -21,7 +21,7 @@
#define ZLIB_OUT_SIZE 4096
#define ZLIB_IN_SIZE 4096
-extern char *supports_compression(const pg_compress_specification compression_spec);
+extern char *pgdump_supports_compression(const pg_compress_specification compression_spec);
/*
* Prototype for callback function used in writeData()
@@ -172,7 +172,7 @@ struct CompressFileHandle
extern CompressFileHandle *InitCompressFileHandle(const pg_compress_specification compression_spec);
/*
- * Initialize a compress file stream. Deffer the compression algorithm
+ * Initialize a compress file stream. Infer the compression algorithm
* from 'path', either by examining its suffix or by appending the supported
* suffixes in 'path'.
*/
diff --git a/src/bin/pg_dump/compress_lz4.c b/src/bin/pg_dump/compress_lz4.c
index fe1014e6e77..63e794cdc68 100644
--- a/src/bin/pg_dump/compress_lz4.c
+++ b/src/bin/pg_dump/compress_lz4.c
@@ -161,8 +161,8 @@ typedef struct LZ4File
} LZ4File;
/*
- * LZ4 equivalent to feof() or gzeof(). The end of file is reached if there
- * is no decompressed output in the overflow buffer and the end of the file
+ * LZ4 equivalent to feof() or gzeof(). Return true iff there is no
+ * decompressed output in the overflow buffer and the end of the backing file
* is reached.
*/
static int
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 61ebb8fe85d..2063d6f239d 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -388,7 +388,7 @@ RestoreArchive(Archive *AHX)
{
if (te->hadDumper && (te->reqs & REQ_DATA) != 0)
{
- char *errmsg = supports_compression(AH->compression_spec);
+ char *errmsg = pgdump_supports_compression(AH->compression_spec);
if (errmsg)
pg_fatal("cannot restore from compressed archive (%s)",
errmsg);
@@ -3745,7 +3745,7 @@ ReadHead(ArchiveHandle *AH)
else
AH->compression_spec.algorithm = PG_COMPRESSION_GZIP;
- errmsg = supports_compression(AH->compression_spec);
+ errmsg = pgdump_supports_compression(AH->compression_spec);
if (errmsg)
{
pg_log_warning("archive is compressed, but this installation does not support compression (%s) -- no data will be available",
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 24ba936332d..ce2242195f3 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -733,8 +733,6 @@ main(int argc, char **argv)
#ifdef HAVE_LIBZ
parse_compress_specification(PG_COMPRESSION_GZIP, NULL,
&compression_spec);
-#else
- /* Nothing to do in the default case */
#endif
}
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 72b19ee6cde..ad7bc5c194b 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -4248,10 +4248,10 @@ foreach my $run (sort keys %pgdump_runs)
my $test_key = $run;
my $run_db = 'postgres';
- # Skip command-level tests for gzip if there is no support for it.
+ # Skip command-level tests for gzip/lz4 if they're not supported.
if ($pgdump_runs{$run}->{compile_option} &&
- ($pgdump_runs{$run}->{compile_option} eq 'gzip' && !$supports_gzip) ||
- ($pgdump_runs{$run}->{compile_option} eq 'lz4' && !$supports_lz4))
+ (($pgdump_runs{$run}->{compile_option} eq 'gzip' && !$supports_gzip) ||
+ ($pgdump_runs{$run}->{compile_option} eq 'lz4' && !$supports_lz4)))
{
note "$run: skipped due to no $pgdump_runs{$run}->{compile_option} support";
next;
--
2.34.1
