Replace BASE_BACKUP COMPRESSION_LEVEL option with COMPRESSION_DETAIL.

There are more compression parameters that can be specified than just
an integer compression level, so rename the new COMPRESSION_LEVEL
option to COMPRESSION_DETAIL before it gets released. Introduce a
flexible syntax for that option to allow arbitrary options to be
specified without needing to adjust the main replication grammar,
and common code to parse it that is shared between the client and
the server.

This commit doesn't actually add any new compression parameters,
so the only user-visible change is that you can now type something
like pg_basebackup --compress gzip:level=5 instead of writing just
pg_basebackup --compress gzip:5. However, it should make it easy to
add new options. If for example gzip starts offering fries, we can
support pg_basebackup --compress gzip:level=5,fries=true for the
benefit of users who want fries with that.

Along the way, this fixes a few things in pg_basebackup so that the
pg_basebackup can be used with a server-side compression algorithm
that pg_basebackup itself does not understand. For example,
pg_basebackup --compress server-lz4 could still succeed even if
only the server and not the client has LZ4 support, provided that
the other options to pg_basebackup don't require the client to
decompress the archive.

Patch by me. Reviewed by Justin Pryzby and Dagfinn Ilmari Mannsåker.

Discussion: http://postgr.es/m/CA+TgmoYvpetyRAbbg1M8b3-iHsaN4nsgmWPjOENu5-doHuJ7fA@mail.gmail.com

Author: robertmhaas

doc/src/sgml/protocol.sgml

@@ -2731,14 +2731,24 @@ The commands accepted in replication mode are:
        </varlistentry>
 
        <varlistentry>
-        <term><literal>COMPRESSION_LEVEL</literal> <replaceable>level</replaceable></term>
+        <term><literal>COMPRESSION_DETAIL</literal> <replaceable>detail</replaceable></term>
         <listitem>
          <para>
-          Specifies the compression level to be used. This should only be
-          used in conjunction with the <literal>COMPRESSION</literal> option.
-          For <literal>gzip</literal> the value should be an integer between 1
-          and 9, for <literal>lz4</literal> between 1 and 12, and for
-          <literal>zstd</literal> it should be between 1 and 22.
+          Specifies details for the chosen compression method. This should only
+          be used in conjunction with the <literal>COMPRESSION</literal>
+          option.  If the value is an integer, it specifies the compression
+          level.  Otherwise, it should be a comma-separated list of items,
+          each of the form <literal>keyword</literal> or
+          <literal>keyword=value</literal>. Currently, the only supported
+          keyword is <literal>level</literal>, which sets the compression
+          level.
+        </para>
+
+        <para>
+          For <literal>gzip</literal> the compression level should be an
+          integer between 1 and 9, for <literal>lz4</literal> an integer
+          between 1 and 12, and for <literal>zstd</literal> an integer
+          between 1 and 22.
          </para>
         </listitem>
        </varlistentry>

doc/src/sgml/ref/pg_basebackup.sgml

@@ -399,9 +399,9 @@ PostgreSQL documentation
 
      <varlistentry>
       <term><option>-Z <replaceable class="parameter">level</replaceable></option></term>
-      <term><option>-Z [{client|server}-]<replaceable class="parameter">method</replaceable></option>[:<replaceable>level</replaceable>]</term>
+      <term><option>-Z [{client|server}-]<replaceable class="parameter">method</replaceable>[:<replaceable>detail</replaceable>]</option></term>
       <term><option>--compress=<replaceable class="parameter">level</replaceable></option></term>
-      <term><option>--compress=[{client|server}-]<replaceable class="parameter">method</replaceable></option>[:<replaceable>level</replaceable>]</term>
+      <term><option>--compress=[{client|server}-]<replaceable class="parameter">method</replaceable>[:<replaceable>detail</replaceable>]</option></term>
       <listitem>
        <para>
         Requests compression of the backup. If <literal>client</literal> or
