Fixes for compression options of pg_receivewal and refactoring of backup_compression.{c,h}

Started by Michael Paquieralmost 4 years ago9 messages
#1Michael Paquier
michael@paquier.xyz
3 attachment(s)

Hi all,
(Added Robert and Georgios in CC:)

Since babbbb5 and the introduction of LZ4, I have reworked the way
compression is controlled for pg_receivewal, with two options:
- --compress-method, settable to "gzip", "none" or "lz4".
- --compress, to pass down a compression level, where the allowed
range is 1-9. If passing down 0, we'd get an error rather than
implying no compression, contrary to what we did in ~14.

I initially thought that this was fine as-is, but then Robert and
others have worked on client/server compression for pg_basebackup,
introducing a much better design with centralized APIs where one can
use METHOD:DETAIL for as compression value, where DETAIL is a
comma-separated list of keyword=value (keyword = "level" or
"workers"), with centralized checks and an extensible design.

This is something I think we had better fix before beta1, because now
we have binaries that use an inconsistent set of options. So,
attached is a patch set aimed at rework this option set from the
ground, taking advantage of the recent work done by Robert and others
for pg_basebackup:
- 0001 is a simple rename of backup_compression.{c,h} to
compression.{c,h}, removing anything related to base backups from
that. One extra reason behind this renaming is that I would like to
use this infra for pg_dump, but that's material for 16~.
- 0002 removes WalCompressionMethod, replacing it by
pg_compress_algorithm as these are the same enums. Robert complained
about the confusion that WalCompressionMethod could lead to as this
could be used for the compression of data, and not only WAL. I have
renamed some variables to be more consistent, while on it.
- 0003 is the actual option rework for pg_receivewal. This takes
advantage of 0001, leading to the result of removing --compress-method
and replacing it with --compress, taking care of the backward
compatibility problems for an integer value, aka 0 implies no
compression and val > 0 implies gzip. One bonus reason to switch to
that is that this would make the addition of zstd for pg_receivewal
easier in the future.

I am going to add an open item for this stuff. Comments or thoughts?

Thanks,
--
Michael

Attachments:

v1-0001-Rename-backup_compression.-c-h-to-compression.-c-.patchtext/x-diff; charset=us-asciiDownload
From ccef39021a254c3ca4d0a380f138bf47e9b9b9c7 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Mon, 11 Apr 2022 13:50:18 +0900
Subject: [PATCH v1 1/3] Rename backup_compression.{c,h} to compression.{c,h}

Compression option handling (level, algorithm or even workers) can be
used across several parts of the system and not only base backups, with
at least pg_receivewal and pg_dump.  Structures, objects and routines
are renamed in consequence, to remove the concept of base backups from
that.
---
 src/include/common/backup_compression.h       | 46 -------------
 src/include/common/compression.h              | 46 +++++++++++++
 src/include/replication/basebackup_sink.h     |  8 +--
 src/backend/replication/basebackup.c          | 30 ++++-----
 src/backend/replication/basebackup_gzip.c     |  4 +-
 src/backend/replication/basebackup_lz4.c      |  4 +-
 src/backend/replication/basebackup_zstd.c     | 10 +--
 src/common/Makefile                           |  2 +-
 .../{backup_compression.c => compression.c}   | 66 +++++++++----------
 src/bin/pg_basebackup/bbstreamer.h            |  8 +--
 src/bin/pg_basebackup/bbstreamer_gzip.c       |  4 +-
 src/bin/pg_basebackup/bbstreamer_lz4.c        |  4 +-
 src/bin/pg_basebackup/bbstreamer_zstd.c       |  6 +-
 src/bin/pg_basebackup/nls.mk                  |  2 +-
 src/bin/pg_basebackup/pg_basebackup.c         | 44 ++++++-------
 src/tools/msvc/Mkvcbuild.pm                   |  2 +-
 16 files changed, 143 insertions(+), 143 deletions(-)
 delete mode 100644 src/include/common/backup_compression.h
 create mode 100644 src/include/common/compression.h
 rename src/common/{backup_compression.c => compression.c} (80%)

diff --git a/src/include/common/backup_compression.h b/src/include/common/backup_compression.h
deleted file mode 100644
index 6a0ecaa99c..0000000000
--- a/src/include/common/backup_compression.h
+++ /dev/null
@@ -1,46 +0,0 @@
-/*-------------------------------------------------------------------------
- *
- * backup_compression.h
- *
- * Shared definitions for backup compression methods and specifications.
- *
- * Portions Copyright (c) 1996-2022, PostgreSQL Global Development Group
- *
- * IDENTIFICATION
- *		  src/common/backup_compression.h
- *-------------------------------------------------------------------------
- */
-
-#ifndef BACKUP_COMPRESSION_H
-#define BACKUP_COMPRESSION_H
-
-typedef enum bc_algorithm
-{
-	BACKUP_COMPRESSION_NONE,
-	BACKUP_COMPRESSION_GZIP,
-	BACKUP_COMPRESSION_LZ4,
-	BACKUP_COMPRESSION_ZSTD
-} bc_algorithm;
-
-#define	BACKUP_COMPRESSION_OPTION_LEVEL			(1 << 0)
-#define BACKUP_COMPRESSION_OPTION_WORKERS		(1 << 1)
-
-typedef struct bc_specification
-{
-	bc_algorithm algorithm;
-	unsigned	options;		/* OR of BACKUP_COMPRESSION_OPTION constants */
-	int			level;
-	int			workers;
-	char	   *parse_error;	/* NULL if parsing was OK, else message */
-} bc_specification;
-
-extern bool parse_bc_algorithm(char *name, bc_algorithm *algorithm);
-extern const char *get_bc_algorithm_name(bc_algorithm algorithm);
-
-extern void parse_bc_specification(bc_algorithm algorithm,
-								   char *specification,
-								   bc_specification *result);
-
-extern char *validate_bc_specification(bc_specification *);
-
-#endif
diff --git a/src/include/common/compression.h b/src/include/common/compression.h
new file mode 100644
index 0000000000..b54deb9ead
--- /dev/null
+++ b/src/include/common/compression.h
@@ -0,0 +1,46 @@
+/*-------------------------------------------------------------------------
+ *
+ * compression.h
+ *
+ * Shared definitions for compression methods and specifications.
+ *
+ * Portions Copyright (c) 1996-2022, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ *		  src/common/compression.h
+ *-------------------------------------------------------------------------
+ */
+
+#ifndef PG_COMPRESSION_H
+#define PG_COMPRESSION_H
+
+typedef enum pg_compress_algorithm
+{
+	PG_COMPRESSION_NONE,
+	PG_COMPRESSION_GZIP,
+	PG_COMPRESSION_LZ4,
+	PG_COMPRESSION_ZSTD
+} pg_compress_algorithm;
+
+#define	PG_COMPRESSION_OPTION_LEVEL			(1 << 0)
+#define PG_COMPRESSION_OPTION_WORKERS		(1 << 1)
+
+typedef struct pg_compress_specification
+{
+	pg_compress_algorithm algorithm;
+	unsigned	options;		/* OR of PG_COMPRESSION_OPTION constants */
+	int			level;
+	int			workers;
+	char	   *parse_error;	/* NULL if parsing was OK, else message */
+} pg_compress_specification;
+
+extern bool parse_compress_algorithm(char *name, pg_compress_algorithm *algorithm);
+extern const char *get_compress_algorithm_name(pg_compress_algorithm algorithm);
+
+extern void parse_compress_specification(pg_compress_algorithm algorithm,
+								   char *specification,
+								   pg_compress_specification *result);
+
+extern char *validate_compress_specification(pg_compress_specification *);
+
+#endif
diff --git a/src/include/replication/basebackup_sink.h b/src/include/replication/basebackup_sink.h
index 654df28576..36278cac14 100644
--- a/src/include/replication/basebackup_sink.h
+++ b/src/include/replication/basebackup_sink.h
@@ -27,7 +27,7 @@
 #define BASEBACKUP_SINK_H
 
 #include "access/xlog_internal.h"
-#include "common/backup_compression.h"
+#include "common/compression.h"
 #include "nodes/pg_list.h"
 
 /* Forward declarations. */
@@ -284,9 +284,9 @@ extern void bbsink_forward_cleanup(bbsink *sink);
 
 /* Constructors for various types of sinks. */
 extern bbsink *bbsink_copystream_new(bool send_to_client);
-extern bbsink *bbsink_gzip_new(bbsink *next, bc_specification *);
-extern bbsink *bbsink_lz4_new(bbsink *next, bc_specification *);
-extern bbsink *bbsink_zstd_new(bbsink *next, bc_specification *);
+extern bbsink *bbsink_gzip_new(bbsink *next, pg_compress_specification *);
+extern bbsink *bbsink_lz4_new(bbsink *next, pg_compress_specification *);
+extern bbsink *bbsink_zstd_new(bbsink *next, pg_compress_specification *);
 extern bbsink *bbsink_progress_new(bbsink *next, bool estimate_backup_size);
 extern bbsink *bbsink_server_new(bbsink *next, char *pathname);
 extern bbsink *bbsink_throttle_new(bbsink *next, uint32 maxrate);
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index 27e4152446..c555b2f2ed 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -17,7 +17,7 @@
 #include <time.h>
 
 #include "access/xlog_internal.h"	/* for pg_start/stop_backup */
-#include "common/backup_compression.h"
+#include "common/compression.h"
 #include "common/file_perm.h"
 #include "commands/defrem.h"
 #include "lib/stringinfo.h"
@@ -68,8 +68,8 @@ typedef struct
 	bool		use_copytblspc;
 	BaseBackupTargetHandle *target_handle;
 	backup_manifest_option manifest;
-	bc_algorithm compression;
-	bc_specification compression_specification;
+	pg_compress_algorithm compression;
+	pg_compress_specification specification;
 	pg_checksum_type manifest_checksum_type;
 } basebackup_options;
 
@@ -691,8 +691,8 @@ parse_basebackup_options(List *options, basebackup_options *opt)
 	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;
