diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml index c549c0858a..946d563491 100644 --- a/doc/src/sgml/protocol.sgml +++ b/doc/src/sgml/protocol.sgml @@ -2752,9 +2752,13 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;" The level keyword sets the compression level. For gzip the compression level should be an - integer between 1 and 9, for lz4 an integer - between 1 and 12, and for zstd an integer - between 1 and 22. + integer between 1 and 9 + (default Z_DEFAULT_COMPRESSION or + -1), for lz4 an integer + between 1 and 12 (default 0 for fast compression + mode), and for zstd an integer between + 1 and 22 (default + ZSTD_CLEVEL_DEFAULT or 3). diff --git a/src/backend/backup/basebackup_gzip.c b/src/backend/backup/basebackup_gzip.c index a965866ff2..1ec42791f1 100644 --- a/src/backend/backup/basebackup_gzip.c +++ b/src/backend/backup/basebackup_gzip.c @@ -72,13 +72,9 @@ bbsink_gzip_new(bbsink *next, pg_compress_specification *compress) Assert(next != NULL); - if ((compress->options & PG_COMPRESSION_OPTION_LEVEL) == 0) - compresslevel = Z_DEFAULT_COMPRESSION; - else - { - compresslevel = compress->level; - Assert(compresslevel >= 1 && compresslevel <= 9); - } + compresslevel = compress->level; + Assert((compresslevel >= 1 && compresslevel <= 9) || + compresslevel == Z_DEFAULT_COMPRESSION); sink = palloc0(sizeof(bbsink_gzip)); *((const bbsink_ops **) &sink->base.bbs_ops) = &bbsink_gzip_ops; diff --git a/src/backend/backup/basebackup_lz4.c b/src/backend/backup/basebackup_lz4.c index d919e3dec7..986272ab9a 100644 --- a/src/backend/backup/basebackup_lz4.c +++ b/src/backend/backup/basebackup_lz4.c @@ -72,13 +72,8 @@ bbsink_lz4_new(bbsink *next, pg_compress_specification *compress) Assert(next != NULL); - if ((compress->options & PG_COMPRESSION_OPTION_LEVEL) == 0) - compresslevel = 0; - else - { - compresslevel = compress->level; - Assert(compresslevel >= 1 && compresslevel <= 12); - } + compresslevel = compress->level; + Assert(compresslevel >= 0 && compresslevel <= 12); sink = palloc0(sizeof(bbsink_lz4)); *((const bbsink_ops **) &sink->base.bbs_ops) = &bbsink_lz4_ops; diff --git a/src/backend/backup/basebackup_zstd.c b/src/backend/backup/basebackup_zstd.c index 865067f8dc..84256e3fa2 100644 --- a/src/backend/backup/basebackup_zstd.c +++ b/src/backend/backup/basebackup_zstd.c @@ -96,14 +96,11 @@ bbsink_zstd_begin_backup(bbsink *sink) if (!mysink->cctx) elog(ERROR, "could not create zstd compression context"); - if ((compress->options & PG_COMPRESSION_OPTION_LEVEL) != 0) - { - ret = ZSTD_CCtx_setParameter(mysink->cctx, ZSTD_c_compressionLevel, - compress->level); - if (ZSTD_isError(ret)) - elog(ERROR, "could not set zstd compression level to %d: %s", - compress->level, ZSTD_getErrorName(ret)); - } + ret = ZSTD_CCtx_setParameter(mysink->cctx, ZSTD_c_compressionLevel, + compress->level); + if (ZSTD_isError(ret)) + elog(ERROR, "could not set zstd compression level to %d: %s", + compress->level, ZSTD_getErrorName(ret)); if ((compress->options & PG_COMPRESSION_OPTION_WORKERS) != 0) { diff --git a/src/bin/pg_basebackup/bbstreamer_gzip.c b/src/bin/pg_basebackup/bbstreamer_gzip.c index e7261910d8..c3455ffbdd 100644 --- a/src/bin/pg_basebackup/bbstreamer_gzip.c +++ b/src/bin/pg_basebackup/bbstreamer_gzip.c @@ -107,9 +107,7 @@ bbstreamer_gzip_writer_new(char *pathname, FILE *file, pg_fatal("could not open output file: %m"); } - if ((compress->options & PG_COMPRESSION_OPTION_LEVEL) != 0 && - gzsetparams(streamer->gzfile, compress->level, - Z_DEFAULT_STRATEGY) != Z_OK) + if (gzsetparams(streamer->gzfile, compress->level, Z_DEFAULT_STRATEGY) != Z_OK) pg_fatal("could not set compression level %d: %s", compress->level, get_gz_error(streamer->gzfile)); diff --git a/src/bin/pg_basebackup/bbstreamer_lz4.c b/src/bin/pg_basebackup/bbstreamer_lz4.c index b9752354c9..ed2aa01305 100644 --- a/src/bin/pg_basebackup/bbstreamer_lz4.c +++ b/src/bin/pg_basebackup/bbstreamer_lz4.c @@ -88,8 +88,7 @@ bbstreamer_lz4_compressor_new(bbstreamer *next, pg_compress_specification *compr prefs = &streamer->prefs; memset(prefs, 0, sizeof(LZ4F_preferences_t)); prefs->frameInfo.blockSizeID = LZ4F_max256KB; - if ((compress->options & PG_COMPRESSION_OPTION_LEVEL) != 0) - prefs->compressionLevel = compress->level; + prefs->compressionLevel = compress->level; ctxError = LZ4F_createCompressionContext(&streamer->cctx, LZ4F_VERSION); if (LZ4F_isError(ctxError)) diff --git a/src/bin/pg_basebackup/bbstreamer_zstd.c b/src/bin/pg_basebackup/bbstreamer_zstd.c index 8a839145a6..1207dd771a 100644 --- a/src/bin/pg_basebackup/bbstreamer_zstd.c +++ b/src/bin/pg_basebackup/bbstreamer_zstd.c @@ -84,15 +84,12 @@ bbstreamer_zstd_compressor_new(bbstreamer *next, pg_compress_specification *comp if (!streamer->cctx) pg_fatal("could not create zstd compression context"); - /* Set compression level, if specified */ - if ((compress->options & PG_COMPRESSION_OPTION_LEVEL) != 0) - { - ret = ZSTD_CCtx_setParameter(streamer->cctx, ZSTD_c_compressionLevel, - compress->level); - if (ZSTD_isError(ret)) - pg_fatal("could not set zstd compression level to %d: %s", - compress->level, ZSTD_getErrorName(ret)); - } + /* Set compression level */ + ret = ZSTD_CCtx_setParameter(streamer->cctx, ZSTD_c_compressionLevel, + compress->level); + if (ZSTD_isError(ret)) + pg_fatal("could not set zstd compression level to %d: %s", + compress->level, ZSTD_getErrorName(ret)); /* Set # of workers, if specified */ if ((compress->options & PG_COMPRESSION_OPTION_WORKERS) != 0) diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index ae73b4bc92..8c3eba0a61 100644 --- a/src/bin/pg_basebackup/pg_basebackup.c +++ b/src/bin/pg_basebackup/pg_basebackup.c @@ -2020,9 +2020,7 @@ BaseBackup(char *compression_algorithm, char *compression_detail, if (client_compress->algorithm == PG_COMPRESSION_GZIP) { wal_compress_algorithm = PG_COMPRESSION_GZIP; - wal_compress_level = - (client_compress->options & PG_COMPRESSION_OPTION_LEVEL) - != 0 ? client_compress->level : 0; + wal_compress_level = client_compress->level; } else { @@ -2031,7 +2029,8 @@ BaseBackup(char *compression_algorithm, char *compression_detail, } StartLogStreamer(xlogstart, starttli, sysidentifier, - wal_compress_algorithm, wal_compress_level); + wal_compress_algorithm, + wal_compress_level); } if (serverMajor >= 1500) diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c index ea3902c971..e6c975d4f4 100644 --- a/src/bin/pg_basebackup/pg_receivewal.c +++ b/src/bin/pg_basebackup/pg_receivewal.c @@ -877,38 +877,11 @@ main(int argc, char **argv) pg_fatal("invalid compression specification: %s", error_detail); - /* Extract the compression level, if found in the specification */ - if ((compression_spec.options & PG_COMPRESSION_OPTION_LEVEL) != 0) - compresslevel = compression_spec.level; - - switch (compression_algorithm) - { - case PG_COMPRESSION_NONE: - /* nothing to do */ - break; - case PG_COMPRESSION_GZIP: -#ifdef HAVE_LIBZ - if ((compression_spec.options & PG_COMPRESSION_OPTION_LEVEL) == 0) - { - pg_log_info("no value specified for --compress, switching to default"); - compresslevel = Z_DEFAULT_COMPRESSION; - } -#else - pg_fatal("this build does not support compression with %s", - "gzip"); -#endif - break; - case PG_COMPRESSION_LZ4: -#ifndef USE_LZ4 - pg_fatal("this build does not support compression with %s", - "LZ4"); -#endif - break; - case PG_COMPRESSION_ZSTD: - pg_fatal("compression with %s is not yet supported", "ZSTD"); - break; - } + /* Extract the compression level */ + compresslevel = compression_spec.level; + if (compression_algorithm == PG_COMPRESSION_ZSTD) + pg_fatal("compression with %s is not yet supported", "ZSTD"); /* * Check existence of destination folder. diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl index 3d1a4ddd5c..f03f43d668 100644 --- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl +++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl @@ -86,71 +86,81 @@ $node->restart; # Now that we have a server that supports replication commands, test whether # certain invalid compression commands fail on the client side with client-side # compression and on the server side with server-side compression. -my $client_fails = 'pg_basebackup: error: '; -my $server_fails = - 'pg_basebackup: error: could not initiate base backup: ERROR: '; -my @compression_failure_tests = ( - [ - 'extrasquishy', - 'unrecognized compression algorithm "extrasquishy"', - 'failure on invalid compression algorithm' - ], - [ - 'gzip:', - 'invalid compression specification: found empty string where a compression option was expected', - 'failure on empty compression options list' - ], - [ - 'gzip:thunk', - 'invalid compression specification: unknown compression option "thunk"', - 'failure on unknown compression option' - ], - [ - 'gzip:level', - 'invalid compression specification: compression option "level" requires a value', - 'failure on missing compression level' - ], - [ - 'gzip:level=', - 'invalid compression specification: value for compression option "level" must be an integer', - 'failure on empty compression level' - ], - [ - 'gzip:level=high', - 'invalid compression specification: value for compression option "level" must be an integer', - 'failure on non-numeric compression level' - ], - [ - 'gzip:level=236', - 'invalid compression specification: compression algorithm "gzip" expects a compression level between 1 and 9', - 'failure on out-of-range compression level' - ], - [ - 'gzip:level=9,', - 'invalid compression specification: found empty string where a compression option was expected', - 'failure on extra, empty compression option' - ], - [ - 'gzip:workers=3', - 'invalid compression specification: compression algorithm "gzip" does not accept a worker count', - 'failure on worker count for gzip' - ],); -for my $cft (@compression_failure_tests) +SKIP: { - my $cfail = quotemeta($client_fails . $cft->[1]); - my $sfail = quotemeta($server_fails . $cft->[1]); - $node->command_fails_like( - [ 'pg_basebackup', '-D', "$tempdir/backup", '--compress', $cft->[0] ], - qr/$cfail/, - 'client ' . $cft->[2]); - $node->command_fails_like( + skip "postgres was not built with ZLIB support", 6 + if (!check_pg_config("#define HAVE_LIBZ 1")); + + my $client_fails = 'pg_basebackup: error: '; + my $server_fails = + 'pg_basebackup: error: could not initiate base backup: ERROR: '; + my @compression_failure_tests = ( [ - 'pg_basebackup', '-D', - "$tempdir/backup", '--compress', - 'server-' . $cft->[0] + 'extrasquishy', + 'unrecognized compression algorithm "extrasquishy"', + 'failure on invalid compression algorithm' ], - qr/$sfail/, - 'server ' . $cft->[2]); + [ + 'gzip:', + 'invalid compression specification: found empty string where a compression option was expected', + 'failure on empty compression options list' + ], + [ + 'gzip:thunk', + 'invalid compression specification: unknown compression option "thunk"', + 'failure on unknown compression option' + ], + [ + 'gzip:level', + 'invalid compression specification: compression option "level" requires a value', + 'failure on missing compression level' + ], + [ + 'gzip:level=', + 'invalid compression specification: value for compression option "level" must be an integer', + 'failure on empty compression level' + ], + [ + 'gzip:level=high', + 'invalid compression specification: value for compression option "level" must be an integer', + 'failure on non-numeric compression level' + ], + [ + 'gzip:level=236', + 'invalid compression specification: compression algorithm "gzip" expects a compression level between 1 and 9', + 'failure on out-of-range compression level' + ], + [ + 'gzip:level=9,', + 'invalid compression specification: found empty string where a compression option was expected', + 'failure on extra, empty compression option' + ], + [ + 'gzip:workers=3', + 'invalid compression specification: compression algorithm "gzip" does not accept a worker count', + 'failure on worker count for gzip' + ],); + for my $cft (@compression_failure_tests) + { + my $cfail = quotemeta($client_fails . $cft->[1]); + my $sfail = quotemeta($server_fails . $cft->[1]); + $node->command_fails_like( + [ + 'pg_basebackup', '-D', + "$tempdir/backup", '--compress', + $cft->[0] + ], + qr/$cfail/, + 'client ' . $cft->[2]); + $node->command_fails_like( + [ + 'pg_basebackup', '-D', + "$tempdir/backup", '--compress', + 'server-' . $cft->[0] + ], + qr/$sfail/, + 'server ' . $cft->[2]); + } } # Write some files to test that they are not copied. diff --git a/src/common/compression.c b/src/common/compression.c index da3c291c0f..e40ce98ef3 100644 --- a/src/common/compression.c +++ b/src/common/compression.c @@ -27,6 +27,13 @@ #include "postgres_fe.h" #endif +#ifdef USE_ZSTD +#include +#endif +#ifdef HAVE_LIBZ +#include +#endif + #include "common/compression.h" static int expect_integer_value(char *keyword, char *value, @@ -88,6 +95,9 @@ get_compress_algorithm_name(pg_compress_algorithm algorithm) * Note, however, even if there's no parse error, the string might not make * sense: e.g. for gzip, level=12 is not sensible, but it does parse OK. * + * The compression level is assigned by default if not directly specified + * by the specification. + * * Use validate_compress_specification() to find out whether a compression * specification is semantically sensible. */ @@ -101,9 +111,46 @@ parse_compress_specification(pg_compress_algorithm algorithm, char *specificatio /* Initial setup of result object. */ result->algorithm = algorithm; result->options = 0; - result->level = -1; result->parse_error = NULL; + /* + * Assign a default level depending on the compression method. This may + * be enforced later. + */ + switch (result->algorithm) + { + case PG_COMPRESSION_NONE: + result->level = 0; + break; + case PG_COMPRESSION_LZ4: +#ifdef USE_LZ4 + result->level = 0; /* fast compression mode */ +#else + result->parse_error = + psprintf(_("this build does not support compression with %s"), + "LZ4"); +#endif + break; + case PG_COMPRESSION_ZSTD: +#ifdef USE_ZSTD + result->level = ZSTD_CLEVEL_DEFAULT; +#else + result->parse_error = + psprintf(_("this build does not support compression with %s"), + "ZSTD"); +#endif + break; + case PG_COMPRESSION_GZIP: +#ifdef HAVE_LIBZ + result->level = Z_DEFAULT_COMPRESSION; +#else + result->parse_error = + psprintf(_("this build does not support compression with %s"), + "gzip"); +#endif + break; + } + /* If there is no specification, we're done already. */ if (specification == NULL) return; @@ -113,7 +160,6 @@ parse_compress_specification(pg_compress_algorithm algorithm, char *specificatio if (specification != bare_level_endp && *bare_level_endp == '\0') { result->level = bare_level; - result->options |= PG_COMPRESSION_OPTION_LEVEL; return; } @@ -175,7 +221,11 @@ parse_compress_specification(pg_compress_algorithm algorithm, char *specificatio if (strcmp(keyword, "level") == 0) { result->level = expect_integer_value(keyword, value, result); - result->options |= PG_COMPRESSION_OPTION_LEVEL; + + /* + * No need to set a flag in "options", there is a default level + * set at least thanks to the logic above. + */ } else if (strcmp(keyword, "workers") == 0) { @@ -249,36 +299,49 @@ expect_integer_value(char *keyword, char *value, pg_compress_specification *resu char * validate_compress_specification(pg_compress_specification *spec) { + int min_level = 1; + int max_level = 1; + int default_level = 0; + /* If it didn't even parse OK, it's definitely no good. */ if (spec->parse_error != NULL) return spec->parse_error; /* - * If a compression level was specified, check that the algorithm expects - * a compression level and that the level is within the legal range for - * the algorithm. + * Check that the algorithm expects a compression level and it is within + * the legal range for the algorithm. */ - if ((spec->options & PG_COMPRESSION_OPTION_LEVEL) != 0) + switch (spec->algorithm) { - int min_level = 1; - int max_level; - - if (spec->algorithm == PG_COMPRESSION_GZIP) + case PG_COMPRESSION_GZIP: max_level = 9; - else if (spec->algorithm == PG_COMPRESSION_LZ4) +#ifdef HAVE_LIBZ + default_level = Z_DEFAULT_COMPRESSION; +#endif + break; + case PG_COMPRESSION_LZ4: max_level = 12; - else if (spec->algorithm == PG_COMPRESSION_ZSTD) + default_level = 0; /* fast mode */ + break; + case PG_COMPRESSION_ZSTD: max_level = 22; - else - return psprintf(_("compression algorithm \"%s\" does not accept a compression level"), - get_compress_algorithm_name(spec->algorithm)); - - if (spec->level < min_level || spec->level > max_level) - return psprintf(_("compression algorithm \"%s\" expects a compression level between %d and %d"), - get_compress_algorithm_name(spec->algorithm), - min_level, max_level); +#ifdef USE_ZSTD + default_level = ZSTD_CLEVEL_DEFAULT; +#endif + break; + case PG_COMPRESSION_NONE: + if (spec->level != 0) + return psprintf(_("compression algorithm \"%s\" does not accept a compression level"), + get_compress_algorithm_name(spec->algorithm)); + break; } + if ((spec->level < min_level || spec->level > max_level) && + spec->level != default_level) + return psprintf(_("compression algorithm \"%s\" expects a compression level between %d and %d (default at %d)"), + get_compress_algorithm_name(spec->algorithm), + min_level, max_level, default_level); + /* * Of the compression algorithms that we currently support, only zstd * allows parallel workers. diff --git a/src/include/common/compression.h b/src/include/common/compression.h index 7aa4240a27..bb80e881af 100644 --- a/src/include/common/compression.h +++ b/src/include/common/compression.h @@ -22,8 +22,7 @@ typedef enum pg_compress_algorithm PG_COMPRESSION_ZSTD } pg_compress_algorithm; -#define PG_COMPRESSION_OPTION_LEVEL (1 << 0) -#define PG_COMPRESSION_OPTION_WORKERS (1 << 1) +#define PG_COMPRESSION_OPTION_WORKERS (1 << 0) typedef struct pg_compress_specification {