@@ -419,13 +419,20 @@ PostgreSQL documentation
        <para>
         The compression method can be set to <literal>gzip</literal>,
         <literal>lz4</literal>, <literal>zstd</literal>, or
-        <literal>none</literal> for no compression. A compression level can
-        optionally be specified, by appending the level number after a colon
-        (<literal>:</literal>). If no level is specified, the default
-        compression level will be used. If only a level is specified without
-        mentioning an algorithm, <literal>gzip</literal> compression will be
-        used if the level is greater than 0, and no compression will be used if
-        the level is 0.
+        <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 form
+        <literal>keyword</literal> or <literal>keyword=value</literal>.
+        Currently, the only supported keyword is <literal>level</literal>,
+        which sets the compression level.
+       </para>
+       <para>
+        If no compression level is specified, the default compression level
+        will be used. If only a level is specified without mentioning an
+        algorithm, <literal>gzip</literal> compression will be used if the
+        level is greater than 0, and no compression will be used if the level
+        is 0.
        </para>
        <para>
         When the tar format is used with <literal>gzip</literal>,

src/backend/replication/basebackup.c

@@ -17,6 +17,7 @@
 #include <time.h>
 
 #include "access/xlog_internal.h"	/* for pg_start/stop_backup */
+#include "common/backup_compression.h"
 #include "common/file_perm.h"
 #include "commands/defrem.h"
 #include "lib/stringinfo.h"
@@ -54,14 +55,6 @@
  */
 #define SINK_BUFFER_LENGTH			Max(32768, BLCKSZ)
 
-typedef enum
-{
-	BACKUP_COMPRESSION_NONE,
-	BACKUP_COMPRESSION_GZIP,
-	BACKUP_COMPRESSION_LZ4,
-	BACKUP_COMPRESSION_ZSTD
-} basebackup_compression_type;
-
 typedef struct
 {
 	const char *label;
@@ -75,8 +68,8 @@ typedef struct
 	bool		use_copytblspc;
 	BaseBackupTargetHandle *target_handle;
 	backup_manifest_option manifest;
-	basebackup_compression_type	compression;
-	int			compression_level;
+	bc_algorithm compression;
+	bc_specification compression_specification;
 	pg_checksum_type manifest_checksum_type;
 } basebackup_options;
 
@@ -713,12 +706,14 @@ parse_basebackup_options(List *options, basebackup_options *opt)
 	char	   *target_str = NULL;
 	char	   *target_detail_str = NULL;
 	bool		o_compression = false;
-	bool		o_compression_level = false;
+	bool		o_compression_detail = false;
+	char	   *compression_detail_str = NULL;
 
 	MemSet(opt, 0, sizeof(*opt));
 	opt->manifest = MANIFEST_OPTION_NO;
 	opt->manifest_checksum_type = CHECKSUM_TYPE_CRC32C;
 	opt->compression = BACKUP_COMPRESSION_NONE;