+	opt->compression = PG_COMPRESSION_NONE;
+	opt->specification.algorithm = PG_COMPRESSION_NONE;
 
 	foreach(lopt, options)
 	{
@@ -859,7 +859,7 @@ parse_basebackup_options(List *options, basebackup_options *opt)
 				ereport(ERROR,
 						(errcode(ERRCODE_SYNTAX_ERROR),
 						 errmsg("duplicate option \"%s\"", defel->defname)));
-			if (!parse_bc_algorithm(optval, &opt->compression))
+			if (!parse_compress_algorithm(optval, &opt->compression))
 				ereport(ERROR,
 						(errcode(ERRCODE_SYNTAX_ERROR),
 						 errmsg("unrecognized compression algorithm \"%s\"",
@@ -924,10 +924,10 @@ parse_basebackup_options(List *options, basebackup_options *opt)
 	{
 		char	   *error_detail;
 
-		parse_bc_specification(opt->compression, compression_detail_str,
-							   &opt->compression_specification);
+		parse_compress_specification(opt->compression, compression_detail_str,
+							   &opt->specification);
 		error_detail =
-			validate_bc_specification(&opt->compression_specification);
+			validate_compress_specification(&opt->specification);
 		if (error_detail != NULL)
 			ereport(ERROR,
 					errcode(ERRCODE_SYNTAX_ERROR),
@@ -978,12 +978,12 @@ SendBaseBackup(BaseBackupCmd *cmd)
 		sink = bbsink_throttle_new(sink, opt.maxrate);
 
 	/* Set up server-side compression, if client requested it */
-	if (opt.compression == BACKUP_COMPRESSION_GZIP)
-		sink = bbsink_gzip_new(sink, &opt.compression_specification);
-	else if (opt.compression == BACKUP_COMPRESSION_LZ4)
-		sink = bbsink_lz4_new(sink, &opt.compression_specification);
-	else if (opt.compression == BACKUP_COMPRESSION_ZSTD)
-		sink = bbsink_zstd_new(sink, &opt.compression_specification);
+	if (opt.compression == PG_COMPRESSION_GZIP)
+		sink = bbsink_gzip_new(sink, &opt.specification);
+	else if (opt.compression == PG_COMPRESSION_LZ4)
+		sink = bbsink_lz4_new(sink, &opt.specification);
+	else if (opt.compression == PG_COMPRESSION_ZSTD)
+		sink = bbsink_zstd_new(sink, &opt.specification);
 
 	/* Set up progress reporting. */
 	sink = bbsink_progress_new(sink, opt.progress);
diff --git a/src/backend/replication/basebackup_gzip.c b/src/backend/replication/basebackup_gzip.c
index e4df57b121..b68afef86c 100644
--- a/src/backend/replication/basebackup_gzip.c
+++ b/src/backend/replication/basebackup_gzip.c
@@ -59,7 +59,7 @@ const bbsink_ops bbsink_gzip_ops = {
  * Create a new basebackup sink that performs gzip compression.
  */
 bbsink *
-bbsink_gzip_new(bbsink *next, bc_specification *compress)
+bbsink_gzip_new(bbsink *next, pg_compress_specification *compress)
 {
 #ifndef HAVE_LIBZ
 	ereport(ERROR,
@@ -72,7 +72,7 @@ bbsink_gzip_new(bbsink *next, bc_specification *compress)
 
 	Assert(next != NULL);
 
-	if ((compress->options & BACKUP_COMPRESSION_OPTION_LEVEL) == 0)
+	if ((compress->options & PG_COMPRESSION_OPTION_LEVEL) == 0)
 		compresslevel = Z_DEFAULT_COMPRESSION;
 	else
 	{
diff --git a/src/backend/replication/basebackup_lz4.c b/src/backend/replication/basebackup_lz4.c
index 48929321a4..310285d4c5 100644
--- a/src/backend/replication/basebackup_lz4.c
+++ b/src/backend/replication/basebackup_lz4.c
@@ -59,7 +59,7 @@ const bbsink_ops bbsink_lz4_ops = {
  * Create a new basebackup sink that performs lz4 compression.
  */
 bbsink *
-bbsink_lz4_new(bbsink *next, bc_specification *compress)
+bbsink_lz4_new(bbsink *next, pg_compress_specification *compress)
 {
 #ifndef USE_LZ4
 	ereport(ERROR,
@@ -72,7 +72,7 @@ bbsink_lz4_new(bbsink *next, bc_specification *compress)
 
 	Assert(next != NULL);
 
-	if ((compress->options & BACKUP_COMPRESSION_OPTION_LEVEL) == 0)
+	if ((compress->options & PG_COMPRESSION_OPTION_LEVEL) == 0)
 		compresslevel = 0;
 	else
 	{
diff --git a/src/backend/replication/basebackup_zstd.c b/src/backend/replication/basebackup_zstd.c
index f6876f4811..97fb25b99f 100644
--- a/src/backend/replication/basebackup_zstd.c
+++ b/src/backend/replication/basebackup_zstd.c
@@ -26,7 +26,7 @@ typedef struct bbsink_zstd
 	bbsink		base;
 
 	/* Compression options */
-	bc_specification *compress;
+	pg_compress_specification *compress;
 
 	ZSTD_CCtx  *cctx;
 	ZSTD_outBuffer zstd_outBuf;
@@ -58,7 +58,7 @@ const bbsink_ops bbsink_zstd_ops = {
  * Create a new basebackup sink that performs zstd compression.
  */
 bbsink *
-bbsink_zstd_new(bbsink *next, bc_specification *compress)
+bbsink_zstd_new(bbsink *next, pg_compress_specification *compress)
 {
 #ifndef USE_ZSTD
 	ereport(ERROR,
@@ -90,13 +90,13 @@ bbsink_zstd_begin_backup(bbsink *sink)
 	bbsink_zstd *mysink = (bbsink_zstd *) sink;
 	size_t		output_buffer_bound;
 	size_t		ret;
-	bc_specification *compress = mysink->compress;
+	pg_compress_specification *compress = mysink->compress;
 
 	mysink->cctx = ZSTD_createCCtx();
 	if (!mysink->cctx)
 		elog(ERROR, "could not create zstd compression context");
 
-	if ((compress->options & BACKUP_COMPRESSION_OPTION_LEVEL) != 0)
+	if ((compress->options & PG_COMPRESSION_OPTION_LEVEL) != 0)
 	{
 		ret = ZSTD_CCtx_setParameter(mysink->cctx, ZSTD_c_compressionLevel,
 									 compress->level);
@@ -105,7 +105,7 @@ bbsink_zstd_begin_backup(bbsink *sink)
 				 compress->level, ZSTD_getErrorName(ret));
 	}
 
-	if ((compress->options & BACKUP_COMPRESSION_OPTION_WORKERS) != 0)
+	if ((compress->options & PG_COMPRESSION_OPTION_WORKERS) != 0)
 	{
 		/*
 		 * On older versions of libzstd, this option does not exist, and trying
diff --git a/src/common/Makefile b/src/common/Makefile
index f627349835..e9af7346c9 100644
--- a/src/common/Makefile
+++ b/src/common/Makefile
@@ -47,9 +47,9 @@ LIBS += $(PTHREAD_LIBS)
 
 OBJS_COMMON = \
 	archive.o \
-	backup_compression.o \
 	base64.o \
 	checksum_helper.o \
+	compression.o \
 	config_info.o \
 	controldata_utils.o \
 	d2s.o \
diff --git a/src/common/backup_compression.c b/src/common/compression.c
similarity index 80%
rename from src/common/backup_compression.c
rename to src/common/compression.c
index 867f2f2eb5..632fae6144 100644
--- a/src/common/backup_compression.c
+++ b/src/common/compression.c
@@ -1,8 +1,8 @@
 /*-------------------------------------------------------------------------
  *
- * backup_compression.c
+ * compression.c
  *
- * Shared code for backup compression methods and specifications.
+ * Shared code for compression methods and specifications.
  *
  * A compression specification specifies the parameters that should be used
  * when performing compression with a specific algorithm. The simplest
@@ -12,12 +12,12 @@
  * Otherwise, a compression specification is a comma-separated list of items,
  * each having the form keyword or keyword=value.
  *
- * Currently, the only supported keyword is "level".
+ * Currently, the only supported keywords are "level" and "workers".
  *
  * Portions Copyright (c) 1996-2022, PostgreSQL Global Development Group
  *
  * IDENTIFICATION
- *		  src/common/backup_compression.c
+ *		  src/common/compression.c
  *-------------------------------------------------------------------------
  */
 
@@ -27,26 +27,26 @@
 #include "postgres_fe.h"
 #endif
 
-#include "common/backup_compression.h"
+#include "common/compression.h"
 
 static int	expect_integer_value(char *keyword, char *value,
-								 bc_specification *result);
+								 pg_compress_specification *result);
 
 /*
  * Look up a compression algorithm by name. Returns true and sets *algorithm
  * if the name is recognized. Otherwise returns false.
  */
 bool
-parse_bc_algorithm(char *name, bc_algorithm *algorithm)
+parse_compress_algorithm(char *name, pg_compress_algorithm *algorithm)
 {
 	if (strcmp(name, "none") == 0)
-		*algorithm = BACKUP_COMPRESSION_NONE;
+		*algorithm = PG_COMPRESSION_NONE;
 	else if (strcmp(name, "gzip") == 0)
-		*algorithm = BACKUP_COMPRESSION_GZIP;
+		*algorithm = PG_COMPRESSION_GZIP;
 	else if (strcmp(name, "lz4") == 0)
-		*algorithm = BACKUP_COMPRESSION_LZ4;
+		*algorithm = PG_COMPRESSION_LZ4;
 	else if (strcmp(name, "zstd") == 0)
-		*algorithm = BACKUP_COMPRESSION_ZSTD;
+		*algorithm = PG_COMPRESSION_ZSTD;
 	else
 		return false;
 	return true;
@@ -57,17 +57,17 @@ parse_bc_algorithm(char *name, bc_algorithm *algorithm)
  * algorithm.
  */
 const char *
-get_bc_algorithm_name(bc_algorithm algorithm)
+get_compress_algorithm_name(pg_compress_algorithm algorithm)
 {
 	switch (algorithm)
 	{
-		case BACKUP_COMPRESSION_NONE:
+		case PG_COMPRESSION_NONE:
 			return "none";
-		case BACKUP_COMPRESSION_GZIP:
+		case PG_COMPRESSION_GZIP:
 			return "gzip";
-		case BACKUP_COMPRESSION_LZ4:
+		case PG_COMPRESSION_LZ4:
 			return "lz4";
-		case BACKUP_COMPRESSION_ZSTD:
+		case PG_COMPRESSION_ZSTD:
 			return "zstd";
 			/* no default, to provoke compiler warnings if values are added */
 	}
@@ -88,12 +88,12 @@ get_bc_algorithm_name(bc_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.
  *
- * Use validate_bc_specification() to find out whether a compression
+ * Use validate_compress_specification() to find out whether a compression
  * specification is semantically sensible.
  */
 void
-parse_bc_specification(bc_algorithm algorithm, char *specification,
-					   bc_specification *result)
+parse_compress_specification(pg_compress_algorithm algorithm, char *specification,
+					   pg_compress_specification *result)
 {
 	int			bare_level;
 	char	   *bare_level_endp;
@@ -113,7 +113,7 @@ parse_bc_specification(bc_algorithm algorithm, char *specification,
 	if (specification != bare_level_endp && *bare_level_endp == '\0')
 	{
 		result->level = bare_level;
-		result->options |= BACKUP_COMPRESSION_OPTION_LEVEL;
+		result->options |= PG_COMPRESSION_OPTION_LEVEL;
 		return;
 	}
 
@@ -175,12 +175,12 @@ parse_bc_specification(bc_algorithm algorithm, char *specification,
 		if (strcmp(keyword, "level") == 0)
 		{
 			result->level = expect_integer_value(keyword, value, result);
-			result->options |= BACKUP_COMPRESSION_OPTION_LEVEL;
+			result->options |= PG_COMPRESSION_OPTION_LEVEL;
 		}
 		else if (strcmp(keyword, "workers") == 0)
 		{
 			result->workers = expect_integer_value(keyword, value, result);
-			result->options |= BACKUP_COMPRESSION_OPTION_WORKERS;
+			result->options |= PG_COMPRESSION_OPTION_WORKERS;
 		}
 		else
 			result->parse_error =
@@ -215,7 +215,7 @@ parse_bc_specification(bc_algorithm algorithm, char *specification,
  * and return -1.
  */
 static int
-expect_integer_value(char *keyword, char *value, bc_specification *result)
+expect_integer_value(char *keyword, char *value, pg_compress_specification *result)
 {
 	int			ivalue;
 	char	   *ivalue_endp;
@@ -247,7 +247,7 @@ expect_integer_value(char *keyword, char *value, bc_specification *result)
  * compression method.
  */
 char *
-validate_bc_specification(bc_specification *spec)
+validate_compress_specification(pg_compress_specification *spec)
 {
 	/* If it didn't even parse OK, it's definitely no good. */
 	if (spec->parse_error != NULL)
@@ -258,24 +258,24 @@ validate_bc_specification(bc_specification *spec)
 	 * a compression level and that the level is within the legal range for
 	 * the algorithm.
 	 */
-	if ((spec->options & BACKUP_COMPRESSION_OPTION_LEVEL) != 0)
+	if ((spec->options & PG_COMPRESSION_OPTION_LEVEL) != 0)
 	{
 		int			min_level = 1;
 		int			max_level;
 
-		if (spec->algorithm == BACKUP_COMPRESSION_GZIP)
+		if (spec->algorithm == PG_COMPRESSION_GZIP)
 			max_level = 9;
-		else if (spec->algorithm == BACKUP_COMPRESSION_LZ4)
+		else if (spec->algorithm == PG_COMPRESSION_LZ4)
 			max_level = 12;
-		else if (spec->algorithm == BACKUP_COMPRESSION_ZSTD)
+		else if (spec->algorithm == PG_COMPRESSION_ZSTD)
 			max_level = 22;
 		else
 			return psprintf(_("compression algorithm \"%s\" does not accept a compression level"),
-							get_bc_algorithm_name(spec->algorithm));
+							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_bc_algorithm_name(spec->algorithm),
+							get_compress_algorithm_name(spec->algorithm),
 							min_level, max_level);
 	}
 
@@ -283,11 +283,11 @@ validate_bc_specification(bc_specification *spec)
 	 * Of the compression algorithms that we currently support, only zstd
 	 * allows parallel workers.
 	 */
-	if ((spec->options & BACKUP_COMPRESSION_OPTION_WORKERS) != 0 &&
-		(spec->algorithm != BACKUP_COMPRESSION_ZSTD))
+	if ((spec->options & PG_COMPRESSION_OPTION_WORKERS) != 0 &&
+		(spec->algorithm != PG_COMPRESSION_ZSTD))
 	{
 		return psprintf(_("compression algorithm \"%s\" does not accept a worker count"),
-						get_bc_algorithm_name(spec->algorithm));
+						get_compress_algorithm_name(spec->algorithm));
 	}
 
 	return NULL;
diff --git a/src/bin/pg_basebackup/bbstreamer.h b/src/bin/pg_basebackup/bbstreamer.h
index dfa3f77af4..29b48ef54d 100644
--- a/src/bin/pg_basebackup/bbstreamer.h
+++ b/src/bin/pg_basebackup/bbstreamer.h
@@ -22,7 +22,7 @@
 #ifndef BBSTREAMER_H
 #define BBSTREAMER_H
 
-#include "common/backup_compression.h"
+#include "common/compression.h"
 #include "lib/stringinfo.h"
 #include "pqexpbuffer.h"
 
@@ -201,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,
-											  bc_specification *compress);
+											  pg_compress_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,
-												 bc_specification *compress);
+												 pg_compress_specification *compress);
 extern bbstreamer *bbstreamer_lz4_decompressor_new(bbstreamer *next);
 extern bbstreamer *bbstreamer_zstd_compressor_new(bbstreamer *next,
-												  bc_specification *compress);
+												  pg_compress_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);
diff --git a/src/bin/pg_basebackup/bbstreamer_gzip.c b/src/bin/pg_basebackup/bbstreamer_gzip.c
index 1ab7ee6ea9..b3bfcd62ac 100644
--- a/src/bin/pg_basebackup/bbstreamer_gzip.c
+++ b/src/bin/pg_basebackup/bbstreamer_gzip.c
@@ -77,7 +77,7 @@ const bbstreamer_ops bbstreamer_gzip_decompressor_ops = {
  */
 bbstreamer *
 bbstreamer_gzip_writer_new(char *pathname, FILE *file,
-						   bc_specification *compress)
+						   pg_compress_specification *compress)
 {
 #ifdef HAVE_LIBZ
 	bbstreamer_gzip_writer *streamer;
@@ -107,7 +107,7 @@ bbstreamer_gzip_writer_new(char *pathname, FILE *file,
 			pg_fatal("could not open output file: %m");
 	}
 
-	if ((compress->options & BACKUP_COMPRESSION_OPTION_LEVEL) != 0 &&
+	if ((compress->options & PG_COMPRESSION_OPTION_LEVEL) != 0 &&
 		gzsetparams(streamer->gzfile, compress->level,
 					Z_DEFAULT_STRATEGY) != Z_OK)
 		pg_fatal("could not set compression level %d: %s",
diff --git a/src/bin/pg_basebackup/bbstreamer_lz4.c b/src/bin/pg_basebackup/bbstreamer_lz4.c
index 2f75ba5602..6070a72cdb 100644
--- a/src/bin/pg_basebackup/bbstreamer_lz4.c
+++ b/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, bc_specification *compress)
+bbstreamer_lz4_compressor_new(bbstreamer *next, pg_compress_specification *compress)
 {
 #ifdef USE_LZ4
 	bbstreamer_lz4_frame   *streamer;
@@ -88,7 +88,7 @@ bbstreamer_lz4_compressor_new(bbstreamer *next, bc_specification *compress)
 	prefs = &streamer->prefs;
 	memset(prefs, 0, sizeof(LZ4F_preferences_t));
 	prefs->frameInfo.blockSizeID = LZ4F_max256KB;
-	if ((compress->options & BACKUP_COMPRESSION_OPTION_LEVEL) != 0)
+	if ((compress->options & PG_COMPRESSION_OPTION_LEVEL) != 0)
 		prefs->compressionLevel = compress->level;
 
 	ctxError = LZ4F_createCompressionContext(&streamer->cctx, LZ4F_VERSION);
diff --git a/src/bin/pg_basebackup/bbstreamer_zstd.c b/src/bin/pg_basebackup/bbstreamer_zstd.c
index a5167e9fea..8a839145a6 100644
--- a/src/bin/pg_basebackup/bbstreamer_zstd.c
+++ b/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, bc_specification *compress)
+bbstreamer_zstd_compressor_new(bbstreamer *next, pg_compress_specification *compress)
 {
 #ifdef USE_ZSTD
 	bbstreamer_zstd_frame *streamer;
@@ -85,7 +85,7 @@ bbstreamer_zstd_compressor_new(bbstreamer *next, bc_specification *compress)
 		pg_fatal("could not create zstd compression context");
 
 	/* Set compression level, if specified */
-	if ((compress->options & BACKUP_COMPRESSION_OPTION_LEVEL) != 0)
+	if ((compress->options & PG_COMPRESSION_OPTION_LEVEL) != 0)
 	{
 		ret = ZSTD_CCtx_setParameter(streamer->cctx, ZSTD_c_compressionLevel,
 									 compress->level);
@@ -95,7 +95,7 @@ bbstreamer_zstd_compressor_new(bbstreamer *next, bc_specification *compress)
 	}
 
 	/* Set # of workers, if specified */
-	if ((compress->options & BACKUP_COMPRESSION_OPTION_WORKERS) != 0)
+	if ((compress->options & PG_COMPRESSION_OPTION_WORKERS) != 0)
 	{
 		/*
 		 * On older versions of libzstd, this option does not exist, and
diff --git a/src/bin/pg_basebackup/nls.mk b/src/bin/pg_basebackup/nls.mk
index cf8570ce18..298a6e02f0 100644
--- a/src/bin/pg_basebackup/nls.mk
+++ b/src/bin/pg_basebackup/nls.mk
@@ -14,7 +14,7 @@ GETTEXT_FILES    = $(FRONTEND_COMMON_GETTEXT_FILES) \
                    receivelog.c \
                    streamutil.c \
                    walmethods.c \
-                   ../../common/backup_compression.c \
+                   ../../common/compression.c \
                    ../../common/fe_memutils.c \
                    ../../common/file_utils.c \
                    ../../fe_utils/recovery_gen.c
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 65dcfff0a0..2df86f746b 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -29,7 +29,7 @@
 
 #include "access/xlog_internal.h"
 #include "bbstreamer.h"
-#include "common/backup_compression.h"
+#include "common/compression.h"
 #include "common/file_perm.h"
 #include "common/file_utils.h"
 #include "common/logging.h"
@@ -58,7 +58,7 @@ typedef struct TablespaceList
 typedef struct ArchiveStreamState
 {
 	int			tablespacenum;
-	bc_specification   *compress;
+	pg_compress_specification   *compress;
 	bbstreamer *streamer;
 	bbstreamer *manifest_inject_streamer;
 	PQExpBuffer manifest_buffer;
@@ -198,7 +198,7 @@ static bbstreamer *CreateBackupStreamer(char *archive_name, char *spclocation,
 										bbstreamer **manifest_inject_streamer_p,
 										bool is_recovery_guc_supported,
 										bool expect_unterminated_tarfile,
-										bc_specification *compress);
+										pg_compress_specification *compress);
 static void ReceiveArchiveStreamChunk(size_t r, char *copybuf,
 									  void *callback_data);
 static char GetCopyDataByte(size_t r, char *copybuf, size_t *cursor);
@@ -207,7 +207,7 @@ static uint64 GetCopyDataUInt64(size_t r, char *copybuf, size_t *cursor);
 static void GetCopyDataEnd(size_t r, char *copybuf, size_t cursor);
 static void ReportCopyDataParseError(size_t r, char *copybuf);
 static void ReceiveTarFile(PGconn *conn, char *archive_name, char *spclocation,
-						   bool tablespacenum, bc_specification *compress);
+						   bool tablespacenum, pg_compress_specification *compress);
 static void ReceiveTarCopyChunk(size_t r, char *copybuf, void *callback_data);
 static void ReceiveBackupManifest(PGconn *conn);
 static void ReceiveBackupManifestChunk(size_t r, char *copybuf,
@@ -217,7 +217,7 @@ static void ReceiveBackupManifestInMemoryChunk(size_t r, char *copybuf,
 											   void *callback_data);
 static void BaseBackup(char *compression_algorithm, char *compression_detail,
 					   CompressionLocation compressloc,
-					   bc_specification *client_compress);
+					   pg_compress_specification *client_compress);
 
 static bool reached_end_position(XLogRecPtr segendpos, uint32 timeline,
 								 bool segment_finished);
@@ -1077,7 +1077,7 @@ CreateBackupStreamer(char *archive_name, char *spclocation,
 					 bbstreamer **manifest_inject_streamer_p,
 					 bool is_recovery_guc_supported,
 					 bool expect_unterminated_tarfile,
-					 bc_specification *compress)
+					 pg_compress_specification *compress)
 {
 	bbstreamer *streamer = NULL;
 	bbstreamer *manifest_inject_streamer = NULL;
@@ -1193,23 +1193,23 @@ CreateBackupStreamer(char *archive_name, char *spclocation,
 			archive_file = NULL;
 		}
 
-		if (compress->algorithm == BACKUP_COMPRESSION_NONE)
+		if (compress->algorithm == PG_COMPRESSION_NONE)
 			streamer = bbstreamer_plain_writer_new(archive_filename,
 												   archive_file);
-		else if (compress->algorithm == BACKUP_COMPRESSION_GZIP)
+		else if (compress->algorithm == PG_COMPRESSION_GZIP)
 		{
 			strlcat(archive_filename, ".gz", sizeof(archive_filename));
 			streamer = bbstreamer_gzip_writer_new(archive_filename,
 												  archive_file, compress);
 		}
-		else if (compress->algorithm == BACKUP_COMPRESSION_LZ4)
+		else if (compress->algorithm == PG_COMPRESSION_LZ4)
 		{
 			strlcat(archive_filename, ".lz4", sizeof(archive_filename));
 			streamer = bbstreamer_plain_writer_new(archive_filename,
 												   archive_file);
 			streamer = bbstreamer_lz4_compressor_new(streamer, compress);
 		}
-		else if (compress->algorithm == BACKUP_COMPRESSION_ZSTD)
+		else if (compress->algorithm == PG_COMPRESSION_ZSTD)
 		{
 			strlcat(archive_filename, ".zst", sizeof(archive_filename));
 			streamer = bbstreamer_plain_writer_new(archive_filename,
@@ -1288,7 +1288,7 @@ CreateBackupStreamer(char *archive_name, char *spclocation,
  * manifest if present - as a single COPY stream.
  */
 static void
-ReceiveArchiveStream(PGconn *conn, bc_specification *compress)
+ReceiveArchiveStream(PGconn *conn, pg_compress_specification *compress)
 {
 	ArchiveStreamState state;
 
@@ -1604,7 +1604,7 @@ ReportCopyDataParseError(size_t r, char *copybuf)
  */
 static void
 ReceiveTarFile(PGconn *conn, char *archive_name, char *spclocation,
-			   bool tablespacenum, bc_specification *compress)
+			   bool tablespacenum, pg_compress_specification *compress)
 {
 	WriteTarState state;
 	bbstreamer *manifest_inject_streamer;
@@ -1758,7 +1758,7 @@ ReceiveBackupManifestInMemoryChunk(size_t r, char *copybuf,
 
 static void
 BaseBackup(char *compression_algorithm, char *compression_detail,
-		   CompressionLocation compressloc, bc_specification *client_compress)
+		   CompressionLocation compressloc, pg_compress_specification *client_compress)
 {
 	PGresult   *res;
 	char	   *sysidentifier;
@@ -2025,11 +2025,11 @@ BaseBackup(char *compression_algorithm, char *compression_detail,
 		if (verbose)
 			pg_log_info("starting background WAL receiver");
 
-		if (client_compress->algorithm == BACKUP_COMPRESSION_GZIP)
+		if (client_compress->algorithm == PG_COMPRESSION_GZIP)
 		{
 			wal_compress_method = COMPRESSION_GZIP;
 			wal_compress_level =
-				(client_compress->options & BACKUP_COMPRESSION_OPTION_LEVEL)
+				(client_compress->options & PG_COMPRESSION_OPTION_LEVEL)
 				!= 0 ? client_compress->level : 0;
 		}
 		else
@@ -2316,7 +2316,7 @@ main(int argc, char **argv)
 	char	   *compression_algorithm = "none";
 	char	   *compression_detail = NULL;
 	CompressionLocation	compressloc = COMPRESS_LOCATION_UNSPECIFIED;
-	bc_specification	client_compress;
+	pg_compress_specification	client_compress;
 
 	pg_logging_init(argv[0]);
 	progname = get_progname(argv[0]);
@@ -2558,15 +2558,15 @@ main(int argc, char **argv)
 	 */
 	if (compressloc == COMPRESS_LOCATION_CLIENT)
 	{
-		bc_algorithm	alg;
+		pg_compress_algorithm	alg;
 		char	   *error_detail;
 
-		if (!parse_bc_algorithm(compression_algorithm, &alg))
+		if (!parse_compress_algorithm(compression_algorithm, &alg))
 			pg_fatal("unrecognized compression algorithm \"%s\"",
 					 compression_algorithm);
 
-		parse_bc_specification(alg, compression_detail, &client_compress);
-		error_detail = validate_bc_specification(&client_compress);
+		parse_compress_specification(alg, compression_detail, &client_compress);
+		error_detail = validate_compress_specification(&client_compress);
 		if (error_detail != NULL)
 			pg_fatal("invalid compression specification: %s",
 					 error_detail);
@@ -2574,7 +2574,7 @@ main(int argc, char **argv)
 	else
 	{
 		Assert(compressloc == COMPRESS_LOCATION_SERVER);
-		client_compress.algorithm = BACKUP_COMPRESSION_NONE;
+		client_compress.algorithm = PG_COMPRESSION_NONE;
 		client_compress.options = 0;
 	}
 
@@ -2593,7 +2593,7 @@ main(int argc, char **argv)
 	 * Client-side compression doesn't make sense unless tar format is in use.
 	 */
 	if (format == 'p' && compressloc == COMPRESS_LOCATION_CLIENT &&
-		client_compress.algorithm != BACKUP_COMPRESSION_NONE)
+		client_compress.algorithm != PG_COMPRESSION_NONE)
 	{
 		pg_log_error("only tar mode backups can be compressed");
 		pg_log_error_hint("Try \"%s --help\" for more information.", progname);
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index de8676d339..46904fee0e 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -124,7 +124,7 @@ sub mkvcbuild
 	}
 
 	our @pgcommonallfiles = qw(
-	  archive.c backup_compression.c base64.c checksum_helper.c
+	  archive.c base64.c checksum_helper.c compression.c
 	  config_info.c controldata_utils.c d2s.c encnames.c exec.c
 	  f2s.c file_perm.c file_utils.c hashfn.c ip.c jsonapi.c
 	  keywords.c kwlookup.c link-canary.c md5_common.c
-- 
2.35.1

v1-0002-Replace-WalCompressionMethod-by-common-pg_compres.patchtext/x-diff; charset=us-asciiDownload
From 2a96bd519139a2eaebc993c58229386f2e525996 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Mon, 11 Apr 2022 13:28:43 +0900
Subject: [PATCH v1 2/3] Replace WalCompressionMethod by common
 pg_compress_algorithm

The same structure with the same set of elements, exists in
compression.h, so let's make use of the centralized version.  Some of
the variables based previously on WalCompressionMethod are renamed to
stick better with the new naming.
---
 src/bin/pg_basebackup/pg_basebackup.c | 18 ++---
 src/bin/pg_basebackup/pg_receivewal.c | 44 ++++++------
 src/bin/pg_basebackup/receivelog.c    |  2 +-
 src/bin/pg_basebackup/walmethods.c    | 96 +++++++++++++--------------
 src/bin/pg_basebackup/walmethods.h    | 16 ++---
 5 files changed, 84 insertions(+), 92 deletions(-)

diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 2df86f746b..09ae27089d 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -520,7 +520,7 @@ typedef struct
 	char		xlog[MAXPGPATH];	/* directory or tarfile depending on mode */
 	char	   *sysidentifier;
 	int			timeline;
-	WalCompressionMethod	wal_compress_method;
+	pg_compress_algorithm	wal_compress_algo;
 	int			wal_compress_level;
 } logstreamer_param;
 
@@ -550,11 +550,11 @@ LogStreamerMain(logstreamer_param *param)
 	stream.replication_slot = replication_slot;
 	if (format == 'p')
 		stream.walmethod = CreateWalDirectoryMethod(param->xlog,
-													COMPRESSION_NONE, 0,
+													PG_COMPRESSION_NONE, 0,
 													stream.do_sync);
 	else
 		stream.walmethod = CreateWalTarMethod(param->xlog,
-											  param->wal_compress_method,
+											  param->wal_compress_algo,
 											  param->wal_compress_level,
 											  stream.do_sync);
 
@@ -602,7 +602,7 @@ LogStreamerMain(logstreamer_param *param)
  */
 static void
 StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier,
-				 WalCompressionMethod wal_compress_method,
+				 pg_compress_algorithm wal_compress_algo,
 				 int wal_compress_level)
 {
 	logstreamer_param *param;
@@ -613,7 +613,7 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier,
 	param = pg_malloc0(sizeof(logstreamer_param));
 	param->timeline = timeline;
 	param->sysidentifier = sysidentifier;
-	param->wal_compress_method = wal_compress_method;
+	param->wal_compress_algo = wal_compress_algo;
 	param->wal_compress_level = wal_compress_level;
 
 	/* Convert the starting position */
@@ -2019,7 +2019,7 @@ BaseBackup(char *compression_algorithm, char *compression_detail,
 	 */
 	if (includewal == STREAM_WAL)
 	{
-		WalCompressionMethod	wal_compress_method;
+		pg_compress_algorithm	wal_compress_algo;
 		int		wal_compress_level;
 
 		if (verbose)
@@ -2027,19 +2027,19 @@ BaseBackup(char *compression_algorithm, char *compression_detail,
 
 		if (client_compress->algorithm == PG_COMPRESSION_GZIP)
 		{
-			wal_compress_method = COMPRESSION_GZIP;
+			wal_compress_algo = PG_COMPRESSION_GZIP;
 			wal_compress_level =
 				(client_compress->options & PG_COMPRESSION_OPTION_LEVEL)
 				!= 0 ? client_compress->level : 0;
 		}
 		else
 		{
-			wal_compress_method = COMPRESSION_NONE;
+			wal_compress_algo = PG_COMPRESSION_NONE;
 			wal_compress_level = 0;
 		}
 
 		StartLogStreamer(xlogstart, starttli, sysidentifier,
-						 wal_compress_method, wal_compress_level);
+						 wal_compress_algo, 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 23e04741fd..b371bad644 100644
--- a/src/bin/pg_basebackup/pg_receivewal.c
+++ b/src/bin/pg_basebackup/pg_receivewal.c
@@ -52,7 +52,7 @@ static bool do_drop_slot = false;
 static bool do_sync = true;
 static bool synchronous = false;
 static char *replication_slot = NULL;
-static WalCompressionMethod compression_method = COMPRESSION_NONE;
+static pg_compress_algorithm compression_algo = PG_COMPRESSION_NONE;
 static XLogRecPtr endpos = InvalidXLogRecPtr;
 
 
@@ -114,7 +114,7 @@ usage(void)
  */
 static bool
 is_xlogfilename(const char *filename, bool *ispartial,
-				WalCompressionMethod *wal_compression_method)
+				pg_compress_algorithm *wal_compression_algo)
 {
 	size_t		fname_len = strlen(filename);
 	size_t		xlog_pattern_len = strspn(filename, "0123456789ABCDEF");
@@ -127,7 +127,7 @@ is_xlogfilename(const char *filename, bool *ispartial,
 	if (fname_len == XLOG_FNAME_LEN)
 	{
 		*ispartial = false;
-		*wal_compression_method = COMPRESSION_NONE;
+		*wal_compression_algo = PG_COMPRESSION_NONE;
 		return true;
 	}
 
@@ -136,7 +136,7 @@ is_xlogfilename(const char *filename, bool *ispartial,
 		strcmp(filename + XLOG_FNAME_LEN, ".gz") == 0)
 	{
 		*ispartial = false;
-		*wal_compression_method = COMPRESSION_GZIP;
+		*wal_compression_algo = PG_COMPRESSION_GZIP;
 		return true;
 	}
 
@@ -145,7 +145,7 @@ is_xlogfilename(const char *filename, bool *ispartial,
 		strcmp(filename + XLOG_FNAME_LEN, ".lz4") == 0)
 	{
 		*ispartial = false;
-		*wal_compression_method = COMPRESSION_LZ4;
+		*wal_compression_algo = PG_COMPRESSION_LZ4;
 		return true;
 	}
 
@@ -154,7 +154,7 @@ is_xlogfilename(const char *filename, bool *ispartial,
 		strcmp(filename + XLOG_FNAME_LEN, ".partial") == 0)
 	{
 		*ispartial = true;
-		*wal_compression_method = COMPRESSION_NONE;
+		*wal_compression_algo = PG_COMPRESSION_NONE;
 		return true;
 	}
 
@@ -163,7 +163,7 @@ is_xlogfilename(const char *filename, bool *ispartial,
 		strcmp(filename + XLOG_FNAME_LEN, ".gz.partial") == 0)
 	{
 		*ispartial = true;
-		*wal_compression_method = COMPRESSION_GZIP;
+		*wal_compression_algo = PG_COMPRESSION_GZIP;
 		return true;
 	}
 
@@ -172,7 +172,7 @@ is_xlogfilename(const char *filename, bool *ispartial,
 		strcmp(filename + XLOG_FNAME_LEN, ".lz4.partial") == 0)
 	{
 		*ispartial = true;
-		*wal_compression_method = COMPRESSION_LZ4;
+		*wal_compression_algo = PG_COMPRESSION_LZ4;
 		return true;
 	}
 
@@ -279,11 +279,11 @@ FindStreamingStart(uint32 *tli)
 	{
 		uint32		tli;
 		XLogSegNo	segno;
-		WalCompressionMethod wal_compression_method;
+		pg_compress_algorithm wal_compression_algo;
 		bool		ispartial;
 
 		if (!is_xlogfilename(dirent->d_name,
-							 &ispartial, &wal_compression_method))
+							 &ispartial, &wal_compression_algo))
 			continue;
 
 		/*
@@ -309,7 +309,7 @@ FindStreamingStart(uint32 *tli)
 		 * where WAL segments could have been compressed by a different source
 		 * than pg_receivewal, like an archive_command with lz4.
 		 */
-		if (!ispartial && wal_compression_method == COMPRESSION_NONE)
+		if (!ispartial && wal_compression_algo == PG_COMPRESSION_NONE)
 		{
 			struct stat statbuf;
 			char		fullpath[MAXPGPATH * 2];
@@ -325,7 +325,7 @@ FindStreamingStart(uint32 *tli)
 				continue;
 			}
 		}
-		else if (!ispartial && wal_compression_method == COMPRESSION_GZIP)
+		else if (!ispartial && wal_compression_algo == PG_COMPRESSION_GZIP)
 		{
 			int			fd;
 			char		buf[4];
@@ -364,7 +364,7 @@ FindStreamingStart(uint32 *tli)
 				continue;
 			}
 		}
-		else if (!ispartial && wal_compression_method == COMPRESSION_LZ4)
+		else if (!ispartial && wal_compression_algo == PG_COMPRESSION_LZ4)
 		{
 #ifdef USE_LZ4
 #define LZ4_CHUNK_SZ	64 * 1024	/* 64kB as maximum chunk size read */
@@ -590,7 +590,7 @@ StreamLog(void)
 	stream.do_sync = do_sync;
 	stream.mark_done = false;
 	stream.walmethod = CreateWalDirectoryMethod(basedir,
-												compression_method,
+												compression_algo,
 												compresslevel,
 												stream.do_sync);
 	stream.partial_suffix = ".partial";
@@ -750,11 +750,11 @@ main(int argc, char **argv)
 				break;
 			case 6:
 				if (pg_strcasecmp(optarg, "gzip") == 0)
-					compression_method = COMPRESSION_GZIP;
+					compression_algo = PG_COMPRESSION_GZIP;
 				else if (pg_strcasecmp(optarg, "lz4") == 0)
-					compression_method = COMPRESSION_LZ4;
+					compression_algo = PG_COMPRESSION_LZ4;
 				else if (pg_strcasecmp(optarg, "none") == 0)
-					compression_method = COMPRESSION_NONE;
+					compression_algo = PG_COMPRESSION_NONE;
 				else
 					pg_fatal("invalid value \"%s\" for option %s",
 							 optarg, "--compression-method");
@@ -814,9 +814,9 @@ main(int argc, char **argv)
 	/*
 	 * Compression-related options.
 	 */
-	switch (compression_method)
+	switch (compression_algo)
 	{
-		case COMPRESSION_NONE:
+		case PG_COMPRESSION_NONE:
 			if (compresslevel != 0)
 			{
 				pg_log_error("cannot use --compress with --compression-method=%s",
@@ -825,7 +825,7 @@ main(int argc, char **argv)
 				exit(1);
 			}
 			break;
-		case COMPRESSION_GZIP:
+		case PG_COMPRESSION_GZIP:
 #ifdef HAVE_LIBZ
 			if (compresslevel == 0)
 			{
@@ -837,7 +837,7 @@ main(int argc, char **argv)
 					 "gzip");
 #endif
 			break;
-		case COMPRESSION_LZ4:
+		case PG_COMPRESSION_LZ4:
 #ifdef USE_LZ4
 			if (compresslevel != 0)
 			{
@@ -851,7 +851,7 @@ main(int argc, char **argv)
 					 "LZ4");
 #endif
 			break;
-		case COMPRESSION_ZSTD:
+		case PG_COMPRESSION_ZSTD:
 			pg_fatal("compression with %s is not yet supported", "ZSTD");
 			break;
 	}
diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c
index 42d50931d3..e122f4099b 100644
--- a/src/bin/pg_basebackup/receivelog.c
+++ b/src/bin/pg_basebackup/receivelog.c
@@ -114,7 +114,7 @@ open_walfile(StreamCtl *stream, XLogRecPtr startpoint)
 	 * When streaming to tar, no file with this name will exist before, so we
 	 * never have to verify a size.
 	 */
-	if (stream->walmethod->compression_method() == COMPRESSION_NONE &&
+	if (stream->walmethod->compression_algo() == PG_COMPRESSION_NONE &&
 		stream->walmethod->existsfile(fn))
 	{
 		size = stream->walmethod->get_file_size(fn);
diff --git a/src/bin/pg_basebackup/walmethods.c b/src/bin/pg_basebackup/walmethods.c
index acd242d2c9..2412498fed 100644
--- a/src/bin/pg_basebackup/walmethods.c
+++ b/src/bin/pg_basebackup/walmethods.c
@@ -49,7 +49,7 @@
 typedef struct DirectoryMethodData
 {
 	char	   *basedir;
-	WalCompressionMethod compression_method;
+	pg_compress_algorithm compression_algo;
 	int			compression_level;
 	bool		sync;
 	const char *lasterrstring;	/* if set, takes precedence over lasterrno */
@@ -97,8 +97,8 @@ dir_get_file_name(const char *pathname, const char *temp_suffix)
 
 	snprintf(filename, MAXPGPATH, "%s%s%s",
 			 pathname,
-			 dir_data->compression_method == COMPRESSION_GZIP ? ".gz" :
-			 dir_data->compression_method == COMPRESSION_LZ4 ? ".lz4" : "",
+			 dir_data->compression_algo == PG_COMPRESSION_GZIP ? ".gz" :
+			 dir_data->compression_algo == PG_COMPRESSION_LZ4 ? ".lz4" : "",
 			 temp_suffix ? temp_suffix : "");
 
 	return filename;
@@ -141,7 +141,7 @@ dir_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
 	}
 
 #ifdef HAVE_LIBZ
-	if (dir_data->compression_method == COMPRESSION_GZIP)
+	if (dir_data->compression_algo == PG_COMPRESSION_GZIP)
 	{
 		gzfp = gzdopen(fd, "wb");
 		if (gzfp == NULL)
@@ -161,7 +161,7 @@ dir_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
 	}
 #endif
 #ifdef USE_LZ4
-	if (dir_data->compression_method == COMPRESSION_LZ4)
+	if (dir_data->compression_algo == PG_COMPRESSION_LZ4)
 	{
 		size_t		ctx_out;
 		size_t		header_size;
@@ -202,7 +202,7 @@ dir_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
 #endif
 
 	/* Do pre-padding on non-compressed files */
-	if (pad_to_size && dir_data->compression_method == COMPRESSION_NONE)
+	if (pad_to_size && dir_data->compression_algo == PG_COMPRESSION_NONE)
 	{
 		PGAlignedXLogBlock zerobuf;
 		int			bytes;
@@ -241,12 +241,12 @@ dir_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
 		{
 			dir_data->lasterrno = errno;
 #ifdef HAVE_LIBZ
-			if (dir_data->compression_method == COMPRESSION_GZIP)
+			if (dir_data->compression_algo == PG_COMPRESSION_GZIP)
 				gzclose(gzfp);
 			else
 #endif
 #ifdef USE_LZ4
-			if (dir_data->compression_method == COMPRESSION_LZ4)
+			if (dir_data->compression_algo == PG_COMPRESSION_LZ4)
 			{
 				(void) LZ4F_compressEnd(ctx, lz4buf, lz4bufsize, NULL);
 				(void) LZ4F_freeCompressionContext(ctx);
@@ -262,11 +262,11 @@ dir_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
 
 	f = pg_malloc0(sizeof(DirectoryMethodFile));
 #ifdef HAVE_LIBZ
-	if (dir_data->compression_method == COMPRESSION_GZIP)
+	if (dir_data->compression_algo == PG_COMPRESSION_GZIP)
 		f->gzfp = gzfp;
 #endif
 #ifdef USE_LZ4
-	if (dir_data->compression_method == COMPRESSION_LZ4)
+	if (dir_data->compression_algo == PG_COMPRESSION_LZ4)
 	{
 		f->ctx = ctx;
 		f->lz4buf = lz4buf;
@@ -294,7 +294,7 @@ dir_write(Walfile f, const void *buf, size_t count)
 	dir_clear_error();
 
 #ifdef HAVE_LIBZ
-	if (dir_data->compression_method == COMPRESSION_GZIP)
+	if (dir_data->compression_algo == PG_COMPRESSION_GZIP)
 	{
 		errno = 0;
 		r = (ssize_t) gzwrite(df->gzfp, buf, count);
@@ -307,7 +307,7 @@ dir_write(Walfile f, const void *buf, size_t count)
 	else
 #endif
 #ifdef USE_LZ4
-	if (dir_data->compression_method == COMPRESSION_LZ4)
+	if (dir_data->compression_algo == PG_COMPRESSION_LZ4)
 	{
 		size_t		chunk;
 		size_t		remaining;
@@ -387,7 +387,7 @@ dir_close(Walfile f, WalCloseMethod method)
 	dir_clear_error();
 
 #ifdef HAVE_LIBZ
-	if (dir_data->compression_method == COMPRESSION_GZIP)
+	if (dir_data->compression_algo == PG_COMPRESSION_GZIP)
 	{
 		errno = 0;				/* in case gzclose() doesn't set it */
 		r = gzclose(df->gzfp);
@@ -395,7 +395,7 @@ dir_close(Walfile f, WalCloseMethod method)
 	else
 #endif
 #ifdef USE_LZ4
-	if (dir_data->compression_method == COMPRESSION_LZ4)
+	if (dir_data->compression_algo == PG_COMPRESSION_LZ4)
 	{
 		size_t		compressed;
 
@@ -514,7 +514,7 @@ dir_sync(Walfile f)
 		return 0;
 
 #ifdef HAVE_LIBZ
-	if (dir_data->compression_method == COMPRESSION_GZIP)
+	if (dir_data->compression_algo == PG_COMPRESSION_GZIP)
 	{
 		if (gzflush(((DirectoryMethodFile *) f)->gzfp, Z_SYNC_FLUSH) != Z_OK)
 		{
@@ -524,7 +524,7 @@ dir_sync(Walfile f)
 	}
 #endif
 #ifdef USE_LZ4
-	if (dir_data->compression_method == COMPRESSION_LZ4)
+	if (dir_data->compression_algo == PG_COMPRESSION_LZ4)
 	{
 		DirectoryMethodFile *df = (DirectoryMethodFile *) f;
 		size_t		compressed;
@@ -571,10 +571,10 @@ dir_get_file_size(const char *pathname)
 	return statbuf.st_size;
 }
 
-static WalCompressionMethod
-dir_compression_method(void)
+static pg_compress_algorithm
+dir_compression_algo(void)
 {
-	return dir_data->compression_method;
+	return dir_data->compression_algo;
 }
 
 static bool
@@ -618,7 +618,7 @@ dir_finish(void)
 
 WalWriteMethod *
 CreateWalDirectoryMethod(const char *basedir,
-						 WalCompressionMethod compression_method,
+						 pg_compress_algorithm compression_algo,
 						 int compression_level, bool sync)
 {
 	WalWriteMethod *method;
@@ -629,7 +629,7 @@ CreateWalDirectoryMethod(const char *basedir,
 	method->get_current_pos = dir_get_current_pos;
 	method->get_file_size = dir_get_file_size;
 	method->get_file_name = dir_get_file_name;
-	method->compression_method = dir_compression_method;
+	method->compression_algo = dir_compression_algo;
 	method->close = dir_close;
 	method->sync = dir_sync;
 	method->existsfile = dir_existsfile;
@@ -637,7 +637,7 @@ CreateWalDirectoryMethod(const char *basedir,
 	method->getlasterror = dir_getlasterror;
 
 	dir_data = pg_malloc0(sizeof(DirectoryMethodData));
-	dir_data->compression_method = compression_method;
+	dir_data->compression_algo = compression_algo;
 	dir_data->compression_level = compression_level;
 	dir_data->basedir = pg_strdup(basedir);
 	dir_data->sync = sync;
@@ -672,7 +672,7 @@ typedef struct TarMethodData
 {
 	char	   *tarfilename;
 	int			fd;
-	WalCompressionMethod compression_method;
+	pg_compress_algorithm compression_algo;
 	int			compression_level;
 	bool		sync;
 	TarMethodFile *currentfile;
@@ -759,7 +759,7 @@ tar_write(Walfile f, const void *buf, size_t count)
 	tar_clear_error();
 
 	/* Tarfile will always be positioned at the end */
-	if (tar_data->compression_method == COMPRESSION_NONE)
+	if (tar_data->compression_algo == PG_COMPRESSION_NONE)
 	{
 		errno = 0;
 		r = write(tar_data->fd, buf, count);
@@ -773,7 +773,7 @@ tar_write(Walfile f, const void *buf, size_t count)
 		return r;
 	}
 #ifdef HAVE_LIBZ
-	else if (tar_data->compression_method == COMPRESSION_GZIP)
+	else if (tar_data->compression_algo == PG_COMPRESSION_GZIP)
 	{
 		if (!tar_write_compressed_data(unconstify(void *, buf), count, false))
 			return -1;
@@ -842,7 +842,7 @@ tar_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
 		}
 
 #ifdef HAVE_LIBZ
-		if (tar_data->compression_method == COMPRESSION_GZIP)
+		if (tar_data->compression_algo == PG_COMPRESSION_GZIP)
 		{
 			tar_data->zp = (z_streamp) pg_malloc(sizeof(z_stream));
 			tar_data->zp->zalloc = Z_NULL;
@@ -893,7 +893,7 @@ tar_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
 	pg_free(tmppath);
 
 #ifdef HAVE_LIBZ
-	if (tar_data->compression_method == COMPRESSION_GZIP)
+	if (tar_data->compression_algo == PG_COMPRESSION_GZIP)
 	{
 		/* Flush existing data */
 		if (!tar_write_compressed_data(NULL, 0, true))
@@ -918,7 +918,7 @@ tar_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
 	}
 	tar_data->currentfile->currpos = 0;
 
-	if (tar_data->compression_method == COMPRESSION_NONE)
+	if (tar_data->compression_algo == PG_COMPRESSION_NONE)
 	{
 		errno = 0;
 		if (write(tar_data->fd, tar_data->currentfile->header,
@@ -932,7 +932,7 @@ tar_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
 		}
 	}
 #ifdef HAVE_LIBZ
-	else if (tar_data->compression_method == COMPRESSION_GZIP)
+	else if (tar_data->compression_algo == PG_COMPRESSION_GZIP)
 	{
 		/* Write header through the zlib APIs but with no compression */
 		if (!tar_write_compressed_data(tar_data->currentfile->header,
@@ -962,7 +962,7 @@ tar_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
 	if (pad_to_size)
 	{
 		tar_data->currentfile->pad_to_size = pad_to_size;
-		if (tar_data->compression_method == COMPRESSION_NONE)
+		if (tar_data->compression_algo == PG_COMPRESSION_NONE)
 		{
 			/* Uncompressed, so pad now */
 			if (!tar_write_padding_data(tar_data->currentfile, pad_to_size))
@@ -993,10 +993,10 @@ tar_get_file_size(const char *pathname)
 	return -1;
 }
 
-static WalCompressionMethod
-tar_compression_method(void)
+static pg_compress_algorithm
+tar_compression_algo(void)
 {
-	return tar_data->compression_method;
+	return tar_data->compression_algo;
 }
 
 static off_t
@@ -1023,7 +1023,7 @@ tar_sync(Walfile f)
 	 * Always sync the whole tarfile, because that's all we can do. This makes
 	 * no sense on compressed files, so just ignore those.
 	 */
-	if (tar_data->compression_method != COMPRESSION_NONE)
+	if (tar_data->compression_algo != PG_COMPRESSION_NONE)
 		return 0;
 
 	r = fsync(tar_data->fd);
@@ -1044,7 +1044,7 @@ tar_close(Walfile f, WalCloseMethod method)
 
 	if (method == CLOSE_UNLINK)
 	{
-		if (tar_data->compression_method != COMPRESSION_NONE)
+		if (tar_data->compression_algo != PG_COMPRESSION_NONE)
 		{
 			tar_set_error("unlink not supported with compression");
 			return -1;
@@ -1075,7 +1075,7 @@ tar_close(Walfile f, WalCloseMethod method)
 	 */
 	if (tf->pad_to_size)
 	{
-		if (tar_data->compression_method == COMPRESSION_GZIP)
+		if (tar_data->compression_algo == PG_COMPRESSION_GZIP)
 		{
 			/*
 			 * A compressed tarfile is padded on close since we cannot know
@@ -1116,7 +1116,7 @@ tar_close(Walfile f, WalCloseMethod method)
 
 
 #ifdef HAVE_LIBZ
-	if (tar_data->compression_method == COMPRESSION_GZIP)
+	if (tar_data->compression_algo == PG_COMPRESSION_GZIP)
 	{
 		/* Flush the current buffer */
 		if (!tar_write_compressed_data(NULL, 0, true))
@@ -1145,7 +1145,7 @@ tar_close(Walfile f, WalCloseMethod method)
 		tar_data->lasterrno = errno;
 		return -1;
 	}
-	if (tar_data->compression_method == COMPRESSION_NONE)
+	if (tar_data->compression_algo == PG_COMPRESSION_NONE)
 	{
 		errno = 0;
 		if (write(tar_data->fd, tf->header, TAR_BLOCK_SIZE) != TAR_BLOCK_SIZE)
@@ -1156,7 +1156,7 @@ tar_close(Walfile f, WalCloseMethod method)
 		}
 	}
 #ifdef HAVE_LIBZ
-	else if (tar_data->compression_method == COMPRESSION_GZIP)
+	else if (tar_data->compression_algo == PG_COMPRESSION_GZIP)
 	{
 		/* Turn off compression */
 		if (deflateParams(tar_data->zp, 0, 0) != Z_OK)
@@ -1230,7 +1230,7 @@ tar_finish(void)
 
 	/* A tarfile always ends with two empty blocks */
 	MemSet(zerobuf, 0, sizeof(zerobuf));
-	if (tar_data->compression_method == COMPRESSION_NONE)
+	if (tar_data->compression_algo == PG_COMPRESSION_NONE)
 	{
 		errno = 0;
 		if (write(tar_data->fd, zerobuf, sizeof(zerobuf)) != sizeof(zerobuf))
@@ -1241,7 +1241,7 @@ tar_finish(void)
 		}
 	}
 #ifdef HAVE_LIBZ
-	else if (tar_data->compression_method == COMPRESSION_GZIP)
+	else if (tar_data->compression_algo == PG_COMPRESSION_GZIP)
 	{
 		if (!tar_write_compressed_data(zerobuf, sizeof(zerobuf), false))
 			return false;
@@ -1324,18 +1324,18 @@ tar_finish(void)
 }
 
 /*
- * The argument compression_method is currently ignored. It is in place for
+ * The argument compression_algo is currently ignored. It is in place for
  * symmetry with CreateWalDirectoryMethod which uses it for distinguishing
  * between the different compression methods. CreateWalTarMethod and its family
  * of functions handle only zlib compression.
  */
 WalWriteMethod *
 CreateWalTarMethod(const char *tarbase,
-				   WalCompressionMethod compression_method,
+				   pg_compress_algorithm compression_algo,
 				   int compression_level, bool sync)
 {
 	WalWriteMethod *method;
-	const char *suffix = (compression_method == COMPRESSION_GZIP) ?
+	const char *suffix = (compression_algo == PG_COMPRESSION_GZIP) ?
 	".tar.gz" : ".tar";
 
 	method = pg_malloc0(sizeof(WalWriteMethod));
@@ -1344,7 +1344,7 @@ CreateWalTarMethod(const char *tarbase,
 	method->get_current_pos = tar_get_current_pos;
 	method->get_file_size = tar_get_file_size;
 	method->get_file_name = tar_get_file_name;
-	method->compression_method = tar_compression_method;
+	method->compression_algo = tar_compression_algo;
 	method->close = tar_close;
 	method->sync = tar_sync;
 	method->existsfile = tar_existsfile;
@@ -1355,11 +1355,11 @@ CreateWalTarMethod(const char *tarbase,
 	tar_data->tarfilename = pg_malloc0(strlen(tarbase) + strlen(suffix) + 1);
 	sprintf(tar_data->tarfilename, "%s%s", tarbase, suffix);
 	tar_data->fd = -1;
-	tar_data->compression_method = compression_method;
+	tar_data->compression_algo = compression_algo;
 	tar_data->compression_level = compression_level;
 	tar_data->sync = sync;
 #ifdef HAVE_LIBZ
-	if (compression_method == COMPRESSION_GZIP)
+	if (compression_algo == PG_COMPRESSION_GZIP)
 		tar_data->zlibOut = (char *) pg_malloc(ZLIB_OUT_SIZE + 1);
 #endif
 
@@ -1371,7 +1371,7 @@ FreeWalTarMethod(void)
 {
 	pg_free(tar_data->tarfilename);
 #ifdef HAVE_LIBZ
-	if (tar_data->compression_method == COMPRESSION_GZIP)
+	if (tar_data->compression_algo == PG_COMPRESSION_GZIP)
 		pg_free(tar_data->zlibOut);
 #endif
 	pg_free(tar_data);
diff --git a/src/bin/pg_basebackup/walmethods.h b/src/bin/pg_basebackup/walmethods.h
index ec54019cfc..15639607d3 100644
--- a/src/bin/pg_basebackup/walmethods.h
+++ b/src/bin/pg_basebackup/walmethods.h
@@ -9,6 +9,7 @@
  *-------------------------------------------------------------------------
  */
 
+#include "common/compression.h"
 
 typedef void *Walfile;
 
@@ -19,15 +20,6 @@ typedef enum
 	CLOSE_NO_RENAME
 } WalCloseMethod;
 
-/* Types of compression supported */
-typedef enum
-{
-	COMPRESSION_GZIP,
-	COMPRESSION_LZ4,
-	COMPRESSION_ZSTD,
-	COMPRESSION_NONE
-} WalCompressionMethod;
-
 /*
  * A WalWriteMethod structure represents the different methods used
  * to write the streaming WAL as it's received.
@@ -68,7 +60,7 @@ struct WalWriteMethod
 	char	   *(*get_file_name) (const char *pathname, const char *temp_suffix);
 
 	/* Returns the compression method */
-	WalCompressionMethod (*compression_method) (void);
+	pg_compress_algorithm (*compression_algo) (void);
 
 	/*
 	 * Write count number of bytes to the file, and return the number of bytes
@@ -104,10 +96,10 @@ struct WalWriteMethod
  *						   not all those required for pg_receivewal)
  */
 WalWriteMethod *CreateWalDirectoryMethod(const char *basedir,
-										 WalCompressionMethod compression_method,
+										 pg_compress_algorithm compression_algo,
 										 int compression, bool sync);
 WalWriteMethod *CreateWalTarMethod(const char *tarbase,
-								   WalCompressionMethod compression_method,
+								   pg_compress_algorithm compression_algo,
 								   int compression, bool sync);
 
 /* Cleanup routines for previously-created methods */
-- 
2.35.1

v1-0003-Rework-compression-options-of-pg_receivewal.patchtext/x-diff; charset=us-asciiDownload
From c77f99bf7d6cfe45fd5a2d4ee5029059d873d803 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Mon, 11 Apr 2022 15:33:27 +0900
Subject: [PATCH v1 3/3] Rework compression options of pg_receivewal

Since babbbb5 and the introduction of LZ4 in pg_receivewal, the
compression of archived WAL is controlled by two options:
- --compression-method with "gzip", "none" or "lz4" as possible value.
- --compress=N to specify a compression level, with a
backward-incompatible change where a value of zero leads to a failure
instead of no compression.

This commit takes advantage of the previous refactoring done to rework
the compression options of pg_receivewal:
- --compression-method is removed.
- --compress is extended to use the same grammar as pg_basebackup, as of
a METHOD:DETAIL specification, where a METHOD is "gzip", "none" or "lz4"
and a DETAIL is a comma-separated list of options, the only keyword
supported is now "level" to control the compression level.  If only an
integer is specified as value of this option, "none" is implied for 0
and "gzip" is implied.  So this brings back --compress to be
backward-compatible with ~14, while still supporting LZ4.

This has the advantage of centralizing the set of checks used by
pg_basebackup.
---
 src/bin/pg_basebackup/pg_receivewal.c        | 130 +++++++++++++------
 src/bin/pg_basebackup/t/020_pg_receivewal.pl |  16 +--
 doc/src/sgml/ref/pg_receivewal.sgml          |  47 ++++---
 3 files changed, 120 insertions(+), 73 deletions(-)

diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c
index b371bad644..a47b4a8ef7 100644
--- a/src/bin/pg_basebackup/pg_receivewal.c
+++ b/src/bin/pg_basebackup/pg_receivewal.c
@@ -57,6 +57,8 @@ static XLogRecPtr endpos = InvalidXLogRecPtr;
 
 
 static void usage(void);
+static void parse_compress_options(char *option, char **algorithm,
+								   char **detail);
 static DIR *get_destination_dir(char *dest_folder);
 static void close_destination_dir(DIR *dest_dir, char *dest_folder);
 static XLogRecPtr FindStreamingStart(uint32 *tli);
@@ -90,9 +92,8 @@ usage(void)
 	printf(_("      --synchronous      flush write-ahead log immediately after writing\n"));
 	printf(_("  -v, --verbose          output verbose messages\n"));
 	printf(_("  -V, --version          output version information, then exit\n"));
-	printf(_("      --compression-method=METHOD\n"
-			 "                         method to compress logs\n"));
-	printf(_("  -Z, --compress=1-9     compress logs with given compression level\n"));
+	printf(_("  -Z, --compress=METHOD[:DETAIL]\n"
+			 "                         compress as specified\n"));
 	printf(_("  -?, --help             show this help, then exit\n"));
 	printf(_("\nConnection options:\n"));
 	printf(_("  -d, --dbname=CONNSTR   connection string\n"));
@@ -108,6 +109,66 @@ usage(void)
 	printf(_("%s home page: <%s>\n"), PACKAGE_NAME, PACKAGE_URL);
 }
 
+/*
+ * Basic parsing of a value specified for -Z/--compress
+ *
+ * The parsing consists of a METHOD:DETAIL string fed later on to a more
+ * advanced routine in charge of proper validation checks.  This only extracts
+ * METHOD and DETAIL.  If only an integer is found, the method is implied by
+ * the value specified.
+ */
+static void
+parse_compress_options(char *option, char **algorithm, char **detail)
+{
+	char	   *sep;
+	char	   *endp;
+	long		result;
+
+	/*
+	 * Check whether the compression specification consists of a bare integer.
+	 *
+	 * For backward-compatibility, assume "none" if the integer found is zero
+	 * and "gzip" otherwise.
+	 */
+	result = strtol(option, &endp, 10);
+	if (*endp == '\0')
+	{
+		if (result == 0)
+		{
+			*algorithm = pstrdup("none");
+			*detail = NULL;
+		}
+		else
+		{
+			*algorithm = pstrdup("gzip");
+			*detail = pstrdup(option);
+		}
+		return;
+	}
+
+	/*
+	 * Check whether there is a compression detail following the algorithm
+	 * name.
+	 */
+	sep = strchr(option, ':');
+	if (sep == NULL)
+	{
+		*algorithm = pstrdup(option);
+		*detail = NULL;
+	}
+	else
+	{
+		char   *alg;
+
+		alg = palloc((sep - option) + 1);
+		memcpy(alg, option, sep - option);
+		alg[sep - option] = '\0';
+
+		*algorithm = alg;
+		*detail = pstrdup(sep + 1);
+	}
+}
+
 /*
  * Check if the filename looks like a WAL file, letting caller know if this
  * WAL segment is partial and/or compressed.
@@ -651,7 +712,6 @@ main(int argc, char **argv)
 		{"if-not-exists", no_argument, NULL, 3},
 		{"synchronous", no_argument, NULL, 4},
 		{"no-sync", no_argument, NULL, 5},
-		{"compression-method", required_argument, NULL, 6},
 		{NULL, 0, NULL, 0}
 	};
 
@@ -660,6 +720,10 @@ main(int argc, char **argv)
 	char	   *db_name;
 	uint32		hi,
 				lo;
+	pg_compress_specification compression_spec;
+	char	   *compression_detail = NULL;
+	char	   *compression_algo_str = "none";
+	char	   *error_detail = NULL;
 
 	pg_logging_init(argv[0]);
 	progname = get_progname(argv[0]);
@@ -728,9 +792,8 @@ main(int argc, char **argv)
 				verbose++;
 				break;
 			case 'Z':
-				if (!option_parse_int(optarg, "-Z/--compress", 1, 9,
-									  &compresslevel))
-					exit(1);
+				parse_compress_options(optarg, &compression_algo_str,
+									   &compression_detail);
 				break;
 /* action */
 			case 1:
@@ -748,17 +811,6 @@ main(int argc, char **argv)
 			case 5:
 				do_sync = false;
 				break;
-			case 6:
-				if (pg_strcasecmp(optarg, "gzip") == 0)
-					compression_algo = PG_COMPRESSION_GZIP;
-				else if (pg_strcasecmp(optarg, "lz4") == 0)
-					compression_algo = PG_COMPRESSION_LZ4;
-				else if (pg_strcasecmp(optarg, "none") == 0)
-					compression_algo = PG_COMPRESSION_NONE;
-				else
-					pg_fatal("invalid value \"%s\" for option %s",
-							 optarg, "--compression-method");
-				break;
 			default:
 				/* getopt_long already emitted a complaint */
 				pg_log_error_hint("Try \"%s --help\" for more information.", progname);
@@ -810,24 +862,29 @@ main(int argc, char **argv)
 		exit(1);
 	}
 
-
 	/*
-	 * Compression-related options.
+	 * Compression options
 	 */
+	if (!parse_compress_algorithm(compression_algo_str, &compression_algo))
+		pg_fatal("unrecognized compression algorithm \"%s\"",
+				 compression_algo_str);
+
+	parse_compress_specification(compression_algo, compression_detail,
+								 &compression_spec);
+	error_detail = validate_compress_specification(&compression_spec);
+	if (error_detail != NULL)
+		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_algo)
 	{
-		case PG_COMPRESSION_NONE:
-			if (compresslevel != 0)
-			{
-				pg_log_error("cannot use --compress with --compression-method=%s",
-							 "none");
-				pg_log_error_hint("Try \"%s --help\" for more information.", progname);
-				exit(1);
-			}
-			break;
 		case PG_COMPRESSION_GZIP:
 #ifdef HAVE_LIBZ
-			if (compresslevel == 0)
+			if ((compression_spec.options & PG_COMPRESSION_OPTION_LEVEL) == 0)
 			{
 				pg_log_info("no value specified for --compress, switching to default");
 				compresslevel = Z_DEFAULT_COMPRESSION;
@@ -837,16 +894,11 @@ main(int argc, char **argv)
 					 "gzip");
 #endif
 			break;
+		case PG_COMPRESSION_NONE:
+			/* nothing to do */
+			break;
 		case PG_COMPRESSION_LZ4:
-#ifdef USE_LZ4
-			if (compresslevel != 0)
-			{
-				pg_log_error("cannot use --compress with --compression-method=%s",
-							 "lz4");
-				pg_log_error_hint("Try \"%s --help\" for more information.", progname);
-				exit(1);
-			}
-#else
+#ifndef USE_LZ4
 			pg_fatal("this build does not support compression with %s",
 					 "LZ4");
 #endif
diff --git a/src/bin/pg_basebackup/t/020_pg_receivewal.pl b/src/bin/pg_basebackup/t/020_pg_receivewal.pl
index 8c38816b22..57e274cd34 100644
--- a/src/bin/pg_basebackup/t/020_pg_receivewal.pl
+++ b/src/bin/pg_basebackup/t/020_pg_receivewal.pl
@@ -35,11 +35,10 @@ $primary->command_fails(
 	'failure if --synchronous specified with --no-sync');
 $primary->command_fails_like(
 	[
-		'pg_receivewal', '-D', $stream_dir, '--compression-method', 'none',
-		'--compress',    '1'
+		'pg_receivewal', '-D', $stream_dir, '--compress', 'none:1',
 	],
-	qr/\Qpg_receivewal: error: cannot use --compress with --compression-method=none/,
-	'failure if --compress specified with --compression-method=none');
+	qr/\Qpg_receivewal: error: invalid compression specification: compression algorithm "none" does not accept a compression level/,
+	'failure if --compress none:N (where N > 0)');
 
 # Slot creation and drop
 my $slot_name = 'test';
@@ -93,15 +92,12 @@ SKIP:
 	chomp($nextlsn);
 	$primary->psql('postgres', 'INSERT INTO test_table VALUES (2);');
 
-	# Note the trailing whitespace after the value of --compress, that is
-	# a valid value.
 	$primary->command_ok(
 		[
 			'pg_receivewal',        '-D',
 			$stream_dir,            '--verbose',
 			'--endpos',             $nextlsn,
-			'--compression-method', 'gzip',
-			'--compress',           '1 ',
+			'--compress',           'gzip:1',
 			'--no-loop'
 		],
 		"streaming some WAL using ZLIB compression");
@@ -156,10 +152,10 @@ SKIP:
 			'pg_receivewal', '-D',
 			$stream_dir,     '--verbose',
 			'--endpos',      $nextlsn,
-			'--no-loop',     '--compression-method',
+			'--no-loop',     '--compress',
 			'lz4'
 		],
-		'streaming some WAL using --compression-method=lz4');
+		'streaming some WAL using --compress=lz4');
 
 	# Verify that the stored files are generated with their expected
 	# names.
diff --git a/doc/src/sgml/ref/pg_receivewal.sgml b/doc/src/sgml/ref/pg_receivewal.sgml
index b846213fb7..4fe9e1a874 100644
--- a/doc/src/sgml/ref/pg_receivewal.sgml
+++ b/doc/src/sgml/ref/pg_receivewal.sgml
@@ -263,15 +263,32 @@ PostgreSQL documentation
      </varlistentry>
 
      <varlistentry>
-      <term><option>--compression-method=<replaceable class="parameter">method</replaceable></option></term>
+      <term><option>-Z <replaceable class="parameter">level</replaceable></option></term>
+      <term><option>-Z <replaceable class="parameter">method</replaceable>[:<replaceable>detail</replaceable>]</option></term>
+      <term><option>--compress=<replaceable class="parameter">level</replaceable></option></term>
+      <term><option>--compress=<replaceable class="parameter">method</replaceable>[:<replaceable>detail</replaceable>]</option></term>
       <listitem>
        <para>
-        Enables compression of write-ahead logs using the specified method.
-        Supported values are <literal>gzip</literal>, <literal>lz4</literal>
-        (if <productname>PostgreSQL</productname> was compiled with
-        <option>--with-lz4</option>), and <literal>none</literal>.
+        Enables compression of write-ahead logs.
+       </para>
+       <para>
+        The compression method can be set to <literal>gzip</literal>,
+        <literal>lz4</literal> (if <productname>PostgreSQL</productname>
+        was compiled with <option>--with-lz4</option>) 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
+        form <literal>keyword</literal> or <literal>keyword=value</literal>.
+        Currently, the only supported keyword is <literal>level</literal>.
+       </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>
         The suffix <filename>.gz</filename> will automatically be added to
         all filenames when using <literal>gzip</literal>, and the suffix
@@ -279,24 +296,6 @@ PostgreSQL documentation
        </para>
       </listitem>
      </varlistentry>
-
-     <varlistentry>
-      <term><option>-Z <replaceable class="parameter">level</replaceable></option></term>
-      <term><option>--compress=<replaceable class="parameter">level</replaceable></option></term>
-      <listitem>
-       <para>
-        Specifies the compression level (<literal>1</literal> through
-        <literal>9</literal>, <literal>1</literal> being worst compression
-        and <literal>9</literal> being best compression) for WAL segments
-        compressed with <application>gzip</application>.
-       </para>
-
-       <para>
-        This option requires <option>--compression-method</option> to be
-        specified with <literal>gzip</literal>.
-       </para>
-      </listitem>
-     </varlistentry>
     </variablelist>
 
    <para>
-- 
2.35.1

#2Noname
gkokolatos@pm.me
In reply to: Michael Paquier (#1)
Re: Fixes for compression options of pg_receivewal and refactoring of backup_compression.{c,h}

On Monday, April 11th, 2022 at 8:52 AM, Michael Paquier <michael@paquier.xyz>
wrote:

This is something I think we had better fix before beta1, because now
we have binaries that use an inconsistent set of options. So,
attached is a patch set aimed at rework this option set from the
ground, taking advantage of the recent work done by Robert and others
for pg_basebackup:

Agreed. It is rather inconsistent now.

- 0001 is a simple rename of backup_compression.{c,h} to
compression.{c,h}, removing anything related to base backups from
that. One extra reason behind this renaming is that I would like to
use this infra for pg_dump, but that's material for 16~.

I agree with the design. If you permit me a couple of nitpicks regarding naming.

+typedef enum pg_compress_algorithm
+{
+   PG_COMPRESSION_NONE,
+   PG_COMPRESSION_GZIP,
+   PG_COMPRESSION_LZ4,
+   PG_COMPRESSION_ZSTD
+} pg_compress_algorithm;

Elsewhere in the codebase, (e.g. create_table.sgml, alter_table.sgml,
brin_tuple.c, detoast.c, toast_compression.c, tupdesc.c, gist.c to mention a
few) variations of of the nomenclature "compression method" are used, like
'VARDATA_COMPRESSED_GET_COMPRESS_METHOD' or 'InvalidCompressionMethod' etc. I
feel that it would be nicer if we followed one naming rule for this and I
recommend to substitute algorithm for method throughout.

On a similar note, it would help readability to be able to distinguish at a
glance the type from the variable. Maybe uppercase or camelcase the type?

Last, even though it is not needed now, it will be helpful to have a
PG_COMPRESSION_INVALID in some scenarios. Though we can add it when we come to
it.

- 0002 removes WalCompressionMethod, replacing it by
pg_compress_algorithm as these are the same enums. Robert complained
about the confusion that WalCompressionMethod could lead to as this
could be used for the compression of data, and not only WAL. I have
renamed some variables to be more consistent, while on it.

It looks good. If you choose to discard the comment regarding the use of
'method' over 'algorithm' from above, can you please use the full word in the
variable, e.g. 'wal_compress_algorithm' instead of 'wal_compress_algo'. I can
not really explain it, the later reads a bit rude. Then again that may be just
me.

- 0003 is the actual option rework for pg_receivewal. This takes
advantage of 0001, leading to the result of removing --compress-method
and replacing it with --compress, taking care of the backward
compatibility problems for an integer value, aka 0 implies no
compression and val > 0 implies gzip. One bonus reason to switch to
that is that this would make the addition of zstd for pg_receivewal
easier in the future.

Looks good.

I am going to add an open item for this stuff. Comments or thoughts?

I agree that it is better to not release pg_receivewal with the distinct set of
options.

Cheers,
//Georgios

Show quoted text

Thanks,
--
Michael

#3Robert Haas
robertmhaas@gmail.com
In reply to: Michael Paquier (#1)
Re: Fixes for compression options of pg_receivewal and refactoring of backup_compression.{c,h}

On Mon, Apr 11, 2022 at 2:52 AM Michael Paquier <michael@paquier.xyz> wrote:

Since babbbb5 and the introduction of LZ4, I have reworked the way
compression is controlled for pg_receivewal, with two options:
- --compress-method, settable to "gzip", "none" or "lz4".
- --compress, to pass down a compression level, where the allowed
range is 1-9. If passing down 0, we'd get an error rather than
implying no compression, contrary to what we did in ~14.

I initially thought that this was fine as-is, but then Robert and
others have worked on client/server compression for pg_basebackup,
introducing a much better design with centralized APIs where one can
use METHOD:DETAIL for as compression value, where DETAIL is a
comma-separated list of keyword=value (keyword = "level" or
"workers"), with centralized checks and an extensible design.

This is something I think we had better fix before beta1, because now
we have binaries that use an inconsistent set of options. So,
attached is a patch set aimed at rework this option set from the
ground, taking advantage of the recent work done by Robert and others
for pg_basebackup:

+1 for this in general, but I think that naming like
"compression_algo" stinks. If you think "compression_algorithm" is too
long, I think you should use "algorithm" or "compression" or
"compression_method" or something.

--
Robert Haas
EDB: http://www.enterprisedb.com

#4Michael Paquier
michael@paquier.xyz
In reply to: Robert Haas (#3)
Re: Fixes for compression options of pg_receivewal and refactoring of backup_compression.{c,h}

On Mon, Apr 11, 2022 at 11:15:46AM -0400, Robert Haas wrote:

+1 for this in general, but I think that naming like
"compression_algo" stinks. If you think "compression_algorithm" is too
long, I think you should use "algorithm" or "compression" or
"compression_method" or something.

Yes, I found "compression_algorithm" to be too long initially. For
walmethods.c and pg_receivewal.c, it may be better to just stick to
"algorithm" then, at least that's consistent with pg_basebackup.c.
--
Michael

#5Michael Paquier
michael@paquier.xyz
In reply to: Noname (#2)
1 attachment(s)
Re: Fixes for compression options of pg_receivewal and refactoring of backup_compression.{c,h}

On Mon, Apr 11, 2022 at 12:46:02PM +0000, gkokolatos@pm.me wrote:

On Monday, April 11th, 2022 at 8:52 AM, Michael Paquier <michael@paquier.xyz>
wrote:

- 0001 is a simple rename of backup_compression.{c,h} to
compression.{c,h}, removing anything related to base backups from
that. One extra reason behind this renaming is that I would like to
use this infra for pg_dump, but that's material for 16~.

I agree with the design. If you permit me a couple of nitpicks regarding naming.

+typedef enum pg_compress_algorithm
+{
+   PG_COMPRESSION_NONE,
+   PG_COMPRESSION_GZIP,
+   PG_COMPRESSION_LZ4,
+   PG_COMPRESSION_ZSTD
+} pg_compress_algorithm;

Elsewhere in the codebase, (e.g. create_table.sgml, alter_table.sgml,
brin_tuple.c, detoast.c, toast_compression.c, tupdesc.c, gist.c to mention a
few) variations of of the nomenclature "compression method" are used, like
'VARDATA_COMPRESSED_GET_COMPRESS_METHOD' or 'InvalidCompressionMethod' etc. I
feel that it would be nicer if we followed one naming rule for this and I
recommend to substitute algorithm for method throughout.

Technically and as far as I know, both are correct and hold more or
less the same meaning. pg_basebackup's code exposes algorithm in a
more extended way, so I have just stuck to it for the internal
variables and such. Perhaps we could rename the whole, but I see no
strong reason to do that.

Last, even though it is not needed now, it will be helpful to have a
PG_COMPRESSION_INVALID in some scenarios. Though we can add it when we come to
it.

Perhaps. There is no need for it yet, though. pg_dump would not need
that, as well.

- 0002 removes WalCompressionMethod, replacing it by
pg_compress_algorithm as these are the same enums. Robert complained
about the confusion that WalCompressionMethod could lead to as this
could be used for the compression of data, and not only WAL. I have
renamed some variables to be more consistent, while on it.

It looks good. If you choose to discard the comment regarding the use of
'method' over 'algorithm' from above, can you please use the full word in the
variable, e.g. 'wal_compress_algorithm' instead of 'wal_compress_algo'. I can
not really explain it, the later reads a bit rude. Then again that may be just
me.

Thanks. I have been able to do an extra pass on 0001 and 0002, fixing
those naming inconsistencies with "algo" vs "algorithm" that you and
Robert have reported, and applied them. For 0003, I'll look at it
later. Attached is a rebase with improvements about the variable
names.
--
Michael

Attachments:

v2-0001-Rework-compression-options-of-pg_receivewal.patchtext/x-diff; charset=us-asciiDownload
From 94850ac604402371135e85de27a5d9074a947d4b Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Tue, 12 Apr 2022 18:14:28 +0900
Subject: [PATCH v2] Rework compression options of pg_receivewal

Since babbbb5 and the introduction of LZ4 in pg_receivewal, the
compression of archived WAL is controlled by two options:
- --compression-method with "gzip", "none" or "lz4" as possible value.
- --compress=N to specify a compression level, with a
backward-incompatible change where a value of zero leads to a failure
instead of no compression.

This commit takes advantage of the previous refactoring done to rework
the compression options of pg_receivewal:
- --compression-method is removed.
- --compress is extended to use the same grammar as pg_basebackup, as of
a METHOD:DETAIL specification, where a METHOD is "gzip", "none" or "lz4"
and a DETAIL is a comma-separated list of options, the only keyword
supported is now "level" to control the compression level.  If only an
integer is specified as value of this option, "none" is implied for 0
and "gzip" is implied.  So this brings back --compress to be
backward-compatible with ~14, while still supporting LZ4.

This has the advantage of centralizing the set of checks used by
pg_basebackup.
---
 src/bin/pg_basebackup/pg_receivewal.c        | 131 +++++++++++++------
 src/bin/pg_basebackup/t/020_pg_receivewal.pl |  16 +--
 doc/src/sgml/ref/pg_receivewal.sgml          |  47 ++++---
 3 files changed, 121 insertions(+), 73 deletions(-)

diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c
index ede1d4648d..f942883332 100644
--- a/src/bin/pg_basebackup/pg_receivewal.c
+++ b/src/bin/pg_basebackup/pg_receivewal.c
@@ -57,6 +57,8 @@ static XLogRecPtr endpos = InvalidXLogRecPtr;
 
 
 static void usage(void);
+static void parse_compress_options(char *option, char **algorithm,
+								   char **detail);
 static DIR *get_destination_dir(char *dest_folder);
 static void close_destination_dir(DIR *dest_dir, char *dest_folder);
 static XLogRecPtr FindStreamingStart(uint32 *tli);
@@ -90,9 +92,8 @@ usage(void)
 	printf(_("      --synchronous      flush write-ahead log immediately after writing\n"));
 	printf(_("  -v, --verbose          output verbose messages\n"));
 	printf(_("  -V, --version          output version information, then exit\n"));
-	printf(_("      --compression-method=METHOD\n"
-			 "                         method to compress logs\n"));
-	printf(_("  -Z, --compress=1-9     compress logs with given compression level\n"));
+	printf(_("  -Z, --compress=METHOD[:DETAIL]\n"
+			 "                         compress as specified\n"));
 	printf(_("  -?, --help             show this help, then exit\n"));
 	printf(_("\nConnection options:\n"));
 	printf(_("  -d, --dbname=CONNSTR   connection string\n"));
@@ -108,6 +109,66 @@ usage(void)
 	printf(_("%s home page: <%s>\n"), PACKAGE_NAME, PACKAGE_URL);
 }
 
+/*
+ * Basic parsing of a value specified for -Z/--compress
+ *
+ * The parsing consists of a METHOD:DETAIL string fed later on to a more
+ * advanced routine in charge of proper validation checks.  This only extracts
+ * METHOD and DETAIL.  If only an integer is found, the method is implied by
+ * the value specified.
+ */
+static void
+parse_compress_options(char *option, char **algorithm, char **detail)
+{
+	char	   *sep;
+	char	   *endp;
+	long		result;
+
+	/*
+	 * Check whether the compression specification consists of a bare integer.
+	 *
+	 * For backward-compatibility, assume "none" if the integer found is zero
+	 * and "gzip" otherwise.
+	 */
+	result = strtol(option, &endp, 10);
+	if (*endp == '\0')
+	{
+		if (result == 0)
+		{
+			*algorithm = pstrdup("none");
+			*detail = NULL;
+		}
+		else
+		{
+			*algorithm = pstrdup("gzip");
+			*detail = pstrdup(option);
+		}
+		return;
+	}
+
+	/*
+	 * Check whether there is a compression detail following the algorithm
+	 * name.
+	 */
+	sep = strchr(option, ':');
+	if (sep == NULL)
+	{
+		*algorithm = pstrdup(option);
+		*detail = NULL;
+	}
+	else
+	{
+		char   *alg;
+
+		alg = palloc((sep - option) + 1);
+		memcpy(alg, option, sep - option);
+		alg[sep - option] = '\0';
+
+		*algorithm = alg;
+		*detail = pstrdup(sep + 1);
+	}
+}
+
 /*
  * Check if the filename looks like a WAL file, letting caller know if this
  * WAL segment is partial and/or compressed.
@@ -651,7 +712,6 @@ main(int argc, char **argv)
 		{"if-not-exists", no_argument, NULL, 3},
 		{"synchronous", no_argument, NULL, 4},
 		{"no-sync", no_argument, NULL, 5},
-		{"compression-method", required_argument, NULL, 6},
 		{NULL, 0, NULL, 0}
 	};
 
@@ -660,6 +720,10 @@ main(int argc, char **argv)
 	char	   *db_name;
 	uint32		hi,
 				lo;
+	pg_compress_specification compression_spec;
+	char	   *compression_detail = NULL;
+	char	   *compression_algorithm_str = "none";
+	char	   *error_detail = NULL;
 
 	pg_logging_init(argv[0]);
 	progname = get_progname(argv[0]);
@@ -728,9 +792,8 @@ main(int argc, char **argv)
 				verbose++;
 				break;
 			case 'Z':
-				if (!option_parse_int(optarg, "-Z/--compress", 1, 9,
-									  &compresslevel))
-					exit(1);
+				parse_compress_options(optarg, &compression_algorithm_str,
+									   &compression_detail);
 				break;
 /* action */
 			case 1:
@@ -748,17 +811,6 @@ main(int argc, char **argv)
 			case 5:
 				do_sync = false;
 				break;
-			case 6:
-				if (pg_strcasecmp(optarg, "gzip") == 0)
-					compression_algorithm = PG_COMPRESSION_GZIP;
-				else if (pg_strcasecmp(optarg, "lz4") == 0)
-					compression_algorithm = PG_COMPRESSION_LZ4;
-				else if (pg_strcasecmp(optarg, "none") == 0)
-					compression_algorithm = PG_COMPRESSION_NONE;
-				else
-					pg_fatal("invalid value \"%s\" for option %s",
-							 optarg, "--compression-method");
-				break;
 			default:
 				/* getopt_long already emitted a complaint */
 				pg_log_error_hint("Try \"%s --help\" for more information.", progname);
@@ -810,24 +862,30 @@ main(int argc, char **argv)
 		exit(1);
 	}
 
-
 	/*
-	 * Compression-related options.
+	 * Compression options
 	 */
+	if (!parse_compress_algorithm(compression_algorithm_str,
+								  &compression_algorithm))
+		pg_fatal("unrecognized compression algorithm \"%s\"",
+				 compression_algorithm_str);
+
+	parse_compress_specification(compression_algorithm, compression_detail,
+								 &compression_spec);
+	error_detail = validate_compress_specification(&compression_spec);
+	if (error_detail != NULL)
+		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:
-			if (compresslevel != 0)
-			{
-				pg_log_error("cannot use --compress with --compression-method=%s",
-							 "none");
-				pg_log_error_hint("Try \"%s --help\" for more information.", progname);
-				exit(1);
-			}
-			break;
 		case PG_COMPRESSION_GZIP:
 #ifdef HAVE_LIBZ
-			if (compresslevel == 0)
+			if ((compression_spec.options & PG_COMPRESSION_OPTION_LEVEL) == 0)
 			{
 				pg_log_info("no value specified for --compress, switching to default");
 				compresslevel = Z_DEFAULT_COMPRESSION;
@@ -837,16 +895,11 @@ main(int argc, char **argv)
 					 "gzip");
 #endif
 			break;
+		case PG_COMPRESSION_NONE:
+			/* nothing to do */
+			break;
 		case PG_COMPRESSION_LZ4:
-#ifdef USE_LZ4
-			if (compresslevel != 0)
-			{
-				pg_log_error("cannot use --compress with --compression-method=%s",
-							 "lz4");
-				pg_log_error_hint("Try \"%s --help\" for more information.", progname);
-				exit(1);
-			}
-#else
+#ifndef USE_LZ4
 			pg_fatal("this build does not support compression with %s",
 					 "LZ4");
 #endif
diff --git a/src/bin/pg_basebackup/t/020_pg_receivewal.pl b/src/bin/pg_basebackup/t/020_pg_receivewal.pl
index 8c38816b22..57e274cd34 100644
--- a/src/bin/pg_basebackup/t/020_pg_receivewal.pl
+++ b/src/bin/pg_basebackup/t/020_pg_receivewal.pl
@@ -35,11 +35,10 @@ $primary->command_fails(
 	'failure if --synchronous specified with --no-sync');
 $primary->command_fails_like(
 	[
-		'pg_receivewal', '-D', $stream_dir, '--compression-method', 'none',
-		'--compress',    '1'
+		'pg_receivewal', '-D', $stream_dir, '--compress', 'none:1',
 	],
-	qr/\Qpg_receivewal: error: cannot use --compress with --compression-method=none/,
-	'failure if --compress specified with --compression-method=none');
+	qr/\Qpg_receivewal: error: invalid compression specification: compression algorithm "none" does not accept a compression level/,
+	'failure if --compress none:N (where N > 0)');
 
 # Slot creation and drop
 my $slot_name = 'test';
@@ -93,15 +92,12 @@ SKIP:
 	chomp($nextlsn);
 	$primary->psql('postgres', 'INSERT INTO test_table VALUES (2);');
 
-	# Note the trailing whitespace after the value of --compress, that is
-	# a valid value.
 	$primary->command_ok(
 		[
 			'pg_receivewal',        '-D',
 			$stream_dir,            '--verbose',
 			'--endpos',             $nextlsn,
-			'--compression-method', 'gzip',
-			'--compress',           '1 ',
+			'--compress',           'gzip:1',
 			'--no-loop'
 		],
 		"streaming some WAL using ZLIB compression");
@@ -156,10 +152,10 @@ SKIP:
 			'pg_receivewal', '-D',
 			$stream_dir,     '--verbose',
 			'--endpos',      $nextlsn,
-			'--no-loop',     '--compression-method',
+			'--no-loop',     '--compress',
 			'lz4'
 		],
-		'streaming some WAL using --compression-method=lz4');
+		'streaming some WAL using --compress=lz4');
 
 	# Verify that the stored files are generated with their expected
 	# names.
diff --git a/doc/src/sgml/ref/pg_receivewal.sgml b/doc/src/sgml/ref/pg_receivewal.sgml
index b846213fb7..4fe9e1a874 100644
--- a/doc/src/sgml/ref/pg_receivewal.sgml
+++ b/doc/src/sgml/ref/pg_receivewal.sgml
@@ -263,15 +263,32 @@ PostgreSQL documentation
      </varlistentry>
 
      <varlistentry>
-      <term><option>--compression-method=<replaceable class="parameter">method</replaceable></option></term>
+      <term><option>-Z <replaceable class="parameter">level</replaceable></option></term>
+      <term><option>-Z <replaceable class="parameter">method</replaceable>[:<replaceable>detail</replaceable>]</option></term>
+      <term><option>--compress=<replaceable class="parameter">level</replaceable></option></term>
+      <term><option>--compress=<replaceable class="parameter">method</replaceable>[:<replaceable>detail</replaceable>]</option></term>
       <listitem>
        <para>
-        Enables compression of write-ahead logs using the specified method.
-        Supported values are <literal>gzip</literal>, <literal>lz4</literal>
-        (if <productname>PostgreSQL</productname> was compiled with
-        <option>--with-lz4</option>), and <literal>none</literal>.
+        Enables compression of write-ahead logs.
+       </para>
+       <para>
+        The compression method can be set to <literal>gzip</literal>,
+        <literal>lz4</literal> (if <productname>PostgreSQL</productname>
+        was compiled with <option>--with-lz4</option>) 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
+        form <literal>keyword</literal> or <literal>keyword=value</literal>.
+        Currently, the only supported keyword is <literal>level</literal>.
+       </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>
         The suffix <filename>.gz</filename> will automatically be added to
         all filenames when using <literal>gzip</literal>, and the suffix
@@ -279,24 +296,6 @@ PostgreSQL documentation
        </para>
       </listitem>
      </varlistentry>
-
-     <varlistentry>
-      <term><option>-Z <replaceable class="parameter">level</replaceable></option></term>
-      <term><option>--compress=<replaceable class="parameter">level</replaceable></option></term>
-      <listitem>
-       <para>
-        Specifies the compression level (<literal>1</literal> through
-        <literal>9</literal>, <literal>1</literal> being worst compression
-        and <literal>9</literal> being best compression) for WAL segments
-        compressed with <application>gzip</application>.
-       </para>
-
-       <para>
-        This option requires <option>--compression-method</option> to be
-        specified with <literal>gzip</literal>.
-       </para>
-      </listitem>
-     </varlistentry>
     </variablelist>
 
    <para>
-- 
2.35.1

#6Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#5)
1 attachment(s)
Re: Fixes for compression options of pg_receivewal and refactoring of backup_compression.{c,h}

On Tue, Apr 12, 2022 at 06:22:48PM +0900, Michael Paquier wrote:

On Mon, Apr 11, 2022 at 12:46:02PM +0000, gkokolatos@pm.me wrote:

It looks good. If you choose to discard the comment regarding the use of
'method' over 'algorithm' from above, can you please use the full word in the
variable, e.g. 'wal_compress_algorithm' instead of 'wal_compress_algo'. I can
not really explain it, the later reads a bit rude. Then again that may be just
me.

Thanks. I have been able to do an extra pass on 0001 and 0002, fixing
those naming inconsistencies with "algo" vs "algorithm" that you and
Robert have reported, and applied them. For 0003, I'll look at it
later. Attached is a rebase with improvements about the variable
names.

This has been done with the proper renames. With that in place, I see
no reason now to not be able to set the compression level as it is
possible to pass it down with the options available. This requires
only a couple of lines, as of the attached. LZ4 has a dummy structure
called LZ4F_INIT_PREFERENCES to initialize LZ4F_preferences_t, that
holds the compression level before passing it down to
LZ4F_compressBegin(), but that's available only in v1.8.3. Using it
risks lowering down the minimal version of LZ4 we are able to use now,
but replacing that with a set of memset()s is also a way to set up
things as per its documentation.

Thoughts?
--
Michael

Attachments:

receivewal-level.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/bin/pg_basebackup/walmethods.c b/src/bin/pg_basebackup/walmethods.c
index d5bcc208a9..1b3cc5c4f7 100644
--- a/src/bin/pg_basebackup/walmethods.c
+++ b/src/bin/pg_basebackup/walmethods.c
@@ -165,6 +165,7 @@ dir_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
 	{
 		size_t		ctx_out;
 		size_t		header_size;
+		LZ4F_preferences_t prefs;
 
 		ctx_out = LZ4F_createCompressionContext(&ctx, LZ4F_VERSION);
 		if (LZ4F_isError(ctx_out))
@@ -177,8 +178,13 @@ dir_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
 		lz4bufsize = LZ4F_compressBound(LZ4_IN_SIZE, NULL);
 		lz4buf = pg_malloc0(lz4bufsize);
 
+		/* assign the compression level, default is 0 */
+		memset(&prefs, 0, sizeof(prefs));
+		memset(&prefs.frameInfo, 0, sizeof(prefs.frameInfo));
+		prefs.compressionLevel = dir_data->compression_level;
+
 		/* add the header */
-		header_size = LZ4F_compressBegin(ctx, lz4buf, lz4bufsize, NULL);
+		header_size = LZ4F_compressBegin(ctx, lz4buf, lz4bufsize, &prefs);
 		if (LZ4F_isError(header_size))
 		{
 			dir_data->lasterrstring = LZ4F_getErrorName(header_size);
#7Noname
gkokolatos@pm.me
In reply to: Michael Paquier (#6)
Re: Fixes for compression options of pg_receivewal and refactoring of backup_compression.{c,h}

------- Original Message -------
On Wednesday, April 13th, 2022 at 7:25 AM, Michael Paquier <michael@paquier.xyz> wrote:

On Tue, Apr 12, 2022 at 06:22:48PM +0900, Michael Paquier wrote:

On Mon, Apr 11, 2022 at 12:46:02PM +0000, gkokolatos@pm.me wrote:

It looks good. If you choose to discard the comment regarding the use of
'method' over 'algorithm' from above, can you please use the full word in the
variable, e.g. 'wal_compress_algorithm' instead of 'wal_compress_algo'. I can
not really explain it, the later reads a bit rude. Then again that may be just
me.

Thanks. I have been able to do an extra pass on 0001 and 0002, fixing
those naming inconsistencies with "algo" vs "algorithm" that you and
Robert have reported, and applied them. For 0003, I'll look at it
later. Attached is a rebase with improvements about the variable
names.

This has been done with the proper renames. With that in place, I see
no reason now to not be able to set the compression level as it is
possible to pass it down with the options available. This requires
only a couple of lines, as of the attached. LZ4 has a dummy structure
called LZ4F_INIT_PREFERENCES to initialize LZ4F_preferences_t, that
holds the compression level before passing it down to
LZ4F_compressBegin(), but that's available only in v1.8.3. Using it
risks lowering down the minimal version of LZ4 we are able to use now,
but replacing that with a set of memset()s is also a way to set up
things as per its documentation.

Thoughts?

It's really not hard to add compression level. However we had briefly
discussed it in the original thread [1]/messages/by-id/CABUevEwuq7XXyd4fA0W3jY9MsJu9B2WRbHumAA+3WzHrGAQjsg@mail.gmail.com and decided against. That is why
I did not write that code. If the community thinks differently now, let
me know if you would like me to offer a patch for it.

Cheers,
//Georgios

[1]: /messages/by-id/CABUevEwuq7XXyd4fA0W3jY9MsJu9B2WRbHumAA+3WzHrGAQjsg@mail.gmail.com

#8Michael Paquier
michael@paquier.xyz
In reply to: Noname (#7)
Re: Fixes for compression options of pg_receivewal and refactoring of backup_compression.{c,h}

On Wed, Apr 13, 2022 at 02:58:28PM +0000, gkokolatos@pm.me wrote:

It's really not hard to add compression level. However we had briefly
discussed it in the original thread [1] and decided against. That is why
I did not write that code. If the community thinks differently now, let
me know if you would like me to offer a patch for it.

The issue back then was how to design the option set to handle all
that (right? My memories may be short on that), and pg_basebackup
takes care of that with its option design.

This is roughly what has been done here, except that this was for the
contentSize:
/messages/by-id/rYyZ3Fj9qayyY9-egNsV_kkLbL_BSWcOEdi3Mb6M9eQRTkcA2jrqFEHglLUEYnzWR_wttCqn7VI94MZ2p7mwNje51lHTvWYnJ1jHdOceen4=@pm.me

Do you think that the extra test coverage is going to be too much of a
burden? I was thinking about just adding a level to the main lz4
command, with an extra negative test in the SKIP block with a level
out of range
--
Michael

#9Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#8)
Re: Fixes for compression options of pg_receivewal and refactoring of backup_compression.{c,h}

On Thu, Apr 14, 2022 at 06:18:29AM +0900, Michael Paquier wrote:

The issue back then was how to design the option set to handle all
that (right? My memories may be short on that), and pg_basebackup
takes care of that with its option design.

I have looked at that again this morning, and the change is
straight-forward so I saw no reason to not do it. And, applied.
--
Michael