+	opt->compression_specification.algorithm = BACKUP_COMPRESSION_NONE;
 
 	foreach(lopt, options)
 	{
@@ -885,29 +880,21 @@ parse_basebackup_options(List *options, basebackup_options *opt)
 				ereport(ERROR,
 						(errcode(ERRCODE_SYNTAX_ERROR),
 						 errmsg("duplicate option \"%s\"", defel->defname)));
-			if (strcmp(optval, "none") == 0)
-				opt->compression = BACKUP_COMPRESSION_NONE;
-			else if (strcmp(optval, "gzip") == 0)
-				opt->compression = BACKUP_COMPRESSION_GZIP;
-			else if (strcmp(optval, "lz4") == 0)
-				opt->compression = BACKUP_COMPRESSION_LZ4;
-			else if (strcmp(optval, "zstd") == 0)
-				opt->compression = BACKUP_COMPRESSION_ZSTD;
-			else
+			if (!parse_bc_algorithm(optval, &opt->compression))
 				ereport(ERROR,
 						(errcode(ERRCODE_SYNTAX_ERROR),
-						 errmsg("unrecognized compression algorithm: \"%s\"",
+						 errmsg("unrecognized compression algorithm \"%s\"",
 								optval)));
 			o_compression = true;
 		}
-		else if (strcmp(defel->defname, "compression_level") == 0)
+		else if (strcmp(defel->defname, "compression_detail") == 0)
 		{
-			if (o_compression_level)
+			if (o_compression_detail)
 				ereport(ERROR,
 						(errcode(ERRCODE_SYNTAX_ERROR),
 						 errmsg("duplicate option \"%s\"", defel->defname)));
-			opt->compression_level = defGetInt32(defel);
-			o_compression_level = true;
+			compression_detail_str = defGetString(defel);
+			o_compression_detail = true;
 		}
 		else
 			ereport(ERROR,
@@ -949,10 +936,25 @@ parse_basebackup_options(List *options, basebackup_options *opt)
 		opt->target_handle =
 			BaseBackupGetTargetHandle(target_str, target_detail_str);
 
-	if (o_compression_level && !o_compression)
+	if (o_compression_detail && !o_compression)
 		ereport(ERROR,
 				(errcode(ERRCODE_SYNTAX_ERROR),
-				 errmsg("compression level requires compression")));
+				 errmsg("compression detail requires compression")));
+
+	if (o_compression)
+	{
+		char	   *error_detail;
+
+		parse_bc_specification(opt->compression, compression_detail_str,
+							   &opt->compression_specification);
+		error_detail =
+			validate_bc_specification(&opt->compression_specification);
+		if (error_detail != NULL)
+			ereport(ERROR,
+					errcode(ERRCODE_SYNTAX_ERROR),
+					errmsg("invalid compression specification: %s",
+						   error_detail));
+	}
 }
 
 
@@ -998,11 +1000,11 @@ SendBaseBackup(BaseBackupCmd *cmd)
 
 	/* Set up server-side compression, if client requested it */
 	if (opt.compression == BACKUP_COMPRESSION_GZIP)
-		sink = bbsink_gzip_new(sink, opt.compression_level);
+		sink = bbsink_gzip_new(sink, &opt.compression_specification);
 	else if (opt.compression == BACKUP_COMPRESSION_LZ4)
-		sink = bbsink_lz4_new(sink, opt.compression_level);
+		sink = bbsink_lz4_new(sink, &opt.compression_specification);
 	else if (opt.compression == BACKUP_COMPRESSION_ZSTD)
-		sink = bbsink_zstd_new(sink, opt.compression_level);
+		sink = bbsink_zstd_new(sink, &opt.compression_specification);
 
 	/* Set up progress reporting. */
 	sink = bbsink_progress_new(sink, opt.progress);

src/backend/replication/basebackup_gzip.c

@@ -56,12 +56,13 @@ const bbsink_ops bbsink_gzip_ops = {
 #endif
 
 /*
- * Create a new basebackup sink that performs gzip compression using the
- * designated compression level.
+ * Create a new basebackup sink that performs gzip compression.
  */
 bbsink *
-bbsink_gzip_new(bbsink *next, int compresslevel)
+bbsink_gzip_new(bbsink *next, bc_specification *compress)
 {
+	int		compresslevel;
+
 #ifndef HAVE_LIBZ
 	ereport(ERROR,
 			(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -71,15 +72,14 @@ bbsink_gzip_new(bbsink *next, int compresslevel)
 	bbsink_gzip *sink;
 
 	Assert(next != NULL);
-	Assert(compresslevel >= 0 && compresslevel <= 9);
 
-	if (compresslevel == 0)
+	if ((compress->options & BACKUP_COMPRESSION_OPTION_LEVEL) == 0)
 		compresslevel = Z_DEFAULT_COMPRESSION;
-	else if (compresslevel < 0 || compresslevel > 9)
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("gzip compression level %d is out of range",
-						compresslevel)));
+	else
+	{
+		compresslevel = compress->level;
+		Assert(compresslevel >= 1 && compresslevel <= 9);
+	}
 
 	sink = palloc0(sizeof(bbsink_gzip));
 	*((const bbsink_ops **) &sink->base.bbs_ops) = &bbsink_gzip_ops;

src/backend/replication/basebackup_lz4.c

@@ -56,12 +56,13 @@ const bbsink_ops bbsink_lz4_ops = {
 #endif
 
 /*
- * Create a new basebackup sink that performs lz4 compression using the
- * designated compression level.
+ * Create a new basebackup sink that performs lz4 compression.
  */
 bbsink *
-bbsink_lz4_new(bbsink *next, int compresslevel)
+bbsink_lz4_new(bbsink *next, bc_specification *compress)
 {
+	int		compresslevel;
+
 #ifndef USE_LZ4
 	ereport(ERROR,
 			(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -72,11 +73,13 @@ bbsink_lz4_new(bbsink *next, int compresslevel)
 
 	Assert(next != NULL);
 
-	if (compresslevel < 0 || compresslevel > 12)
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("lz4 compression level %d is out of range",
-						compresslevel)));
+	if ((compress->options & BACKUP_COMPRESSION_OPTION_LEVEL) == 0)
+		compresslevel = 0;
+	else
+	{
+		compresslevel = compress->level;
+		Assert(compresslevel >= 1 && compresslevel <= 12);
+	}
 
 	sink = palloc0(sizeof(bbsink_lz4));
 	*((const bbsink_ops **) &sink->base.bbs_ops) = &bbsink_lz4_ops;

src/backend/replication/basebackup_zstd.c

@@ -55,12 +55,13 @@ const bbsink_ops bbsink_zstd_ops = {
 #endif
 
 /*
- * Create a new basebackup sink that performs zstd compression using the
- * designated compression level.
+ * Create a new basebackup sink that performs zstd compression.
  */
 bbsink *
-bbsink_zstd_new(bbsink *next, int compresslevel)
+bbsink_zstd_new(bbsink *next, bc_specification *compress)
 {
+	int		compresslevel;
+
 #ifndef USE_ZSTD
 	ereport(ERROR,
 			(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -71,11 +72,13 @@ bbsink_zstd_new(bbsink *next, int compresslevel)
 
 	Assert(next != NULL);
 
-	if (compresslevel < 0 || compresslevel > 22)
-		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("zstd compression level %d is out of range",
-						compresslevel)));
+	if ((compress->options & BACKUP_COMPRESSION_OPTION_LEVEL) == 0)
+		compresslevel = 0;
+	else
+	{
+		compresslevel = compress->level;
+		Assert(compresslevel >= 1 && compresslevel <= 22);
+	}
 
 	sink = palloc0(sizeof(bbsink_zstd));
 	*((const bbsink_ops **) &sink->base.bbs_ops) = &bbsink_zstd_ops;

src/bin/pg_basebackup/bbstreamer.h

@@ -22,6 +22,7 @@
 #ifndef BBSTREAMER_H
 #define BBSTREAMER_H
 
+#include "common/backup_compression.h"
 #include "lib/stringinfo.h"
 #include "pqexpbuffer.h"
 
@@ -200,17 +201,17 @@ bbstreamer_buffer_until(bbstreamer *streamer, const char **data, int *len,
  */
 extern bbstreamer *bbstreamer_plain_writer_new(char *pathname, FILE *file);
 extern bbstreamer *bbstreamer_gzip_writer_new(char *pathname, FILE *file,
-											  int compresslevel);
+											  bc_specification *compress);
 extern bbstreamer *bbstreamer_extractor_new(const char *basepath,
 											const char *(*link_map) (const char *),
 											void (*report_output_file) (const char *));
 
 extern bbstreamer *bbstreamer_gzip_decompressor_new(bbstreamer *next);
 extern bbstreamer *bbstreamer_lz4_compressor_new(bbstreamer *next,
-												 int compresslevel);
+												 bc_specification *compress);
 extern bbstreamer *bbstreamer_lz4_decompressor_new(bbstreamer *next);
 extern bbstreamer *bbstreamer_zstd_compressor_new(bbstreamer *next,
-												  int compresslevel);
+												  bc_specification *compress);
 extern bbstreamer *bbstreamer_zstd_decompressor_new(bbstreamer *next);
 extern bbstreamer *bbstreamer_tar_parser_new(bbstreamer *next);
 extern bbstreamer *bbstreamer_tar_terminator_new(bbstreamer *next);

src/bin/pg_basebackup/bbstreamer_gzip.c

@@ -76,7 +76,8 @@ const bbstreamer_ops bbstreamer_gzip_decompressor_ops = {
  * closed so that the data may be written there.
  */
 bbstreamer *
-bbstreamer_gzip_writer_new(char *pathname, FILE *file, int compresslevel)
+bbstreamer_gzip_writer_new(char *pathname, FILE *file,
+						   bc_specification *compress)
 {
 #ifdef HAVE_LIBZ
 	bbstreamer_gzip_writer *streamer;
@@ -115,11 +116,11 @@ bbstreamer_gzip_writer_new(char *pathname, FILE *file, int compresslevel)
 		}
 	}
 
-	if (gzsetparams(streamer->gzfile, compresslevel,
+	if (gzsetparams(streamer->gzfile, compress->level,
 					Z_DEFAULT_STRATEGY) != Z_OK)
 	{
 		pg_log_error("could not set compression level %d: %s",
-					 compresslevel, get_gz_error(streamer->gzfile));
+					 compress->level, get_gz_error(streamer->gzfile));
 		exit(1);
 	}
 

src/bin/pg_basebackup/bbstreamer_lz4.c

@@ -67,7 +67,7 @@ const bbstreamer_ops bbstreamer_lz4_decompressor_ops = {
  * blocks.
  */
 bbstreamer *
-bbstreamer_lz4_compressor_new(bbstreamer *next, int compresslevel)
+bbstreamer_lz4_compressor_new(bbstreamer *next, bc_specification *compress)
 {
 #ifdef USE_LZ4
 	bbstreamer_lz4_frame   *streamer;
@@ -89,7 +89,7 @@ bbstreamer_lz4_compressor_new(bbstreamer *next, int compresslevel)
 	prefs = &streamer->prefs;
 	memset(prefs, 0, sizeof(LZ4F_preferences_t));
 	prefs->frameInfo.blockSizeID = LZ4F_max256KB;
-	prefs->compressionLevel = compresslevel;
+	prefs->compressionLevel = compress->level;
 
 	/*
 	 * Find out the compression bound, it specifies the minimum destination

src/bin/pg_basebackup/bbstreamer_zstd.c

@@ -63,7 +63,7 @@ const bbstreamer_ops bbstreamer_zstd_decompressor_ops = {
  * blocks.
  */
 bbstreamer *
-bbstreamer_zstd_compressor_new(bbstreamer *next, int compresslevel)
+bbstreamer_zstd_compressor_new(bbstreamer *next, bc_specification *compress)
 {
 #ifdef USE_ZSTD
 	bbstreamer_zstd_frame *streamer;
@@ -85,7 +85,7 @@ bbstreamer_zstd_compressor_new(bbstreamer *next, int compresslevel)
 
 	/* Initialize stream compression preferences */
 	ZSTD_CCtx_setParameter(streamer->cctx, ZSTD_c_compressionLevel,
-						   compresslevel);
+						   compress->level);
 
 	/* Initialize the ZSTD output buffer. */
 	streamer->zstd_outBuf.dst = streamer->base.bbs_buffer.data;