Subject: Re: Add LZ4 compression in pg_dump

From: Tomas Vondra <[email-masked]>
Date: Thu, 19 Jan 2023 18:55:43 +0100

Hi,

On 1/19/23 17:42, [email-masked] wrote:
> 
> ------- Original Message -------
> On Thursday, January 19th, 2023 at 4:45 PM, Tomas Vondra <[email-masked]> wrote:
>>
>> On 1/18/23 20:05, [email-masked] wrote:
>>
>>> ------- Original Message -------
>>> On Wednesday, January 18th, 2023 at 3:00 PM, Tomas Vondra [email-masked] wrote:
>>
>> I'm not sure I understand why leave the lz4/zstd in this place?
> 
> You are right, it is not obvious. Those were added in 5e73a60488 which is
> already committed in master and I didn't want to backtrack. Of course, I am
> not opposing in doing so if you wish.
> 

Ah, I didn't realize it was already added by earlier commit. In that
case let's not worry about it.

>>
>>>> 2) I wouldn't reorder the cases in WriteDataToArchive, i.e. I'd keep
>>>> "none" at the end. It might make backpatches harder.
>>>
>>> Agreed. However a 'default' is needed in order to avoid compilation warnings.
>>> Also note that 0002 completely does away with cases within WriteDataToArchive.
>>
>>
>> OK, although that's also a consequence of using a "switch" instead of
>> plan "if" branches.
>>
>> Furthermore, I'm not sure we really need the pg_fatal() about invalid
>> compression method in these default blocks. I mean, how could we even
>> get to these places when the build does not support the algorithm? All
>> of this (ReadDataFromArchive, WriteDataToArchive, EndCompressor, ...)
>> happens looong after the compressor was initialized and the method
>> checked, no? So maybe either this should simply do Assert(false) or use
>> a different error message.
> 
> I like Assert(false).
> 

OK, good. Do you agree we should never actually get there, if the
earlier checks work correctly?

>>
>>>> 4) "cfp" struct no longer wraps gzFile, but the comment was not updated.
>>>> FWIW I'm not sure switching to "void *" is an improvement, maybe it'd be
>>>> better to have a "union" of correct types?
>>>
>>> Please find and updated comment and a union in place of the void *. Also
>>> note that 0002 completely does away with cfp in favour of a new struct
>>> CompressFileHandle. I maintained the void * there because it is used by
>>> private methods of the compressors. 0003 contains such an example with
>>> LZ4CompressorState.
>>
>>
>> I wonder if this (and also the previous item) makes sense to keep 0001
>> and 0002 or to combine them. The "intermediate" state is a bit annoying.
> 
> Agreed. It was initially submitted as one patch. Then it was requested to be
> split up in two parts, one to expand the use of the existing API and one to
> replace with the new interface. Unfortunately the expansion of usage of the
> existing API requires some tweaking, but that is not a very good reason for
> the current patch set. I should have done a better job there.
> 
> Please find v22 attach which combines back 0001 and 0002. It is missing the
> documentation that was discussed above as I wanted to give a quick feedback.
> Let me know if you think that the combined version is the one to move forward
> with.
> 

Thanks, I'll take a look.

regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

---

From: Tomas Vondra <[email-masked]>
Date: Fri, 20 Jan 2023 00:34:34 +0100

On 1/19/23 18:55, Tomas Vondra wrote:
>> ...

After taking a look and thinking about it a bit more, I think we should
keep the two parts separate. I think Michael (or whoever proposed) the
split was right, it makes the patches easier to grok.

Sorry for the noise, hopefully we can just revert to the last version.

While reading the thread, I also noticed this:

> By the way, I think that this 0002 should drop all the default clauses
> in the switches for the compression method so as we'd catch any
> missing code paths with compiler warnings if a new compression method
> is added in the future.

Now I realize why there were "not yet implemented" errors for lz4/zstd
in all the switches, and why after removing them you had to add a
default branch.

We DON'T want a default branch, because the idea is that after adding a
new compression algorithm, we get warnings about switches not handling
it correctly.

So I guess we should walk back this change too :-( It's probably easier
to go back to v20 from January 16, and redo the couple remaining things
I commented on.

FWIW I think this is a hint that adding LZ4/ZSTD options, in 5e73a6048,
but without implementation, was not a great idea. It mostly defeats the
idea of getting the compiler warnings - all the places already handle
PG_COMPRESSION_LZ4/PG_COMPRESSION_ZSTD by throwing a pg_fatal. So you'd
have to grep for the options, inspect all the places or something like
that anyway. The warnings would only work for entirely new methods.

However, I now also realize the compressor API in 0002 replaces all of
this with calls to a generic API callback, so trying to improve this was
pretty silly from me.

Please, fix the couple remaining details in v20, add the docs for the
callbacks, and I'll try to polish it and get it committed.

regards

---

From: [email-masked]
Date: Fri, 20 Jan 2023 15:03:30 +0000

------- Original Message -------

> split was right, it makes the patches easier to grok.

Excellent. I will attempt a better split this time round.

> to go back to v20 from January 16, and redo the couple remaining things

Sure.

> this with calls to a generic API callback, so trying to improve this was

I can try to do a better job at splitting things up.

> callbacks, and I'll try to polish it and get it committed.

Excellent. Allow me an attempt to polish and expect a new version soon.

Cheers,
//Georgios

---

From: [email-masked]
Date: Mon, 23 Jan 2023 17:31:55 +0000

------- Original Message -------

> split was right, it makes the patches easier to grok.

Please find attached v23 which reintroduces the split.

0001 is reworked to have a reduced footprint than before. Also in an attempt
to facilitate the readability, 0002 splits the API's and the uncompressed
implementation in separate files.

> to go back to v20 from January 16, and redo the couple remaining things

No problem.

> callbacks, and I'll try to polish it and get it committed.

Thank you very much. Please find an attempt to comply with the requested
changes in the attached.

> regards

From 4861f0cac1ef181bccb37278ec74ec43d3f0df5f Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Mon, 23 Jan 2023 13:24:22 +0000
Subject: [PATCH v23 1/3] Prepare pg_dump internals for additional compression
 methods.

Commit  bf9aa490db introduced cfp in compress_io.{c,h} with the intent of
unifying compression related code and allow for the introduction of additional
archive formats. However, pg_backup_archiver.c was not using that API. This
commit teaches pg_backup_archiver.c about it and is using it throughout.
---
 src/bin/pg_dump/compress_io.c        |  66 +++++++++++---
 src/bin/pg_dump/compress_io.h        |   2 +
 src/bin/pg_dump/pg_backup_archiver.c | 129 ++++++++++-----------------
 src/bin/pg_dump/pg_backup_archiver.h |  27 +-----
 4 files changed, 102 insertions(+), 122 deletions(-)

diff --git a/src/bin/pg_dump/compress_io.c b/src/bin/pg_dump/compress_io.c
index 7a2c80bbc4..ef033914ba 100644
--- a/src/bin/pg_dump/compress_io.c
+++ b/src/bin/pg_dump/compress_io.c
@@ -56,6 +56,10 @@
 #include "compress_io.h"
 #include "pg_backup_utils.h"
 
+#ifdef HAVE_LIBZ
+#include <zlib.h>
+#endif
+
 /*----------------------
  * Compressor API
  *----------------------
@@ -490,16 +494,19 @@ cfopen_write(const char *path, const char *mode,
 }
 
 /*
- * Opens file 'path' in 'mode'. If compression is GZIP, the file
- * is opened with libz gzopen(), otherwise with plain fopen().
+ * This is the workhorse for cfopen() or cfdopen(). It opens file 'path' or
+ * associates a stream 'fd', if 'fd' is a valid descriptor, in 'mode'. The
+ * descriptor is not dup'ed and it is the caller's responsibility to do so.
+ * The caller must verify that the 'compress_algorithm' is supported by the
+ * current build.
  *
  * On failure, return NULL with an error code in errno.
  */
-cfp *
-cfopen(const char *path, const char *mode,
-	   const pg_compress_specification compression_spec)
+static cfp *
+cfopen_internal(const char *path, int fd, const char *mode,
+				pg_compress_specification compression_spec)
 {
-	cfp		   *fp = pg_malloc(sizeof(cfp));
+	cfp		   *fp = pg_malloc0(sizeof(cfp));
 
 	if (compression_spec.algorithm == PG_COMPRESSION_GZIP)
 	{
@@ -511,15 +518,20 @@ cfopen(const char *path, const char *mode,
 
 			snprintf(mode_compression, sizeof(mode_compression), "%s%d",
 					 mode, compression_spec.level);
-			fp->compressedfp = gzopen(path, mode_compression);
+			if (fd >= 0)
+				fp->compressedfp = gzdopen(fd, mode_compression);
+			else
+				fp->compressedfp = gzopen(path, mode_compression);
 		}
 		else
 		{
 			/* don't specify a level, just use the zlib default */
-			fp->compressedfp = gzopen(path, mode);
+			if (fd >= 0)
+				fp->compressedfp = gzdopen(fd, mode);
+			else
+				fp->compressedfp = gzopen(path, mode);
 		}
 
-		fp->uncompressedfp = NULL;
 		if (fp->compressedfp == NULL)
 		{
 			free_keep_errno(fp);
@@ -531,10 +543,11 @@ cfopen(const char *path, const char *mode,
 	}
 	else
 	{
-#ifdef HAVE_LIBZ
-		fp->compressedfp = NULL;
-#endif
-		fp->uncompressedfp = fopen(path, mode);
+		if (fd >= 0)
+			fp->uncompressedfp = fdopen(fd, mode);
+		else
+			fp->uncompressedfp = fopen(path, mode);
+
 		if (fp->uncompressedfp == NULL)
 		{
 			free_keep_errno(fp);
@@ -545,6 +558,33 @@ cfopen(const char *path, const char *mode,
 	return fp;
 }
 
+/*
+ * Opens file 'path' in 'mode' and compression as defined in
+ * compression_spec. The caller must verify that the compression
+ * is supported by the current build.
+ *
+ * On failure, return NULL with an error code in errno.
+ */
+cfp *
+cfopen(const char *path, const char *mode,
+	   const pg_compress_specification compression_spec)
+{
+	return cfopen_internal(path, -1, mode, compression_spec);
+}
+
+/*
+ * Associates a stream 'fd', if 'fd' is a valid descriptor, in 'mode'
+ * and compression as defined in compression_spec. The caller must
+ * verify that the compression is supported by the current build.
+ *
+ * On failure, return NULL with an error code in errno.
+ */
+cfp *
+cfdopen(int fd, const char *mode,
+		const pg_compress_specification compression_spec)
+{
+	return cfopen_internal(NULL, fd, mode, compression_spec);
+}
 
 int
 cfread(void *ptr, int size, cfp *fp)
diff --git a/src/bin/pg_dump/compress_io.h b/src/bin/pg_dump/compress_io.h
index a429dc4789..a6cdf588dd 100644
--- a/src/bin/pg_dump/compress_io.h
+++ b/src/bin/pg_dump/compress_io.h
@@ -54,6 +54,8 @@ typedef struct cfp cfp;
 
 extern cfp *cfopen(const char *path, const char *mode,
 				   const pg_compress_specification compression_spec);
+extern cfp *cfdopen(int fd, const char *mode,
+					const pg_compress_specification compression_spec);
 extern cfp *cfopen_read(const char *path, const char *mode);
 extern cfp *cfopen_write(const char *path, const char *mode,
 						 const pg_compress_specification compression_spec);
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index ba5e6acbbb..ddb712d042 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -31,6 +31,7 @@
 #endif
 
 #include "common/string.h"
+#include "compress_io.h"
 #include "dumputils.h"
 #include "fe_utils/string_utils.h"
 #include "lib/stringinfo.h"
@@ -43,13 +44,6 @@
 #define TEXT_DUMP_HEADER "--\n-- PostgreSQL database dump\n--\n\n"
 #define TEXT_DUMPALL_HEADER "--\n-- PostgreSQL database cluster dump\n--\n\n"
 
-/* state needed to save/restore an archive's output target */
-typedef struct _outputContext
-{
-	void	   *OF;
-	int			gzOut;
-} OutputContext;
-
 /*
  * State for tracking TocEntrys that are ready to process during a parallel
  * restore.  (This used to be a list, and we still call it that, though now
@@ -101,8 +95,8 @@ static void dump_lo_buf(ArchiveHandle *AH);
 static void dumpTimestamp(ArchiveHandle *AH, const char *msg, time_t tim);
 static void SetOutput(ArchiveHandle *AH, const char *filename,
 					  const pg_compress_specification compression_spec);
-static OutputContext SaveOutput(ArchiveHandle *AH);
-static void RestoreOutput(ArchiveHandle *AH, OutputContext savedContext);
+static cfp *SaveOutput(ArchiveHandle *AH);
+static void RestoreOutput(ArchiveHandle *AH, cfp *savedOutput);
 
 static int	restore_toc_entry(ArchiveHandle *AH, TocEntry *te, bool is_parallel);
 static void restore_toc_entries_prefork(ArchiveHandle *AH,
@@ -277,11 +271,8 @@ CloseArchive(Archive *AHX)
 	AH->ClosePtr(AH);
 
 	/* Close the output */
-	errno = 0;					/* in case gzclose() doesn't set it */
-	if (AH->gzOut)
-		res = GZCLOSE(AH->OF);
-	else if (AH->OF != stdout)
-		res = fclose(AH->OF);
+	errno = 0;
+	res = cfclose(AH->OF);
 
 	if (res != 0)
 		pg_fatal("could not close output file: %m");
@@ -362,8 +353,9 @@ RestoreArchive(Archive *AHX)
 	ArchiveHandle *AH = (ArchiveHandle *) AHX;
 	RestoreOptions *ropt = AH->public.ropt;
 	bool		parallel_mode;
+	bool		supports_compression;
 	TocEntry   *te;
-	OutputContext sav;
+	cfp		   *sav;
 
 	AH->stage = STAGE_INITIALIZING;
 
@@ -391,17 +383,27 @@ RestoreArchive(Archive *AHX)
 	/*
 	 * Make sure we won't need (de)compression we haven't got
 	 */
-#ifndef HAVE_LIBZ
+	supports_compression = false;
+	if (AH->compression_spec.algorithm == PG_COMPRESSION_NONE ||
+		AH->compression_spec.algorithm == PG_COMPRESSION_GZIP)
+		supports_compression = true;
+
 	if (AH->compression_spec.algorithm == PG_COMPRESSION_GZIP &&
 		AH->PrintTocDataPtr != NULL)
 	{
 		for (te = AH->toc->next; te != AH->toc; te = te->next)
 		{
 			if (te->hadDumper && (te->reqs & REQ_DATA) != 0)
-				pg_fatal("cannot restore from compressed archive (compression not supported in this installation)");
+			{
+#ifndef HAVE_LIBZ
+				if (AH->compression_spec.algorithm == PG_COMPRESSION_GZIP)
+					supports_compression = false;
+#endif
+				if (supports_compression == false)
+					pg_fatal("cannot restore from compressed archive (compression not supported in this installation)");
+			}
 		}
 	}
-#endif
 
 	/*
 	 * Prepare index arrays, so we can assume we have them throughout restore.
@@ -1133,7 +1135,7 @@ PrintTOCSummary(Archive *AHX)
 	TocEntry   *te;
 	pg_compress_specification out_compression_spec = {0};
 	teSection	curSection;
-	OutputContext sav;
+	cfp		   *sav;
 	const char *fmtName;
 	char		stamp_str[64];
 
@@ -1508,58 +1510,32 @@ static void
 SetOutput(ArchiveHandle *AH, const char *filename,
 		  const pg_compress_specification compression_spec)
 {
-	int			fn;
+	const char *mode;
+	int			fn = -1;
 
 	if (filename)
 	{
 		if (strcmp(filename, "-") == 0)
 			fn = fileno(stdout);
-		else
-			fn = -1;
 	}
 	else if (AH->FH)
 		fn = fileno(AH->FH);
 	else if (AH->fSpec)
 	{
-		fn = -1;
 		filename = AH->fSpec;
 	}
 	else
 		fn = fileno(stdout);
 
-	/* If compression explicitly requested, use gzopen */
-#ifdef HAVE_LIBZ
-	if (compression_spec.algorithm == PG_COMPRESSION_GZIP)
-	{
-		char		fmode[14];
+	if (AH->mode == archModeAppend)
+		mode = PG_BINARY_A;
+	else
+		mode = PG_BINARY_W;
 
-		/* Don't use PG_BINARY_x since this is zlib */
-		sprintf(fmode, "wb%d", compression_spec.level);
-		if (fn >= 0)
-			AH->OF = gzdopen(dup(fn), fmode);
-		else
-			AH->OF = gzopen(filename, fmode);
-		AH->gzOut = 1;
-	}
+	if (fn >= 0)
+		AH->OF = cfdopen(dup(fn), mode, compression_spec);
 	else
-#endif
-	{							/* Use fopen */
-		if (AH->mode == archModeAppend)
-		{
-			if (fn >= 0)
-				AH->OF = fdopen(dup(fn), PG_BINARY_A);
-			else
-				AH->OF = fopen(filename, PG_BINARY_A);
-		}
-		else
-		{
-			if (fn >= 0)
-				AH->OF = fdopen(dup(fn), PG_BINARY_W);
-			else
-				AH->OF = fopen(filename, PG_BINARY_W);
-		}
-		AH->gzOut = 0;
-	}
+		AH->OF = cfopen(filename, mode, compression_spec);
 
 	if (!AH->OF)
 	{
@@ -1570,33 +1546,24 @@ SetOutput(ArchiveHandle *AH, const char *filename,
 	}
 }
 
-static OutputContext
+static cfp *
 SaveOutput(ArchiveHandle *AH)
 {
-	OutputContext sav;
-
-	sav.OF = AH->OF;
-	sav.gzOut = AH->gzOut;
-
-	return sav;
+	return (cfp *) AH->OF;
 }
 
 static void
-RestoreOutput(ArchiveHandle *AH, OutputContext savedContext)
+RestoreOutput(ArchiveHandle *AH, cfp *savedOutput)
 {
 	int			res;
 
-	errno = 0;					/* in case gzclose() doesn't set it */
-	if (AH->gzOut)
-		res = GZCLOSE(AH->OF);
-	else
-		res = fclose(AH->OF);
+	errno = 0;
+	res = cfclose(AH->OF);
 
 	if (res != 0)
 		pg_fatal("could not close output file: %m");
 
-	AH->gzOut = savedContext.gzOut;
-	AH->OF = savedContext.OF;
+	AH->OF = savedOutput;
 }

@@ -1720,22 +1687,17 @@ ahwrite(const void *ptr, size_t size, size_t nmemb, ArchiveHandle *AH)
 
 		bytes_written = size * nmemb;
 	}
-	else if (AH->gzOut)
-		bytes_written = GZWRITE(ptr, size, nmemb, AH->OF);
 	else if (AH->CustomOutPtr)
 		bytes_written = AH->CustomOutPtr(AH, ptr, size * nmemb);
 
+	/*
+	 * If we're doing a restore, and it's direct to DB, and we're connected
+	 * then send it to the DB.
+	 */
+	else if (RestoringToDB(AH))
+		bytes_written = ExecuteSqlCommandBuf(&AH->public, (const char *) ptr, size * nmemb);
 	else
-	{
-		/*
-		 * If we're doing a restore, and it's direct to DB, and we're
-		 * connected then send it to the DB.
-		 */
-		if (RestoringToDB(AH))
-			bytes_written = ExecuteSqlCommandBuf(&AH->public, (const char *) ptr, size * nmemb);
-		else
-			bytes_written = fwrite(ptr, size, nmemb, AH->OF) * size;
-	}
+		bytes_written = cfwrite(ptr, size * nmemb, AH->OF);
 
 	if (bytes_written != size * nmemb)
 		WRITE_ERROR_EXIT;
@@ -2224,6 +2186,7 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,
 		 SetupWorkerPtrType setupWorkerPtr)
 {
 	ArchiveHandle *AH;
+	pg_compress_specification out_compress_spec = {0};
 
 	pg_log_debug("allocating AH for %s, format %d",
 				 FileSpec ? FileSpec : "(stdio)", fmt);
@@ -2277,8 +2240,8 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,
 	memset(&(AH->sqlparse), 0, sizeof(AH->sqlparse));
 
 	/* Open stdout with no compression for AH output handle */
-	AH->gzOut = 0;
-	AH->OF = stdout;
+	out_compress_spec.algorithm = PG_COMPRESSION_NONE;
+	AH->OF = cfdopen(dup(fileno(stdout)), PG_BINARY_A, out_compress_spec);
 
 	/*
 	 * On Windows, we need to use binary mode to read/write non-text files,
diff --git a/src/bin/pg_dump/pg_backup_archiver.h b/src/bin/pg_dump/pg_backup_archiver.h
index f72446ed5b..4725e49747 100644
--- a/src/bin/pg_dump/pg_backup_archiver.h
+++ b/src/bin/pg_dump/pg_backup_archiver.h
@@ -32,30 +32,6 @@
 
 #define LOBBUFSIZE 16384
 
-#ifdef HAVE_LIBZ
-#include <zlib.h>
-#define GZCLOSE(fh) gzclose(fh)
-#define GZWRITE(p, s, n, fh) gzwrite(fh, p, (n) * (s))
-#define GZREAD(p, s, n, fh) gzread(fh, p, (n) * (s))
-#define GZEOF(fh)	gzeof(fh)
-#else
-#define GZCLOSE(fh) fclose(fh)
-#define GZWRITE(p, s, n, fh) (fwrite(p, s, n, fh) * (s))
-#define GZREAD(p, s, n, fh) fread(p, s, n, fh)
-#define GZEOF(fh)	feof(fh)
-/* this is just the redefinition of a libz constant */
-#define Z_DEFAULT_COMPRESSION (-1)
-
-typedef struct _z_stream
-{
-	void	   *next_in;
-	void	   *next_out;
-	size_t		avail_in;
-	size_t		avail_out;
-} z_stream;
-typedef z_stream *z_streamp;
-#endif
-
 /* Data block types */
 #define BLK_DATA 1
 #define BLK_BLOBS 3
@@ -319,8 +295,7 @@ struct _archiveHandle
 
 	char	   *fSpec;			/* Archive File Spec */
 	FILE	   *FH;				/* General purpose file handle */
-	void	   *OF;
-	int			gzOut;			/* Output file */
+	void	   *OF;				/* Output file */
 
 	struct _tocEntry *toc;		/* Header of circular list of TOC entries */
 	int			tocCount;		/* Number of TOC entries */
-- 
2.34.1

From 656dfad3ababfdd4665880494bec0271004702ae Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Mon, 23 Jan 2023 14:29:02 +0000
Subject: [PATCH v23 2/3] Introduce Compress and Compressor API in pg_dump

The purpose of this API is to allow for easier addition of new compression
methods. CompressFileHandle is substituting the cfp* family of functions under a
struct of function pointers for file manipulation. The implementor of a new
compression method is now able to "simply" just add those definitions.

Additionaly custom compressed archives store the compression algorithm in their
header instead of the compression level. The header version number is bumped.
---
 src/bin/pg_dump/Makefile              |   2 +
 src/bin/pg_dump/compress_gzip.c       | 404 ++++++++++++++
 src/bin/pg_dump/compress_gzip.h       |  24 +
 src/bin/pg_dump/compress_io.c         | 746 +++++---------------------
 src/bin/pg_dump/compress_io.h         | 166 +++++-
 src/bin/pg_dump/compress_none.c       | 206 +++++++
 src/bin/pg_dump/compress_none.h       |  24 +
 src/bin/pg_dump/meson.build           |   2 +
 src/bin/pg_dump/pg_backup_archiver.c  | 102 ++--
 src/bin/pg_dump/pg_backup_archiver.h  |   5 +-
 src/bin/pg_dump/pg_backup_custom.c    |  23 +-
 src/bin/pg_dump/pg_backup_directory.c |  94 ++--
 src/bin/pg_dump/t/002_pg_dump.pl      |  10 +-
 src/tools/pginclude/cpluspluscheck    |   2 +
 src/tools/pgindent/typedefs.list      |   2 +
 15 files changed, 1060 insertions(+), 752 deletions(-)
 create mode 100644 src/bin/pg_dump/compress_gzip.c
 create mode 100644 src/bin/pg_dump/compress_gzip.h
 create mode 100644 src/bin/pg_dump/compress_none.c
 create mode 100644 src/bin/pg_dump/compress_none.h

diff --git a/src/bin/pg_dump/Makefile b/src/bin/pg_dump/Makefile
index ef1ed0f3e5..0013bc080c 100644
--- a/src/bin/pg_dump/Makefile
+++ b/src/bin/pg_dump/Makefile
@@ -24,7 +24,9 @@ LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport)
 
 OBJS = \
 	$(WIN32RES) \
+	compress_gzip.o \
 	compress_io.o \
+	compress_none.o \
 	dumputils.o \
 	parallel.o \
 	pg_backup_archiver.o \
diff --git a/src/bin/pg_dump/compress_gzip.c b/src/bin/pg_dump/compress_gzip.c
new file mode 100644
index 0000000000..ffc4e6d56b
--- /dev/null
+++ b/src/bin/pg_dump/compress_gzip.c
@@ -0,0 +1,404 @@
+/*-------------------------------------------------------------------------
+ *
+ * compress_gzip.c
+ *	 Routines for archivers to read or write a gzip compressed data stream.
+ *
+ * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * IDENTIFICATION
+ *	   src/bin/pg_dump/compress_gzip.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "postgres_fe.h"
+#include <unistd.h>
+
+#include "compress_gzip.h"
+#include "pg_backup_utils.h"
+
+#ifdef HAVE_LIBZ
+#include "zlib.h"
+
+/*----------------------
+ * Compressor API
+ *----------------------
+ */
+typedef struct GzipCompressorState
+{
+	z_streamp	zp;
+
+	void	   *outbuf;
+	size_t		outsize;
+} GzipCompressorState;
+
+/* Private routines that support gzip compressed data I/O */
+static void
+DeflateCompressorGzip(ArchiveHandle *AH, CompressorState *cs, bool flush)
+{
+	GzipCompressorState *gzipcs = (GzipCompressorState *) cs->private_data;
+	z_streamp	zp = gzipcs->zp;
+	void	   *out = gzipcs->outbuf;
+	int			res = Z_OK;
+
+	while (gzipcs->zp->avail_in != 0 || flush)
+	{
+		res = deflate(zp, flush ? Z_FINISH : Z_NO_FLUSH);
+		if (res == Z_STREAM_ERROR)
+			pg_fatal("could not compress data: %s", zp->msg);
+		if ((flush && (zp->avail_out < gzipcs->outsize))
+			|| (zp->avail_out == 0)
+			|| (zp->avail_in != 0)
+			)
+		{
+			/*
+			 * Extra paranoia: avoid zero-length chunks, since a zero length
+			 * chunk is the EOF marker in the custom format. This should never
+			 * happen but...
+			 */
+			if (zp->avail_out < gzipcs->outsize)
+			{
+				/*
+				 * Any write function should do its own error checking but to
+				 * make sure we do a check here as well...
+				 */
+				size_t		len = gzipcs->outsize - zp->avail_out;
+
+				cs->writeF(AH, (char *) out, len);
+			}
+			zp->next_out = out;
+			zp->avail_out = gzipcs->outsize;
+		}
+
+		if (res == Z_STREAM_END)
+			break;
+	}
+}
+
+static void
+EndCompressorGzip(ArchiveHandle *AH, CompressorState *cs)
+{
+	GzipCompressorState *gzipcs = (GzipCompressorState *) cs->private_data;
+	z_streamp	zp;
+
+	if (gzipcs->zp)
+	{
+		zp = gzipcs->zp;
+		zp->next_in = NULL;
+		zp->avail_in = 0;
+
+		/* Flush any remaining data from zlib buffer */
+		DeflateCompressorGzip(AH, cs, true);
+
+		if (deflateEnd(zp) != Z_OK)
+			pg_fatal("could not close compression stream: %s", zp->msg);
+
+		pg_free(gzipcs->outbuf);
+		pg_free(gzipcs->zp);
+	}
+
+	pg_free(gzipcs);
+	cs->private_data = NULL;
+}
+
+static void
+WriteDataToArchiveGzip(ArchiveHandle *AH, CompressorState *cs,
+					   const void *data, size_t dLen)
+{
+	GzipCompressorState *gzipcs = (GzipCompressorState *) cs->private_data;
+	z_streamp	zp;
+
+	if (!gzipcs->zp)
+	{
+		zp = gzipcs->zp = (z_streamp) pg_malloc(sizeof(z_stream));
+		zp->zalloc = Z_NULL;
+		zp->zfree = Z_NULL;
+		zp->opaque = Z_NULL;
+
+		/*
+		 * outsize is the buffer size we tell zlib it can output to.  We
+		 * actually allocate one extra byte because some routines want to
+		 * append a trailing zero byte to the zlib output.
+		 */
+		gzipcs->outbuf = pg_malloc(ZLIB_OUT_SIZE + 1);
+		gzipcs->outsize = ZLIB_OUT_SIZE;
+
+		/*
+		 * A level of zero simply copies the input one block at the time. This
+		 * is probably not what the user wanted when calling this interface.
+		 */
+		if (cs->compression_spec.level == 0)
+			pg_fatal("requested to compress the archive yet no level was specified");
+
+		if (deflateInit(zp, cs->compression_spec.level) != Z_OK)
+			pg_fatal("could not initialize compression library: %s", zp->msg);
+
+		/* Just be paranoid - maybe End is called after Start, with no Write */
+		zp->next_out = gzipcs->outbuf;
+		zp->avail_out = gzipcs->outsize;
+	}
+
+	gzipcs->zp->next_in = (void *) unconstify(void *, data);
+	gzipcs->zp->avail_in = dLen;
+	DeflateCompressorGzip(AH, cs, false);
+}
+
+static void
+ReadDataFromArchiveGzip(ArchiveHandle *AH, CompressorState *cs)
+{
+	z_streamp	zp;
+	char	   *out;
+	int			res = Z_OK;
+	size_t		cnt;
+	char	   *buf;
+	size_t		buflen;
+
+	zp = (z_streamp) pg_malloc(sizeof(z_stream));
+	zp->zalloc = Z_NULL;
+	zp->zfree = Z_NULL;
+	zp->opaque = Z_NULL;
+
+	buf = pg_malloc(ZLIB_IN_SIZE);
+	buflen = ZLIB_IN_SIZE;
+
+	out = pg_malloc(ZLIB_OUT_SIZE + 1);
+
+	if (inflateInit(zp) != Z_OK)
+		pg_fatal("could not initialize compression library: %s",
+				 zp->msg);
+
+	/* no minimal chunk size for zlib */
+	while ((cnt = cs->readF(AH, &buf, &buflen)))
+	{
+		zp->next_in = (void *) buf;
+		zp->avail_in = cnt;
+
+		while (zp->avail_in > 0)
+		{
+			zp->next_out = (void *) out;
+			zp->avail_out = ZLIB_OUT_SIZE;
+
+			res = inflate(zp, 0);
+			if (res != Z_OK && res != Z_STREAM_END)
+				pg_fatal("could not uncompress data: %s", zp->msg);
+
+			out[ZLIB_OUT_SIZE - zp->avail_out] = '\0';
+			ahwrite(out, 1, ZLIB_OUT_SIZE - zp->avail_out, AH);
+		}
+	}
+
+	zp->next_in = NULL;
+	zp->avail_in = 0;
+	while (res != Z_STREAM_END)
+	{
+		zp->next_out = (void *) out;
+		zp->avail_out = ZLIB_OUT_SIZE;
+		res = inflate(zp, 0);
+		if (res != Z_OK && res != Z_STREAM_END)
+			pg_fatal("could not uncompress data: %s", zp->msg);
+
+		out[ZLIB_OUT_SIZE - zp->avail_out] = '\0';
+		ahwrite(out, 1, ZLIB_OUT_SIZE - zp->avail_out, AH);
+	}
+
+	if (inflateEnd(zp) != Z_OK)
+		pg_fatal("could not close compression library: %s", zp->msg);
+
+	free(buf);
+	free(out);
+	free(zp);
+}
+
+/* Public routines that support gzip compressed data I/O */
+void
+InitCompressorGzip(CompressorState *cs, const pg_compress_specification compression_spec)
+{
+	GzipCompressorState *gzipcs;
+
+	cs->readData = ReadDataFromArchiveGzip;
+	cs->writeData = WriteDataToArchiveGzip;
+	cs->end = EndCompressorGzip;
+
+	cs->compression_spec = compression_spec;
+
+	gzipcs = (GzipCompressorState *) pg_malloc0(sizeof(GzipCompressorState));
+
+	cs->private_data = gzipcs;
+}
+
+
+/*----------------------
+ * Compress File API
+ *----------------------
+ */
+
+static size_t
+Gzip_read(void *ptr, size_t size, CompressFileHandle *CFH)
+{
+	gzFile		gzfp = (gzFile) CFH->private_data;
+	size_t		ret;
+
+	ret = gzread(gzfp, ptr, size);
+	if (ret != size && !gzeof(gzfp))
+	{
+		int			errnum;
+		const char *errmsg = gzerror(gzfp, &errnum);
+
+		pg_fatal("could not read from input file: %s",
+				 errnum == Z_ERRNO ? strerror(errno) : errmsg);
+	}
+
+	return ret;
+}
+
+static size_t
+Gzip_write(const void *ptr, size_t size, CompressFileHandle *CFH)
+{
+	gzFile		gzfp = (gzFile) CFH->private_data;
+
+	return gzwrite(gzfp, ptr, size);
+}
+
+static int
+Gzip_getc(CompressFileHandle *CFH)
+{
+	gzFile		gzfp = (gzFile) CFH->private_data;
+	int			ret;
+
+	errno = 0;
+	ret = gzgetc(gzfp);
+	if (ret == EOF)
+	{
+		if (!gzeof(gzfp))
+			pg_fatal("could not read from input file: %s", strerror(errno));
+		else
+			pg_fatal("could not read from input file: end of file");
+	}
+
+	return ret;
+}
+
+static char *
+Gzip_gets(char *ptr, int size, CompressFileHandle *CFH)
+{
+	gzFile		gzfp = (gzFile) CFH->private_data;
+
+	return gzgets(gzfp, ptr, size);
+}
+
+static int
+Gzip_close(CompressFileHandle *CFH)
+{
+	gzFile		gzfp = (gzFile) CFH->private_data;
+	int			save_errno;
+	int			ret;
+
+	CFH->private_data = NULL;
+
+	ret = gzclose(gzfp);
+
+	save_errno = errno;
+	errno = save_errno;
+
+	return ret;
+}
+
+static int
+Gzip_eof(CompressFileHandle *CFH)
+{
+	gzFile		gzfp = (gzFile) CFH->private_data;
+
+	return gzeof(gzfp);
+}
+
+static const char *
+Gzip_get_error(CompressFileHandle *CFH)
+{
+	gzFile		gzfp = (gzFile) CFH->private_data;
+	const char *errmsg;
+	int			errnum;
+
+	errmsg = gzerror(gzfp, &errnum);
+	if (errnum == Z_ERRNO)
+		errmsg = strerror(errno);
+
+	return errmsg;
+}
+
+static int
+Gzip_open(const char *path, int fd, const char *mode, CompressFileHandle *CFH)
+{
+	gzFile		gzfp;
+	char		mode_compression[32];
+
+	if (CFH->compression_spec.level != Z_DEFAULT_COMPRESSION)
+	{
+		/*
+		 * user has specified a compression level, so tell zlib to use it
+		 */
+		snprintf(mode_compression, sizeof(mode_compression), "%s%d",
+				 mode, CFH->compression_spec.level);
+	}
+	else
+		strcpy(mode_compression, mode);
+
+	if (fd >= 0)
+		gzfp = gzdopen(dup(fd), mode_compression);
+	else
+		gzfp = gzopen(path, mode_compression);
+
+	if (gzfp == NULL)
+		return 1;
+
+	CFH->private_data = gzfp;
+
+	return 0;
+}
+
+static int
+Gzip_open_write(const char *path, const char *mode, CompressFileHandle *CFH)
+{
+	char	   *fname;
+	int			ret;
+	int			save_errno;
+
+	fname = psprintf("%s.gz", path);
+	ret = CFH->open_func(fname, -1, mode, CFH);
+
+	save_errno = errno;
+	pg_free(fname);
+	errno = save_errno;
+
+	return ret;
+}
+
+void
+InitCompressGzip(CompressFileHandle *CFH, const pg_compress_specification compression_spec)
+{
+	CFH->open_func = Gzip_open;
+	CFH->open_write_func = Gzip_open_write;
+	CFH->read_func = Gzip_read;
+	CFH->write_func = Gzip_write;
+	CFH->gets_func = Gzip_gets;
+	CFH->getc_func = Gzip_getc;
+	CFH->close_func = Gzip_close;
+	CFH->eof_func = Gzip_eof;
+	CFH->get_error_func = Gzip_get_error;
+
+	CFH->compression_spec = compression_spec;
+
+	CFH->private_data = NULL;
+}
+#else							/* HAVE_LIBZ */
+void
+InitCompressorGzip(CompressorState *cs, const pg_compress_specification compression_spec)
+{
+	pg_fatal("this build does not support compression with %s", "gzip");
+}
+
+void
+InitCompressGzip(CompressFileHandle *CFH, const pg_compress_specification compression_spec)
+{
+	pg_fatal("this build does not support compression with %s", "gzip");
+}
+#endif							/* HAVE_LIBZ */
diff --git a/src/bin/pg_dump/compress_gzip.h b/src/bin/pg_dump/compress_gzip.h
new file mode 100644
index 0000000000..45fdf01f7d
--- /dev/null
+++ b/src/bin/pg_dump/compress_gzip.h
@@ -0,0 +1,24 @@
+/*-------------------------------------------------------------------------
+ *
+ * compress_gzip.h
+ *	 GZIP interface to compress_io.c routines
+ *
+ * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * IDENTIFICATION
+ *	   src/bin/pg_dump/compress_gzip.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef _COMPRESS_GZIP_H_
+#define _COMPRESS_GZIP_H_
+
+#include "compress_io.h"
+
+extern void InitCompressorGzip(CompressorState *cs,
+							   const pg_compress_specification compression_spec);
+extern void InitCompressGzip(CompressFileHandle *CFH,
+							 const pg_compress_specification compression_spec);
+
+#endif							/* _COMPRESS_GZIP_H_ */
diff --git a/src/bin/pg_dump/compress_io.c b/src/bin/pg_dump/compress_io.c
index ef033914ba..ea4c266e08 100644
--- a/src/bin/pg_dump/compress_io.c
+++ b/src/bin/pg_dump/compress_io.c
@@ -9,42 +9,54 @@
  *
  * This file includes two APIs for dealing with compressed data. The first
  * provides more flexibility, using callbacks to read/write data from the
- * underlying stream. The second API is a wrapper around fopen/gzopen and
+ * underlying stream. The second API is a wrapper around fopen and
  * friends, providing an interface similar to those, but abstracts away
- * the possible compression. Both APIs use libz for the compression, but
- * the second API uses gzip headers, so the resulting files can be easily
- * manipulated with the gzip utility.
+ * the possible compression. The second API is aimed for the resulting
+ * files to be easily manipulated with an external compression utility
+ * program.
+ *
+ * This file also includes the implementation when compression is none for
+ * both API's.
  *
  * Compressor API
  * --------------
  *
  *	The interface for writing to an archive consists of three functions:
- *	AllocateCompressor, WriteDataToArchive and EndCompressor. First you call
- *	AllocateCompressor, then write all the data by calling WriteDataToArchive
- *	as many times as needed, and finally EndCompressor. WriteDataToArchive
- *	and EndCompressor will call the WriteFunc that was provided to
- *	AllocateCompressor for each chunk of compressed data.
- *
- *	The interface for reading an archive consists of just one function:
- *	ReadDataFromArchive. ReadDataFromArchive reads the whole compressed input
- *	stream, by repeatedly calling the given ReadFunc. ReadFunc returns the
- *	compressed data chunk at a time, and ReadDataFromArchive decompresses it
- *	and passes the decompressed data to ahwrite(), until ReadFunc returns 0
- *	to signal EOF.
- *
- *	The interface is the same for compressed and uncompressed streams.
+ *	AllocateCompressor, writeData, and EndCompressor. First you call
+ *	AllocateCompressor, then write all the data by calling writeData as many
+ *	times as needed, and finally EndCompressor. writeData will call the
+ *	WriteFunc that was provided to AllocateCompressor for each chunk of
+ *	compressed data.
+ *
+ *	The interface for reading an archive consists of the same three functions:
+ *	AllocateCompressor, readData, and EndCompressor. First you call
+ *	AllocateCompressor, then read all the data by calling readData to read the
+ *	whole compressed stream which repeatedly calls the given ReadFunc. ReadFunc
+ *	returns the compressed data one chunk at a time. Then readData decompresses
+ *	it and passes the decompressed data to ahwrite(), until ReadFunc returns 0
+ *	to signal EOF. The interface is the same for compressed and uncompressed
+ *	streams.
  *
  * Compressed stream API
  * ----------------------
  *
- *	The compressed stream API is a wrapper around the C standard fopen() and
- *	libz's gzopen() APIs. It allows you to use the same functions for
- *	compressed and uncompressed streams. cfopen_read() first tries to open
- *	the file with given name, and if it fails, it tries to open the same
- *	file with the .gz suffix. cfopen_write() opens a file for writing, an
- *	extra argument specifies if the file should be compressed, and adds the
- *	.gz suffix to the filename if so. This allows you to easily handle both
- *	compressed and uncompressed files.
+ *	The compressed stream API is providing a set of function pointers for
+ *	opening, reading, writing, and finally closing files. The implemented
+ *	function pointers are documented in the corresponding header file and are
+ *	common for all streams. It allows the caller to use the same functions for
+ *	both compressed and uncompressed streams.
+ *
+ *	The interface consists of three functions, InitCompressFileHandle,
+ *	InitDiscoverCompressFileHandle, and EndCompressFileHandle. If the
+ *	compression is known, then start by calling InitCompressFileHandle,
+ *	otherwise discover it by using InitDiscoverCompressFileHandle. Then call
+ *	the function pointers as required for the read/write operations. Finally
+ *	call EndCompressFileHandle to end the stream.
+ *
+ *	InitDiscoverCompressFileHandle tries to deffer the compression by the
+ *	filename suffix. If the suffix is not yet known, then it tries to simply
+ *	open the file, and if it fails, it tries to open the same file with the .gz
+ *	suffix.
  *
  * IDENTIFICATION
  *	   src/bin/pg_dump/compress_io.c
@@ -53,122 +65,38 @@
  */
 #include "postgres_fe.h"
 
+#include <sys/stat.h>
+#include <unistd.h>
+
+#include "compress_gzip.h"
 #include "compress_io.h"
+#include "compress_none.h"
 #include "pg_backup_utils.h"
 
-#ifdef HAVE_LIBZ
-#include <zlib.h>
-#endif
-
 /*----------------------
  * Compressor API
  *----------------------
  */
 
-/* typedef appears in compress_io.h */
-struct CompressorState
-{
-	pg_compress_specification compression_spec;
-	WriteFunc	writeF;
-
-#ifdef HAVE_LIBZ
-	z_streamp	zp;
-	char	   *zlibOut;
-	size_t		zlibOutSize;
-#endif
-};
-
-/* Routines that support zlib compressed data I/O */
-#ifdef HAVE_LIBZ
-static void InitCompressorZlib(CompressorState *cs, int level);
-static void DeflateCompressorZlib(ArchiveHandle *AH, CompressorState *cs,
-								  bool flush);
-static void ReadDataFromArchiveZlib(ArchiveHandle *AH, ReadFunc readF);
-static void WriteDataToArchiveZlib(ArchiveHandle *AH, CompressorState *cs,
-								   const char *data, size_t dLen);
-static void EndCompressorZlib(ArchiveHandle *AH, CompressorState *cs);
-#endif
-
-/* Routines that support uncompressed data I/O */
-static void ReadDataFromArchiveNone(ArchiveHandle *AH, ReadFunc readF);
-static void WriteDataToArchiveNone(ArchiveHandle *AH, CompressorState *cs,
-								   const char *data, size_t dLen);
-
-/* Public interface routines */
-
-/* Allocate a new compressor */
+/*
+ * Allocate a new compressor.
+ */
 CompressorState *
 AllocateCompressor(const pg_compress_specification compression_spec,
-				   WriteFunc writeF)
+				   ReadFunc readF, WriteFunc writeF)
 {
 	CompressorState *cs;
 
-#ifndef HAVE_LIBZ
-	if (compression_spec.algorithm == PG_COMPRESSION_GZIP)
-		pg_fatal("this build does not support compression with %s", "gzip");
-#endif
-
 	cs = (CompressorState *) pg_malloc0(sizeof(CompressorState));
+	cs->readF = readF;
 	cs->writeF = writeF;
-	cs->compression_spec = compression_spec;
-
-	/*
-	 * Perform compression algorithm specific initialization.
-	 */
-#ifdef HAVE_LIBZ
-	if (cs->compression_spec.algorithm == PG_COMPRESSION_GZIP)
-		InitCompressorZlib(cs, cs->compression_spec.level);
-#endif
-
-	return cs;
-}
 
-/*
- * Read all compressed data from the input stream (via readF) and print it
- * out with ahwrite().
- */
-void
-ReadDataFromArchive(ArchiveHandle *AH,
-					const pg_compress_specification compression_spec,
-					ReadFunc readF)
-{
 	if (compression_spec.algorithm == PG_COMPRESSION_NONE)
-		ReadDataFromArchiveNone(AH, readF);
-	if (compression_spec.algorithm == PG_COMPRESSION_GZIP)
-	{
-#ifdef HAVE_LIBZ
-		ReadDataFromArchiveZlib(AH, readF);
-#else
-		pg_fatal("this build does not support compression with %s", "gzip");
-#endif
-	}
-}
+		InitCompressorNone(cs, compression_spec);
+	else if (compression_spec.algorithm == PG_COMPRESSION_GZIP)
+		InitCompressorGzip(cs, compression_spec);
 
-/*
- * Compress and write data to the output stream (via writeF).
- */
-void
-WriteDataToArchive(ArchiveHandle *AH, CompressorState *cs,
-				   const void *data, size_t dLen)
-{
-	switch (cs->compression_spec.algorithm)
-	{
-		case PG_COMPRESSION_GZIP:
-#ifdef HAVE_LIBZ
-			WriteDataToArchiveZlib(AH, cs, data, dLen);
-#else
-			pg_fatal("this build does not support compression with %s", "gzip");
-#endif
-			break;
-		case PG_COMPRESSION_NONE:
-			WriteDataToArchiveNone(AH, cs, data, dLen);
-			break;
-		case PG_COMPRESSION_LZ4:
-			/* fallthrough */
-		case PG_COMPRESSION_ZSTD:
-			pg_fatal("invalid compression method");
-			break;
-	}
+	return cs;
 }
 
 /*
@@ -177,233 +105,31 @@ WriteDataToArchive(ArchiveHandle *AH, CompressorState *cs,
 void
 EndCompressor(ArchiveHandle *AH, CompressorState *cs)
 {
-#ifdef HAVE_LIBZ
-	if (cs->compression_spec.algorithm == PG_COMPRESSION_GZIP)
-		EndCompressorZlib(AH, cs);
-#endif
-	free(cs);
+	cs->end(AH, cs);
+	pg_free(cs);
 }
 
-/* Private routines, specific to each compression method. */
-
-#ifdef HAVE_LIBZ
-/*
- * Functions for zlib compressed output.
- */
-
-static void
-InitCompressorZlib(CompressorState *cs, int level)
-{
-	z_streamp	zp;
-
-	zp = cs->zp = (z_streamp) pg_malloc(sizeof(z_stream));
-	zp->zalloc = Z_NULL;
-	zp->zfree = Z_NULL;
-	zp->opaque = Z_NULL;
-
-	/*
-	 * zlibOutSize is the buffer size we tell zlib it can output to.  We
-	 * actually allocate one extra byte because some routines want to append a
-	 * trailing zero byte to the zlib output.
-	 */
-	cs->zlibOut = (char *) pg_malloc(ZLIB_OUT_SIZE + 1);
-	cs->zlibOutSize = ZLIB_OUT_SIZE;
-
-	if (deflateInit(zp, level) != Z_OK)
-		pg_fatal("could not initialize compression library: %s",
-				 zp->msg);
-
-	/* Just be paranoid - maybe End is called after Start, with no Write */
-	zp->next_out = (void *) cs->zlibOut;
-	zp->avail_out = cs->zlibOutSize;
-}
-
-static void
-EndCompressorZlib(ArchiveHandle *AH, CompressorState *cs)
-{
-	z_streamp	zp = cs->zp;
-
-	zp->next_in = NULL;
-	zp->avail_in = 0;
-
-	/* Flush any remaining data from zlib buffer */
-	DeflateCompressorZlib(AH, cs, true);
-
-	if (deflateEnd(zp) != Z_OK)
-		pg_fatal("could not close compression stream: %s", zp->msg);
-
-	free(cs->zlibOut);
-	free(cs->zp);
-}
-
-static void
-DeflateCompressorZlib(ArchiveHandle *AH, CompressorState *cs, bool flush)
-{
-	z_streamp	zp = cs->zp;
-	char	   *out = cs->zlibOut;
-	int			res = Z_OK;
-
-	while (cs->zp->avail_in != 0 || flush)
-	{
-		res = deflate(zp, flush ? Z_FINISH : Z_NO_FLUSH);
-		if (res == Z_STREAM_ERROR)
-			pg_fatal("could not compress data: %s", zp->msg);
-		if ((flush && (zp->avail_out < cs->zlibOutSize))
-			|| (zp->avail_out == 0)
-			|| (zp->avail_in != 0)
-			)
-		{
-			/*
-			 * Extra paranoia: avoid zero-length chunks, since a zero length
-			 * chunk is the EOF marker in the custom format. This should never
-			 * happen but...
-			 */
-			if (zp->avail_out < cs->zlibOutSize)
-			{
-				/*
-				 * Any write function should do its own error checking but to
-				 * make sure we do a check here as well...
-				 */
-				size_t		len = cs->zlibOutSize - zp->avail_out;
-
-				cs->writeF(AH, out, len);
-			}
-			zp->next_out = (void *) out;
-			zp->avail_out = cs->zlibOutSize;
-		}
-
-		if (res == Z_STREAM_END)
-			break;
-	}
-}
-
-static void
-WriteDataToArchiveZlib(ArchiveHandle *AH, CompressorState *cs,
-					   const char *data, size_t dLen)
-{
-	cs->zp->next_in = (void *) unconstify(char *, data);
-	cs->zp->avail_in = dLen;
-	DeflateCompressorZlib(AH, cs, false);
-}
-
-static void
-ReadDataFromArchiveZlib(ArchiveHandle *AH, ReadFunc readF)
-{
-	z_streamp	zp;
-	char	   *out;
-	int			res = Z_OK;
-	size_t		cnt;
-	char	   *buf;
-	size_t		buflen;
-
-	zp = (z_streamp) pg_malloc(sizeof(z_stream));
-	zp->zalloc = Z_NULL;
-	zp->zfree = Z_NULL;
-	zp->opaque = Z_NULL;
-
-	buf = pg_malloc(ZLIB_IN_SIZE);
-	buflen = ZLIB_IN_SIZE;
-
-	out = pg_malloc(ZLIB_OUT_SIZE + 1);
-
-	if (inflateInit(zp) != Z_OK)
-		pg_fatal("could not initialize compression library: %s",
-				 zp->msg);
-
-	/* no minimal chunk size for zlib */
-	while ((cnt = readF(AH, &buf, &buflen)))
-	{
-		zp->next_in = (void *) buf;
-		zp->avail_in = cnt;
-
-		while (zp->avail_in > 0)
-		{
-			zp->next_out = (void *) out;
-			zp->avail_out = ZLIB_OUT_SIZE;
-
-			res = inflate(zp, 0);
-			if (res != Z_OK && res != Z_STREAM_END)
-				pg_fatal("could not uncompress data: %s", zp->msg);
-
-			out[ZLIB_OUT_SIZE - zp->avail_out] = '\0';
-			ahwrite(out, 1, ZLIB_OUT_SIZE - zp->avail_out, AH);
-		}
-	}
-
-	zp->next_in = NULL;
-	zp->avail_in = 0;
-	while (res != Z_STREAM_END)
-	{
-		zp->next_out = (void *) out;
-		zp->avail_out = ZLIB_OUT_SIZE;
-		res = inflate(zp, 0);
-		if (res != Z_OK && res != Z_STREAM_END)
-			pg_fatal("could not uncompress data: %s", zp->msg);
-
-		out[ZLIB_OUT_SIZE - zp->avail_out] = '\0';
-		ahwrite(out, 1, ZLIB_OUT_SIZE - zp->avail_out, AH);
-	}
-
-	if (inflateEnd(zp) != Z_OK)
-		pg_fatal("could not close compression library: %s", zp->msg);
-
-	free(buf);
-	free(out);
-	free(zp);
-}
-#endif							/* HAVE_LIBZ */
-
-
-/*
- * Functions for uncompressed output.
- */
-
-static void
-ReadDataFromArchiveNone(ArchiveHandle *AH, ReadFunc readF)
-{
-	size_t		cnt;
-	char	   *buf;
-	size_t		buflen;
-
-	buf = pg_malloc(ZLIB_OUT_SIZE);
-	buflen = ZLIB_OUT_SIZE;
-
-	while ((cnt = readF(AH, &buf, &buflen)))
-	{
-		ahwrite(buf, 1, cnt, AH);
-	}
-
-	free(buf);
-}
-
-static void
-WriteDataToArchiveNone(ArchiveHandle *AH, CompressorState *cs,
-					   const char *data, size_t dLen)
-{
-	cs->writeF(AH, data, dLen);
-}
-
-
 /*----------------------
  * Compressed stream API
  *----------------------
  */
 
 /*
- * cfp represents an open stream, wrapping the underlying FILE or gzFile
- * pointer. This is opaque to the callers.
+ * Private routines
  */
-struct cfp
+static int
+hasSuffix(const char *filename, const char *suffix)
 {
-	FILE	   *uncompressedfp;
-#ifdef HAVE_LIBZ
-	gzFile		compressedfp;
-#endif
-};
+	int			filenamelen = strlen(filename);
+	int			suffixlen = strlen(suffix);
 
-#ifdef HAVE_LIBZ
-static int	hasSuffix(const char *filename, const char *suffix);
-#endif
+	if (filenamelen < suffixlen)
+		return 0;
+
+	return memcmp(&filename[filenamelen - suffixlen],
+				  suffix,
+				  suffixlen) == 0;
+}
 
 /* free() without changing errno; useful in several places below */
 static void
@@ -416,324 +142,102 @@ free_keep_errno(void *p)
 }
 
 /*
- * Open a file for reading. 'path' is the file to open, and 'mode' should
- * be either "r" or "rb".
- *
- * If the file at 'path' does not exist, we append the ".gz" suffix (if 'path'
- * doesn't already have it) and try again. So if you pass "foo" as 'path',
- * this will open either "foo" or "foo.gz".
- *
- * On failure, return NULL with an error code in errno.
+ * Public interface
  */
-cfp *
-cfopen_read(const char *path, const char *mode)
-{
-	cfp		   *fp;
-
-	pg_compress_specification compression_spec = {0};
-
-#ifdef HAVE_LIBZ
-	if (hasSuffix(path, ".gz"))
-	{
-		compression_spec.algorithm = PG_COMPRESSION_GZIP;
-		fp = cfopen(path, mode, compression_spec);
-	}
-	else
-#endif
-	{
-		compression_spec.algorithm = PG_COMPRESSION_NONE;
-		fp = cfopen(path, mode, compression_spec);
-#ifdef HAVE_LIBZ
-		if (fp == NULL)
-		{
-			char	   *fname;
-
-			fname = psprintf("%s.gz", path);
-			compression_spec.algorithm = PG_COMPRESSION_GZIP;
-			fp = cfopen(fname, mode, compression_spec);
-			free_keep_errno(fname);
-		}
-#endif
-	}
-	return fp;
-}
 
 /*
- * Open a file for writing. 'path' indicates the path name, and 'mode' must
- * be a filemode as accepted by fopen() and gzopen() that indicates writing
- * ("w", "wb", "a", or "ab").
- *
- * If 'compression_spec.algorithm' is GZIP, a gzip compressed stream is opened,
- * and 'compression_spec.level' used. The ".gz" suffix is automatically added to
- * 'path' in that case.
- *
- * On failure, return NULL with an error code in errno.
- */
-cfp *
-cfopen_write(const char *path, const char *mode,
-			 const pg_compress_specification compression_spec)
-{
-	cfp		   *fp;
-
-	if (compression_spec.algorithm == PG_COMPRESSION_NONE)
-		fp = cfopen(path, mode, compression_spec);
-	else
-	{
-#ifdef HAVE_LIBZ
-		char	   *fname;
-
-		fname = psprintf("%s.gz", path);
-		fp = cfopen(fname, mode, compression_spec);
-		free_keep_errno(fname);
-#else
-		pg_fatal("this build does not support compression with %s", "gzip");
-		fp = NULL;				/* keep compiler quiet */
-#endif
-	}
-	return fp;
-}
-
-/*
- * This is the workhorse for cfopen() or cfdopen(). It opens file 'path' or
- * associates a stream 'fd', if 'fd' is a valid descriptor, in 'mode'. The
- * descriptor is not dup'ed and it is the caller's responsibility to do so.
- * The caller must verify that the 'compress_algorithm' is supported by the
- * current build.
- *
- * On failure, return NULL with an error code in errno.
+ * Initialize a compress file handle for the specified compression algorithm.
  */
-static cfp *
-cfopen_internal(const char *path, int fd, const char *mode,
-				pg_compress_specification compression_spec)
+CompressFileHandle *
+InitCompressFileHandle(const pg_compress_specification compression_spec)
 {
-	cfp		   *fp = pg_malloc0(sizeof(cfp));
+	CompressFileHandle *CFH;
 
-	if (compression_spec.algorithm == PG_COMPRESSION_GZIP)
-	{
-#ifdef HAVE_LIBZ
-		if (compression_spec.level != Z_DEFAULT_COMPRESSION)
-		{
-			/* user has specified a compression level, so tell zlib to use it */
-			char		mode_compression[32];
-
-			snprintf(mode_compression, sizeof(mode_compression), "%s%d",
-					 mode, compression_spec.level);
-			if (fd >= 0)
-				fp->compressedfp = gzdopen(fd, mode_compression);
-			else
-				fp->compressedfp = gzopen(path, mode_compression);
-		}
-		else
-		{
-			/* don't specify a level, just use the zlib default */
-			if (fd >= 0)
-				fp->compressedfp = gzdopen(fd, mode);
-			else
-				fp->compressedfp = gzopen(path, mode);
-		}
-
-		if (fp->compressedfp == NULL)
-		{
-			free_keep_errno(fp);
-			fp = NULL;
-		}
-#else
-		pg_fatal("this build does not support compression with %s", "gzip");
-#endif
-	}
-	else
-	{
-		if (fd >= 0)
-			fp->uncompressedfp = fdopen(fd, mode);
-		else
-			fp->uncompressedfp = fopen(path, mode);
+	CFH = pg_malloc0(sizeof(CompressFileHandle));
 
-		if (fp->uncompressedfp == NULL)
-		{
-			free_keep_errno(fp);
-			fp = NULL;
-		}
-	}
+	if (compression_spec.algorithm == PG_COMPRESSION_NONE)
+		InitCompressNone(CFH, compression_spec);
+	else if (compression_spec.algorithm == PG_COMPRESSION_GZIP)
+		InitCompressGzip(CFH, compression_spec);
 
-	return fp;
+	return CFH;
 }
 
 /*
- * Opens file 'path' in 'mode' and compression as defined in
- * compression_spec. The caller must verify that the compression
- * is supported by the current build.
+ * Open a file for reading. 'path' is the file to open, and 'mode' should
+ * be either "r" or "rb".
  *
- * On failure, return NULL with an error code in errno.
- */
-cfp *
-cfopen(const char *path, const char *mode,
-	   const pg_compress_specification compression_spec)
-{
-	return cfopen_internal(path, -1, mode, compression_spec);
-}
-
-/*
- * Associates a stream 'fd', if 'fd' is a valid descriptor, in 'mode'
- * and compression as defined in compression_spec. The caller must
- * verify that the compression is supported by the current build.
+ * If the file at 'path' contains the suffix of a supported compression method,
+ * currently this includes only ".gz", then this compression will be used
+ * throughout. Otherwise the compression will be deferred by iteratively trying
+ * to open the file at 'path', first as is, then by appending known compression
+ * suffixes. So if you pass "foo" as 'path', this will open either "foo" or
+ * "foo.gz", trying in that order.
  *
  * On failure, return NULL with an error code in errno.
  */
-cfp *
-cfdopen(int fd, const char *mode,
-		const pg_compress_specification compression_spec)
+CompressFileHandle *
+InitDiscoverCompressFileHandle(const char *path, const char *mode)
 {
-	return cfopen_internal(NULL, fd, mode, compression_spec);
-}
+	CompressFileHandle *CFH = NULL;
+	struct stat st;
+	char	   *fname;
+	pg_compress_specification compression_spec = {0};
 
-int
-cfread(void *ptr, int size, cfp *fp)
-{
-	int			ret;
+	compression_spec.algorithm = PG_COMPRESSION_NONE;
 
-	if (size == 0)
-		return 0;
+	Assert(strcmp(mode, "r") == 0 || strcmp(mode, "rb") == 0);
 
-#ifdef HAVE_LIBZ
-	if (fp->compressedfp)
-	{
-		ret = gzread(fp->compressedfp, ptr, size);
-		if (ret != size && !gzeof(fp->compressedfp))
-		{
-			int			errnum;
-			const char *errmsg = gzerror(fp->compressedfp, &errnum);
+	fname = strdup(path);
 
-			pg_fatal("could not read from input file: %s",
-					 errnum == Z_ERRNO ? strerror(errno) : errmsg);
-		}
-	}
+	if (hasSuffix(fname, ".gz"))
+		compression_spec.algorithm = PG_COMPRESSION_GZIP;
 	else
-#endif
 	{
-		ret = fread(ptr, 1, size, fp->uncompressedfp);
-		if (ret != size && !feof(fp->uncompressedfp))
-			READ_ERROR_EXIT(fp->uncompressedfp);
-	}
-	return ret;
-}
-
-int
-cfwrite(const void *ptr, int size, cfp *fp)
-{
-#ifdef HAVE_LIBZ
-	if (fp->compressedfp)
-		return gzwrite(fp->compressedfp, ptr, size);
-	else
-#endif
-		return fwrite(ptr, 1, size, fp->uncompressedfp);
-}
-
-int
-cfgetc(cfp *fp)
-{
-	int			ret;
+		bool		exists;
 
+		exists = (stat(path, &st) == 0);
+		/* avoid unused warning if it is not build with compression */
+		if (exists)
+			compression_spec.algorithm = PG_COMPRESSION_NONE;
 #ifdef HAVE_LIBZ
-	if (fp->compressedfp)
-	{
-		ret = gzgetc(fp->compressedfp);
-		if (ret == EOF)
+		if (!exists)
 		{
-			if (!gzeof(fp->compressedfp))
-				pg_fatal("could not read from input file: %s", strerror(errno));
-			else
-				pg_fatal("could not read from input file: end of file");
+			free_keep_errno(fname);
+			fname = psprintf("%s.gz", path);
+			exists = (stat(fname, &st) == 0);
+
+			if (exists)
+				compression_spec.algorithm = PG_COMPRESSION_GZIP;
 		}
-	}
-	else
 #endif
-	{
-		ret = fgetc(fp->uncompressedfp);
-		if (ret == EOF)
-			READ_ERROR_EXIT(fp->uncompressedfp);
 	}
 
-	return ret;
-}
-
-char *
-cfgets(cfp *fp, char *buf, int len)
-{
-#ifdef HAVE_LIBZ
-	if (fp->compressedfp)
-		return gzgets(fp->compressedfp, buf, len);
-	else
-#endif
-		return fgets(buf, len, fp->uncompressedfp);
-}
-
-int
-cfclose(cfp *fp)
-{
-	int			result;
-
-	if (fp == NULL)
-	{
-		errno = EBADF;
-		return EOF;
-	}
-#ifdef HAVE_LIBZ
-	if (fp->compressedfp)
+	CFH = InitCompressFileHandle(compression_spec);
+	if (CFH->open_func(fname, -1, mode, CFH))
 	{
-		result = gzclose(fp->compressedfp);
-		fp->compressedfp = NULL;
+		free_keep_errno(CFH);
+		CFH = NULL;
 	}
-	else
-#endif
-	{
-		result = fclose(fp->uncompressedfp);
-		fp->uncompressedfp = NULL;
-	}
-	free_keep_errno(fp);
+	free_keep_errno(fname);
 
-	return result;
+	return CFH;
 }
 
+/*
+ * Close an open file handle and release its memory.
+ *
+ * On failure, returns an error value and sets errno appropriately.
+ */
 int
-cfeof(cfp *fp)
+EndCompressFileHandle(CompressFileHandle *CFH)
 {
-#ifdef HAVE_LIBZ
-	if (fp->compressedfp)
-		return gzeof(fp->compressedfp);
-	else
-#endif
-		return feof(fp->uncompressedfp);
-}
+	int			ret = 0;
 
-const char *
-get_cfp_error(cfp *fp)
-{
-#ifdef HAVE_LIBZ
-	if (fp->compressedfp)
-	{
-		int			errnum;
-		const char *errmsg = gzerror(fp->compressedfp, &errnum);
+	if (CFH->private_data)
+		ret = CFH->close_func(CFH);
 
-		if (errnum != Z_ERRNO)
-			return errmsg;
-	}
-#endif
-	return strerror(errno);
-}
+	free_keep_errno(CFH);
 
-#ifdef HAVE_LIBZ
-static int
-hasSuffix(const char *filename, const char *suffix)
-{
-	int			filenamelen = strlen(filename);
-	int			suffixlen = strlen(suffix);
-
-	if (filenamelen < suffixlen)
-		return 0;
-
-	return memcmp(&filename[filenamelen - suffixlen],
-				  suffix,
-				  suffixlen) == 0;
+	return ret;
 }
-
-#endif
diff --git a/src/bin/pg_dump/compress_io.h b/src/bin/pg_dump/compress_io.h
index a6cdf588dd..a7a4fec036 100644
--- a/src/bin/pg_dump/compress_io.h
+++ b/src/bin/pg_dump/compress_io.h
@@ -21,50 +21,160 @@
 #define ZLIB_OUT_SIZE	4096
 #define ZLIB_IN_SIZE	4096
 
-/* Prototype for callback function to WriteDataToArchive() */
+/*
+ * Prototype for callback function used in writeData()
+ */
 typedef void (*WriteFunc) (ArchiveHandle *AH, const char *buf, size_t len);
 
 /*
- * Prototype for callback function to ReadDataFromArchive()
+ * Prototype for callback function used in readData()
  *
- * ReadDataFromArchive will call the read function repeatedly, until it
- * returns 0 to signal EOF. ReadDataFromArchive passes a buffer to read the
- * data into in *buf, of length *buflen. If that's not big enough for the
- * callback function, it can free() it and malloc() a new one, returning the
- * new buffer and its size in *buf and *buflen.
+ * readData will call the read function repeatedly, until it returns 0 to signal
+ * EOF. readData passes a buffer to read the data into in *buf, of length
+ * *buflen. If that's not big enough for the callback function, it can free() it
+ * and malloc() a new one, returning the new buffer and its size in *buf and
+ * *buflen.
  *
  * Returns the number of bytes read into *buf, or 0 on EOF.
  */
 typedef size_t (*ReadFunc) (ArchiveHandle *AH, char **buf, size_t *buflen);
 
-/* struct definition appears in compress_io.c */
 typedef struct CompressorState CompressorState;
+struct CompressorState
+{
+	/*
+	 * Read all compressed data from the input stream (via readF) and print it
+	 * out with ahwrite().
+	 */
+	void		(*readData) (ArchiveHandle *AH, CompressorState *cs);
+
+	/*
+	 * Compress and write data to the output stream (via writeF).
+	 */
+	void		(*writeData) (ArchiveHandle *AH, CompressorState *cs,
+							  const void *data, size_t dLen);
+
+	/*
+	 * End compression and flush internal buffers if any.
+	 */
+	void		(*end) (ArchiveHandle *AH, CompressorState *cs);
+
+	/*
+	 * Callback function to read from an already processed input stream
+	 */
+	ReadFunc	readF;
+
+	/*
+	 * Callback function to write an already processed chunk of data.
+	 */
+	WriteFunc	writeF;
+
+	/*
+	 * Compression specification for this state.
+	 */
+	pg_compress_specification compression_spec;
+
+	/*
+	 * Private data to be used by the compressor.
+	 */
+	void	   *private_data;
+};
 
 extern CompressorState *AllocateCompressor(const pg_compress_specification compression_spec,
+										   ReadFunc readF,
 										   WriteFunc writeF);
-extern void ReadDataFromArchive(ArchiveHandle *AH,
-								const pg_compress_specification compression_spec,
-								ReadFunc readF);
-extern void WriteDataToArchive(ArchiveHandle *AH, CompressorState *cs,
-							   const void *data, size_t dLen);
 extern void EndCompressor(ArchiveHandle *AH, CompressorState *cs);
 
+/*
+ * Compress File Handle
+ */
+typedef struct CompressFileHandle CompressFileHandle;
+
+struct CompressFileHandle
+{
+	/*
+	 * Open a file in mode.
+	 *
+	 * Pass either 'path' or 'fd' depending on whether a filepath or a file
+	 * descriptor is available. 'mode' can be one of 'r', 'rb', 'w', 'wb',
+	 * 'a', and 'ab'. Requrires an already initialized CompressFileHandle.
+	 */
+	int			(*open_func) (const char *path, int fd, const char *mode,
+							  CompressFileHandle *CFH);
+
+	/*
+	 * Open a file for writing.
+	 *
+	 * 'mode' can be one of ''w', 'wb', 'a', and 'ab'. Requrires an already
+	 * initialized CompressFileHandle.
+	 */
+	int			(*open_write_func) (const char *path, const char *mode,
+									CompressFileHandle *CFH);
 
-typedef struct cfp cfp;
+	/*
+	 * Read 'size' bytes of data from the file and store them into 'ptr'.
+	 */
+	size_t		(*read_func) (void *ptr, size_t size, CompressFileHandle *CFH);
 
-extern cfp *cfopen(const char *path, const char *mode,
-				   const pg_compress_specification compression_spec);
-extern cfp *cfdopen(int fd, const char *mode,
-					const pg_compress_specification compression_spec);
-extern cfp *cfopen_read(const char *path, const char *mode);
-extern cfp *cfopen_write(const char *path, const char *mode,
-						 const pg_compress_specification compression_spec);
-extern int	cfread(void *ptr, int size, cfp *fp);
-extern int	cfwrite(const void *ptr, int size, cfp *fp);
-extern int	cfgetc(cfp *fp);
-extern char *cfgets(cfp *fp, char *buf, int len);
-extern int	cfclose(cfp *fp);
-extern int	cfeof(cfp *fp);
-extern const char *get_cfp_error(cfp *fp);
+	/*
+	 * Write 'size' bytes of data into the file from 'ptr'.
+	 */
+	size_t		(*write_func) (const void *ptr, size_t size,
+							   struct CompressFileHandle *CFH);
 
+	/*
+	 * Read at most size - 1 characters from the compress file handle into
+	 * 's'.
+	 *
+	 * Stop if an EOF or a newline is found first. 's' is always null
+	 * terminated and contains the newline if it was found.
+	 */
+	char	   *(*gets_func) (char *s, int size, CompressFileHandle *CFH);
+
+	/*
+	 * Read the next character from the compress file handle as 'unsigned
+	 * char' cast into 'int'.
+	 */
+	int			(*getc_func) (CompressFileHandle *CFH);
+
+	/*
+	 * Test if EOF is reached in the compress file handle.
+	 */
+	int			(*eof_func) (CompressFileHandle *CFH);
+
+	/*
+	 * Close an open file handle.
+	 */
+	int			(*close_func) (CompressFileHandle *CFH);
+
+	/*
+	 * Get a pointer to a string that describes an error that occured during a
+	 * compress file handle operation.
+	 */
+	const char *(*get_error_func) (CompressFileHandle *CFH);
+
+	/*
+	 * Compression specification for this file handle.
+	 */
+	pg_compress_specification compression_spec;
+
+	/*
+	 * Private data to be used by the compressor.
+	 */
+	void	   *private_data;
+};
+
+/*
+ * Initialize a compress file handle with the requested compression.
+ */
+extern CompressFileHandle *InitCompressFileHandle(const pg_compress_specification compression_spec);
+
+/*
+ * Initialize a compress file stream. Deffer the compression algorithm
+ * from 'path', either by examining its suffix or by appending the supported
+ * suffixes in 'path'.
+ */
+extern CompressFileHandle *InitDiscoverCompressFileHandle(const char *path,
+														  const char *mode);
+extern int	EndCompressFileHandle(CompressFileHandle *CFH);
 #endif
diff --git a/src/bin/pg_dump/compress_none.c b/src/bin/pg_dump/compress_none.c
new file mode 100644
index 0000000000..3a36f94898
--- /dev/null
+++ b/src/bin/pg_dump/compress_none.c
@@ -0,0 +1,206 @@
+/*-------------------------------------------------------------------------
+ *
+ * compress_none.c
+ *	 Routines for archivers to read or write an uncompressed stream.
+ *
+ * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * IDENTIFICATION
+ *	   src/bin/pg_dump/compress_none.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "postgres_fe.h"
+#include <unistd.h>
+
+#include "compress_none.h"
+#include "pg_backup_utils.h"
+
+/*----------------------
+ * Compressor API
+ *----------------------
+ */
+
+/*
+ * Private routines
+ */
+
+static void
+ReadDataFromArchiveNone(ArchiveHandle *AH, CompressorState *cs)
+{
+	size_t		cnt;
+	char	   *buf;
+	size_t		buflen;
+
+	buf = pg_malloc(ZLIB_OUT_SIZE);
+	buflen = ZLIB_OUT_SIZE;
+
+	while ((cnt = cs->readF(AH, &buf, &buflen)))
+	{
+		ahwrite(buf, 1, cnt, AH);
+	}
+
+	free(buf);
+}
+
+
+static void
+WriteDataToArchiveNone(ArchiveHandle *AH, CompressorState *cs,
+					   const void *data, size_t dLen)
+{
+	cs->writeF(AH, data, dLen);
+}
+
+static void
+EndCompressorNone(ArchiveHandle *AH, CompressorState *cs)
+{
+	/* no op */
+}
+
+/*
+ * Public interface
+ */
+
+void
+InitCompressorNone(CompressorState *cs,
+				   const pg_compress_specification compression_spec)
+{
+	cs->readData = ReadDataFromArchiveNone;
+	cs->writeData = WriteDataToArchiveNone;
+	cs->end = EndCompressorNone;
+
+	cs->compression_spec = compression_spec;
+}
+
+
+/*----------------------
+ * Compress File API
+ *----------------------
+ */
+
+/*
+ * Private routines
+ */
+
+static size_t
+read_none(void *ptr, size_t size, CompressFileHandle *CFH)
+{
+	FILE	   *fp = (FILE *) CFH->private_data;
+	size_t		ret;
+
+	if (size == 0)
+		return 0;
+
+	ret = fread(ptr, 1, size, fp);
+	if (ret != size && !feof(fp))
+		pg_fatal("could not read from input file: %s",
+				 strerror(errno));
+
+	return ret;
+}
+
+static size_t
+write_none(const void *ptr, size_t size, CompressFileHandle *CFH)
+{
+	return fwrite(ptr, 1, size, (FILE *) CFH->private_data);
+}
+
+static const char *
+get_error_none(CompressFileHandle *CFH)
+{
+	return strerror(errno);
+}
+
+static char *
+gets_none(char *ptr, int size, CompressFileHandle *CFH)
+{
+	return fgets(ptr, size, (FILE *) CFH->private_data);
+}
+
+static int
+getc_none(CompressFileHandle *CFH)
+{
+	FILE	   *fp = (FILE *) CFH->private_data;
+	int			ret;
+
+	ret = fgetc(fp);
+	if (ret == EOF)
+	{
+		if (!feof(fp))
+			pg_fatal("could not read from input file: %s", strerror(errno));
+		else
+			pg_fatal("could not read from input file: end of file");
+	}
+
+	return ret;
+}
+
+static int
+close_none(CompressFileHandle *CFH)
+{
+	FILE	   *fp = (FILE *) CFH->private_data;
+	int			ret = 0;
+
+	CFH->private_data = NULL;
+
+	if (fp)
+		ret = fclose(fp);
+
+	return ret;
+}
+
+static int
+eof_none(CompressFileHandle *CFH)
+{
+	return feof((FILE *) CFH->private_data);
+}
+
+static int
+open_none(const char *path, int fd, const char *mode, CompressFileHandle *CFH)
+{
+	Assert(CFH->private_data == NULL);
+
+	if (fd >= 0)
+		CFH->private_data = fdopen(dup(fd), mode);
+	else
+		CFH->private_data = fopen(path, mode);
+
+	if (CFH->private_data == NULL)
+		return 1;
+
+	return 0;
+}
+
+static int
+open_write_none(const char *path, const char *mode, CompressFileHandle *CFH)
+{
+	Assert(CFH->private_data == NULL);
+
+	CFH->private_data = fopen(path, mode);
+	if (CFH->private_data == NULL)
+		return 1;
+
+	return 0;
+}
+
+/*
+ * Public interface
+ */
+
+void
+InitCompressNone(CompressFileHandle *CFH,
+				 const pg_compress_specification compression_spec)
+{
+	CFH->open_func = open_none;
+	CFH->open_write_func = open_write_none;
+	CFH->read_func = read_none;
+	CFH->write_func = write_none;
+	CFH->gets_func = gets_none;
+	CFH->getc_func = getc_none;
+	CFH->close_func = close_none;
+	CFH->eof_func = eof_none;
+	CFH->get_error_func = get_error_none;
+
+	CFH->private_data = NULL;
+}
diff --git a/src/bin/pg_dump/compress_none.h b/src/bin/pg_dump/compress_none.h
new file mode 100644
index 0000000000..0e704c5491
--- /dev/null
+++ b/src/bin/pg_dump/compress_none.h
@@ -0,0 +1,24 @@
+/*-------------------------------------------------------------------------
+ *
+ * compress_none.h
+ *	 Uncompressed interface to compress_io.c routines
+ *
+ * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * IDENTIFICATION
+ *	   src/bin/pg_dump/compress_none.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef _COMPRESS_NONE_H_
+#define _COMPRESS_NONE_H_
+
+#include "compress_io.h"
+
+extern void InitCompressorNone(CompressorState *cs,
+							   const pg_compress_specification compression_spec);
+extern void InitCompressNone(CompressFileHandle *CFH,
+							 const pg_compress_specification compression_spec);
+
+#endif							/* _COMPRESS_NONE_H_ */
diff --git a/src/bin/pg_dump/meson.build b/src/bin/pg_dump/meson.build
index ca62f9a374..84e9f0defa 100644
--- a/src/bin/pg_dump/meson.build
+++ b/src/bin/pg_dump/meson.build
@@ -1,7 +1,9 @@
 # Copyright (c) 2022-2023, PostgreSQL Global Development Group
 
 pg_dump_common_sources = files(
+  'compress_gzip.c',
   'compress_io.c',
+  'compress_none.c',
   'dumputils.c',
   'parallel.c',
   'pg_backup_archiver.c',
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index ddb712d042..febdd14885 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -95,8 +95,8 @@ static void dump_lo_buf(ArchiveHandle *AH);
 static void dumpTimestamp(ArchiveHandle *AH, const char *msg, time_t tim);
 static void SetOutput(ArchiveHandle *AH, const char *filename,
 					  const pg_compress_specification compression_spec);
-static cfp *SaveOutput(ArchiveHandle *AH);
-static void RestoreOutput(ArchiveHandle *AH, cfp *savedOutput);
+static CompressFileHandle *SaveOutput(ArchiveHandle *AH);
+static void RestoreOutput(ArchiveHandle *AH, CompressFileHandle *savedOutput);
 
 static int	restore_toc_entry(ArchiveHandle *AH, TocEntry *te, bool is_parallel);
 static void restore_toc_entries_prefork(ArchiveHandle *AH,
@@ -272,7 +272,7 @@ CloseArchive(Archive *AHX)
 
 	/* Close the output */
 	errno = 0;
-	res = cfclose(AH->OF);
+	res = EndCompressFileHandle(AH->OF);
 
 	if (res != 0)
 		pg_fatal("could not close output file: %m");
@@ -355,7 +355,7 @@ RestoreArchive(Archive *AHX)
 	bool		parallel_mode;
 	bool		supports_compression;
 	TocEntry   *te;
-	cfp		   *sav;
+	CompressFileHandle *sav;
 
 	AH->stage = STAGE_INITIALIZING;
 
@@ -1135,7 +1135,7 @@ PrintTOCSummary(Archive *AHX)
 	TocEntry   *te;
 	pg_compress_specification out_compression_spec = {0};
 	teSection	curSection;
-	cfp		   *sav;
+	CompressFileHandle *sav;
 	const char *fmtName;
 	char		stamp_str[64];
 
@@ -1151,9 +1151,10 @@ PrintTOCSummary(Archive *AHX)
 		strcpy(stamp_str, "[unknown]");
 
 	ahprintf(AH, ";\n; Archive created at %s\n", stamp_str);
-	ahprintf(AH, ";     dbname: %s\n;     TOC Entries: %d\n;     Compression: %d\n",
+	ahprintf(AH, ";     dbname: %s\n;     TOC Entries: %d\n;     Compression: %s\n",
 			 sanitize_line(AH->archdbname, false),
-			 AH->tocCount, AH->compression_spec.level);
+			 AH->tocCount,
+			 get_compress_algorithm_name(AH->compression_spec.algorithm));
 
 	switch (AH->format)
 	{
@@ -1510,6 +1511,7 @@ static void
 SetOutput(ArchiveHandle *AH, const char *filename,
 		  const pg_compress_specification compression_spec)
 {
+	CompressFileHandle *CFH;
 	const char *mode;
 	int			fn = -1;
 
@@ -1532,33 +1534,32 @@ SetOutput(ArchiveHandle *AH, const char *filename,
 	else
 		mode = PG_BINARY_W;
 
-	if (fn >= 0)
-		AH->OF = cfdopen(dup(fn), mode, compression_spec);
-	else
-		AH->OF = cfopen(filename, mode, compression_spec);
+	CFH = InitCompressFileHandle(compression_spec);
 
-	if (!AH->OF)
+	if (CFH->open_func(filename, fn, mode, CFH))
 	{
 		if (filename)
 			pg_fatal("could not open output file \"%s\": %m", filename);
 		else
 			pg_fatal("could not open output file: %m");
 	}
+
+	AH->OF = CFH;
 }
 
-static cfp *
+static CompressFileHandle *
 SaveOutput(ArchiveHandle *AH)
 {
-	return (cfp *) AH->OF;
+	return (CompressFileHandle *) AH->OF;
 }
 
 static void
-RestoreOutput(ArchiveHandle *AH, cfp *savedOutput)
+RestoreOutput(ArchiveHandle *AH, CompressFileHandle *savedOutput)
 {
 	int			res;
 
 	errno = 0;
-	res = cfclose(AH->OF);
+	res = EndCompressFileHandle(AH->OF);
 
 	if (res != 0)
 		pg_fatal("could not close output file: %m");
@@ -1697,7 +1698,11 @@ ahwrite(const void *ptr, size_t size, size_t nmemb, ArchiveHandle *AH)
 	else if (RestoringToDB(AH))
 		bytes_written = ExecuteSqlCommandBuf(&AH->public, (const char *) ptr, size * nmemb);
 	else
-		bytes_written = cfwrite(ptr, size * nmemb, AH->OF);
+	{
+		CompressFileHandle *CFH = (CompressFileHandle *) AH->OF;
+
+		bytes_written = CFH->write_func(ptr, size * nmemb, CFH);
+	}
 
 	if (bytes_written != size * nmemb)
 		WRITE_ERROR_EXIT;
@@ -2039,6 +2044,18 @@ ReadStr(ArchiveHandle *AH)
 	return buf;
 }
 
+static bool
+_fileExistsInDirectory(const char *dir, const char *filename)
+{
+	struct stat st;
+	char		buf[MAXPGPATH];
+
+	if (snprintf(buf, MAXPGPATH, "%s/%s", dir, filename) >= MAXPGPATH)
+		pg_fatal("directory name too long: \"%s\"", dir);
+
+	return (stat(buf, &st) == 0 && S_ISREG(st.st_mode));
+}
+
 static int
 _discoverArchiveFormat(ArchiveHandle *AH)
 {
@@ -2069,26 +2086,12 @@ _discoverArchiveFormat(ArchiveHandle *AH)
 		 */
 		if (stat(AH->fSpec, &st) == 0 && S_ISDIR(st.st_mode))
 		{
-			char		buf[MAXPGPATH];
-
-			if (snprintf(buf, MAXPGPATH, "%s/toc.dat", AH->fSpec) >= MAXPGPATH)
-				pg_fatal("directory name too long: \"%s\"",
-						 AH->fSpec);
-			if (stat(buf, &st) == 0 && S_ISREG(st.st_mode))
-			{
-				AH->format = archDirectory;
+			AH->format = archDirectory;
+			if (_fileExistsInDirectory(AH->fSpec, "toc.dat"))
 				return AH->format;
-			}
-
 #ifdef HAVE_LIBZ
-			if (snprintf(buf, MAXPGPATH, "%s/toc.dat.gz", AH->fSpec) >= MAXPGPATH)
-				pg_fatal("directory name too long: \"%s\"",
-						 AH->fSpec);
-			if (stat(buf, &st) == 0 && S_ISREG(st.st_mode))
-			{
-				AH->format = archDirectory;
+			if (_fileExistsInDirectory(AH->fSpec, "toc.dat.gz"))
 				return AH->format;
-			}
 #endif
 			pg_fatal("directory \"%s\" does not appear to be a valid archive (\"toc.dat\" does not exist)",
 					 AH->fSpec);
@@ -2186,6 +2189,7 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,
 		 SetupWorkerPtrType setupWorkerPtr)
 {
 	ArchiveHandle *AH;
+	CompressFileHandle *CFH;
 	pg_compress_specification out_compress_spec = {0};
 
 	pg_log_debug("allocating AH for %s, format %d",
@@ -2241,7 +2245,10 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,
 
 	/* Open stdout with no compression for AH output handle */
 	out_compress_spec.algorithm = PG_COMPRESSION_NONE;
-	AH->OF = cfdopen(dup(fileno(stdout)), PG_BINARY_A, out_compress_spec);
+	CFH = InitCompressFileHandle(out_compress_spec);
+	if (CFH->open_func(NULL, fileno(stdout), PG_BINARY_A, CFH))
+		pg_fatal("could not open stdout for appending: %m");
+	AH->OF = CFH;
 
 	/*
 	 * On Windows, we need to use binary mode to read/write non-text files,
@@ -3654,12 +3661,7 @@ WriteHead(ArchiveHandle *AH)
 	AH->WriteBytePtr(AH, AH->intSize);
 	AH->WriteBytePtr(AH, AH->offSize);
 	AH->WriteBytePtr(AH, AH->format);
-	/*
-	 * For now the compression type is implied by the level.  This will need
-	 * to change once support for more compression algorithms is added,
-	 * requiring a format bump.
-	 */
-	WriteInt(AH, AH->compression_spec.level);
+	AH->WriteBytePtr(AH, AH->compression_spec.algorithm);
 	crtm = *localtime(&AH->createDate);
 	WriteInt(AH, crtm.tm_sec);
 	WriteInt(AH, crtm.tm_min);
@@ -3730,10 +3732,11 @@ ReadHead(ArchiveHandle *AH)
 		pg_fatal("expected format (%d) differs from format found in file (%d)",
 				 AH->format, fmt);
 
-	/* Guess the compression method based on the level */
-	AH->compression_spec.algorithm = PG_COMPRESSION_NONE;
-	if (AH->version >= K_VERS_1_2)
+	if (AH->version >= K_VERS_1_15)
+		AH->compression_spec.algorithm = AH->ReadBytePtr(AH);
+	else if (AH->version >= K_VERS_1_2)
 	{
+		/* Guess the compression method based on the level */
 		if (AH->version < K_VERS_1_4)
 			AH->compression_spec.level = AH->ReadBytePtr(AH);
 		else
@@ -3745,10 +3748,17 @@ ReadHead(ArchiveHandle *AH)
 	else
 		AH->compression_spec.algorithm = PG_COMPRESSION_GZIP;
 
+	if (AH->compression_spec.algorithm != PG_COMPRESSION_NONE)
+	{
+		bool		unsupported = false;
+
 #ifndef HAVE_LIBZ
-	if (AH->compression_spec.algorithm == PG_COMPRESSION_GZIP)
-		pg_fatal("archive is compressed, but this installation does not support compression");
+		if (AH->compression_spec.algorithm == PG_COMPRESSION_GZIP)
+			unsupported = true;
 #endif
+		if (unsupported)
+			pg_fatal("archive is compressed, but this installation does not support compression");
+	}
 
 	if (AH->version >= K_VERS_1_4)
 	{
diff --git a/src/bin/pg_dump/pg_backup_archiver.h b/src/bin/pg_dump/pg_backup_archiver.h
index 4725e49747..18b38c17ab 100644
--- a/src/bin/pg_dump/pg_backup_archiver.h
+++ b/src/bin/pg_dump/pg_backup_archiver.h
@@ -65,10 +65,13 @@
 #define K_VERS_1_13 MAKE_ARCHIVE_VERSION(1, 13, 0)	/* change search_path
 													 * behavior */
 #define K_VERS_1_14 MAKE_ARCHIVE_VERSION(1, 14, 0)	/* add tableam */
+#define K_VERS_1_15 MAKE_ARCHIVE_VERSION(1, 15, 0)	/* add
+													 * compression_algorithm
+													 * in header */
 
 /* Current archive version number (the format we can output) */
 #define K_VERS_MAJOR 1
-#define K_VERS_MINOR 14
+#define K_VERS_MINOR 15
 #define K_VERS_REV 0
 #define K_VERS_SELF MAKE_ARCHIVE_VERSION(K_VERS_MAJOR, K_VERS_MINOR, K_VERS_REV)
 
diff --git a/src/bin/pg_dump/pg_backup_custom.c b/src/bin/pg_dump/pg_backup_custom.c
index d1e54644a9..512ab043af 100644
--- a/src/bin/pg_dump/pg_backup_custom.c
+++ b/src/bin/pg_dump/pg_backup_custom.c
@@ -298,7 +298,9 @@ _StartData(ArchiveHandle *AH, TocEntry *te)
 	_WriteByte(AH, BLK_DATA);	/* Block type */
 	WriteInt(AH, te->dumpId);	/* For sanity check */
 
-	ctx->cs = AllocateCompressor(AH->compression_spec, _CustomWriteFunc);
+	ctx->cs = AllocateCompressor(AH->compression_spec,
+								 NULL,
+								 _CustomWriteFunc);
 }
 
 /*
@@ -317,15 +319,15 @@ _WriteData(ArchiveHandle *AH, const void *data, size_t dLen)
 	CompressorState *cs = ctx->cs;
 
 	if (dLen > 0)
-		/* WriteDataToArchive() internally throws write errors */
-		WriteDataToArchive(AH, cs, data, dLen);
+		/* writeData() internally throws write errors */
+		cs->writeData(AH, cs, data, dLen);
 }
 
 /*
  * Called by the archiver when a dumper's 'DataDumper' routine has
  * finished.
  *
- * Optional.
+ * Mandatory.
  */
 static void
 _EndData(ArchiveHandle *AH, TocEntry *te)
@@ -333,6 +335,8 @@ _EndData(ArchiveHandle *AH, TocEntry *te)
 	lclContext *ctx = (lclContext *) AH->formatData;
 
 	EndCompressor(AH, ctx->cs);
+	ctx->cs = NULL;
+
 	/* Send the end marker */
 	WriteInt(AH, 0);
 }
@@ -377,7 +381,9 @@ _StartLO(ArchiveHandle *AH, TocEntry *te, Oid oid)
 
 	WriteInt(AH, oid);
 
-	ctx->cs = AllocateCompressor(AH->compression_spec, _CustomWriteFunc);
+	ctx->cs = AllocateCompressor(AH->compression_spec,
+								 NULL,
+								 _CustomWriteFunc);
 }
 
 /*
@@ -566,7 +572,12 @@ _PrintTocData(ArchiveHandle *AH, TocEntry *te)
 static void
 _PrintData(ArchiveHandle *AH)
 {
-	ReadDataFromArchive(AH, AH->compression_spec, _CustomReadFunc);
+	CompressorState *cs;
+
+	cs = AllocateCompressor(AH->compression_spec,
+							_CustomReadFunc, NULL);
+	cs->readData(AH, cs);
+	EndCompressor(AH, cs);
 }
 
 static void
diff --git a/src/bin/pg_dump/pg_backup_directory.c b/src/bin/pg_dump/pg_backup_directory.c
index 6800c3ccee..a2f88995c0 100644
--- a/src/bin/pg_dump/pg_backup_directory.c
+++ b/src/bin/pg_dump/pg_backup_directory.c
@@ -50,9 +50,8 @@ typedef struct
 	 */
 	char	   *directory;
 
-	cfp		   *dataFH;			/* currently open data file */
-
-	cfp		   *LOsTocFH;		/* file handle for blobs.toc */
+	CompressFileHandle *dataFH; /* currently open data file */
+	CompressFileHandle *LOsTocFH; /* file handle for blobs.toc */
 	ParallelState *pstate;		/* for parallel backup / restore */
 } lclContext;
 
@@ -198,11 +197,11 @@ InitArchiveFmt_Directory(ArchiveHandle *AH)
 	else
 	{							/* Read Mode */
 		char		fname[MAXPGPATH];
-		cfp		   *tocFH;
+		CompressFileHandle *tocFH;
 
 		setFilePath(AH, fname, "toc.dat");
 
-		tocFH = cfopen_read(fname, PG_BINARY_R);
+		tocFH = InitDiscoverCompressFileHandle(fname, PG_BINARY_R);
 		if (tocFH == NULL)
 			pg_fatal("could not open input file \"%s\": %m", fname);
 
@@ -218,7 +217,7 @@ InitArchiveFmt_Directory(ArchiveHandle *AH)
 		ReadToc(AH);
 
 		/* Nothing else in the file, so close it again... */
-		if (cfclose(tocFH) != 0)
+		if (EndCompressFileHandle(tocFH) != 0)
 			pg_fatal("could not close TOC file: %m");
 		ctx->dataFH = NULL;
 	}
@@ -327,9 +326,9 @@ _StartData(ArchiveHandle *AH, TocEntry *te)
 
 	setFilePath(AH, fname, tctx->filename);
 
-	ctx->dataFH = cfopen_write(fname, PG_BINARY_W,
-							   AH->compression_spec);
-	if (ctx->dataFH == NULL)
+	ctx->dataFH = InitCompressFileHandle(AH->compression_spec);
+
+	if (ctx->dataFH->open_write_func(fname, PG_BINARY_W, ctx->dataFH))
 		pg_fatal("could not open output file \"%s\": %m", fname);
 }
 
@@ -346,15 +345,16 @@ static void
 _WriteData(ArchiveHandle *AH, const void *data, size_t dLen)
 {
 	lclContext *ctx = (lclContext *) AH->formatData;
+	CompressFileHandle *CFH = ctx->dataFH;
 
 	errno = 0;
-	if (dLen > 0 && cfwrite(data, dLen, ctx->dataFH) != dLen)
+	if (dLen > 0 && CFH->write_func(data, dLen, CFH) != dLen)
 	{
 		/* if write didn't set errno, assume problem is no disk space */
 		if (errno == 0)
 			errno = ENOSPC;
 		pg_fatal("could not write to output file: %s",
-				 get_cfp_error(ctx->dataFH));
+				 CFH->get_error_func(CFH));
 	}
 }
 
@@ -370,7 +370,7 @@ _EndData(ArchiveHandle *AH, TocEntry *te)
 	lclContext *ctx = (lclContext *) AH->formatData;
 
 	/* Close the file */
-	if (cfclose(ctx->dataFH) != 0)
+	if (EndCompressFileHandle(ctx->dataFH) != 0)
 		pg_fatal("could not close data file: %m");
 
 	ctx->dataFH = NULL;
@@ -385,26 +385,25 @@ _PrintFileData(ArchiveHandle *AH, char *filename)
 	size_t		cnt;
 	char	   *buf;
 	size_t		buflen;
-	cfp		   *cfp;
+	CompressFileHandle *CFH;
 
 	if (!filename)
 		return;
 
-	cfp = cfopen_read(filename, PG_BINARY_R);
-
-	if (!cfp)
+	CFH = InitDiscoverCompressFileHandle(filename, PG_BINARY_R);
+	if (!CFH)
 		pg_fatal("could not open input file \"%s\": %m", filename);
 
 	buf = pg_malloc(ZLIB_OUT_SIZE);
 	buflen = ZLIB_OUT_SIZE;
 
-	while ((cnt = cfread(buf, buflen, cfp)))
+	while ((cnt = CFH->read_func(buf, buflen, CFH)))
 	{
 		ahwrite(buf, 1, cnt, AH);
 	}
 
 	free(buf);
-	if (cfclose(cfp) != 0)
+	if (EndCompressFileHandle(CFH) != 0)
 		pg_fatal("could not close data file \"%s\": %m", filename);
 }
 
@@ -435,6 +434,7 @@ _LoadLOs(ArchiveHandle *AH)
 {
 	Oid			oid;
 	lclContext *ctx = (lclContext *) AH->formatData;
+	CompressFileHandle *CFH;
 	char		tocfname[MAXPGPATH];
 	char		line[MAXPGPATH];
 
@@ -442,14 +442,14 @@ _LoadLOs(ArchiveHandle *AH)
 
 	setFilePath(AH, tocfname, "blobs.toc");
 
-	ctx->LOsTocFH = cfopen_read(tocfname, PG_BINARY_R);
+	CFH = ctx->LOsTocFH = InitDiscoverCompressFileHandle(tocfname, PG_BINARY_R);
 
 	if (ctx->LOsTocFH == NULL)
 		pg_fatal("could not open large object TOC file \"%s\" for input: %m",
 				 tocfname);
 
 	/* Read the LOs TOC file line-by-line, and process each LO */
-	while ((cfgets(ctx->LOsTocFH, line, MAXPGPATH)) != NULL)
+	while ((CFH->gets_func(line, MAXPGPATH, CFH)) != NULL)
 	{
 		char		lofname[MAXPGPATH + 1];
 		char		path[MAXPGPATH];
@@ -464,11 +464,11 @@ _LoadLOs(ArchiveHandle *AH)
 		_PrintFileData(AH, path);
 		EndRestoreLO(AH, oid);
 	}
-	if (!cfeof(ctx->LOsTocFH))
+	if (!CFH->eof_func(CFH))
 		pg_fatal("error reading large object TOC file \"%s\"",
 				 tocfname);
 
-	if (cfclose(ctx->LOsTocFH) != 0)
+	if (EndCompressFileHandle(ctx->LOsTocFH) != 0)
 		pg_fatal("could not close large object TOC file \"%s\": %m",
 				 tocfname);
 
@@ -488,15 +488,16 @@ _WriteByte(ArchiveHandle *AH, const int i)
 {
 	unsigned char c = (unsigned char) i;
 	lclContext *ctx = (lclContext *) AH->formatData;
+	CompressFileHandle *CFH = ctx->dataFH;
 
 	errno = 0;
-	if (cfwrite(&c, 1, ctx->dataFH) != 1)
+	if (CFH->write_func(&c, 1, CFH) != 1)
 	{
 		/* if write didn't set errno, assume problem is no disk space */
 		if (errno == 0)
 			errno = ENOSPC;
 		pg_fatal("could not write to output file: %s",
-				 get_cfp_error(ctx->dataFH));
+				 CFH->get_error_func(CFH));
 	}
 
 	return 1;
@@ -512,8 +513,9 @@ static int
 _ReadByte(ArchiveHandle *AH)
 {
 	lclContext *ctx = (lclContext *) AH->formatData;
+	CompressFileHandle *CFH = ctx->dataFH;
 
-	return cfgetc(ctx->dataFH);
+	return CFH->getc_func(CFH);
 }
 
 /*
@@ -524,15 +526,16 @@ static void
 _WriteBuf(ArchiveHandle *AH, const void *buf, size_t len)
 {
 	lclContext *ctx = (lclContext *) AH->formatData;
+	CompressFileHandle *CFH = ctx->dataFH;
 
 	errno = 0;
-	if (cfwrite(buf, len, ctx->dataFH) != len)
+	if (CFH->write_func(buf, len, CFH) != len)
 	{
 		/* if write didn't set errno, assume problem is no disk space */
 		if (errno == 0)
 			errno = ENOSPC;
 		pg_fatal("could not write to output file: %s",
-				 get_cfp_error(ctx->dataFH));
+				 CFH->get_error_func(CFH));
 	}
 }
 
@@ -545,12 +548,13 @@ static void
 _ReadBuf(ArchiveHandle *AH, void *buf, size_t len)
 {
 	lclContext *ctx = (lclContext *) AH->formatData;
+	CompressFileHandle *CFH = ctx->dataFH;
 
 	/*
-	 * If there was an I/O error, we already exited in cfread(), so here we
+	 * If there was an I/O error, we already exited in readF(), so here we
 	 * exit on short reads.
 	 */
-	if (cfread(buf, len, ctx->dataFH) != len)
+	if (CFH->read_func(buf, len, CFH) != len)
 		pg_fatal("could not read from input file: end of file");
 }
 
@@ -573,7 +577,7 @@ _CloseArchive(ArchiveHandle *AH)
 
 	if (AH->mode == archModeWrite)
 	{
-		cfp		   *tocFH;
+		CompressFileHandle *tocFH;
 		pg_compress_specification compression_spec = {0};
 		char		fname[MAXPGPATH];
 
@@ -584,8 +588,8 @@ _CloseArchive(ArchiveHandle *AH)
 
 		/* The TOC is always created uncompressed */
 		compression_spec.algorithm = PG_COMPRESSION_NONE;
-		tocFH = cfopen_write(fname, PG_BINARY_W, compression_spec);
-		if (tocFH == NULL)
+		tocFH = InitCompressFileHandle(compression_spec);
+		if (tocFH->open_write_func(fname, PG_BINARY_W, tocFH))
 			pg_fatal("could not open output file \"%s\": %m", fname);
 		ctx->dataFH = tocFH;
 
@@ -598,7 +602,7 @@ _CloseArchive(ArchiveHandle *AH)
 		WriteHead(AH);
 		AH->format = archDirectory;
 		WriteToc(AH);
-		if (cfclose(tocFH) != 0)
+		if (EndCompressFileHandle(tocFH) != 0)
 			pg_fatal("could not close TOC file: %m");
 		WriteDataChunks(AH, ctx->pstate);
 
@@ -649,8 +653,8 @@ _StartLOs(ArchiveHandle *AH, TocEntry *te)
 
 	/* The LO TOC file is never compressed */
 	compression_spec.algorithm = PG_COMPRESSION_NONE;
-	ctx->LOsTocFH = cfopen_write(fname, "ab", compression_spec);
-	if (ctx->LOsTocFH == NULL)
+	ctx->LOsTocFH = InitCompressFileHandle(compression_spec);
+	if (ctx->LOsTocFH->open_write_func(fname, "ab", ctx->LOsTocFH))
 		pg_fatal("could not open output file \"%s\": %m", fname);
 }
 
@@ -667,9 +671,8 @@ _StartLO(ArchiveHandle *AH, TocEntry *te, Oid oid)
 
 	snprintf(fname, MAXPGPATH, "%s/blob_%u.dat", ctx->directory, oid);
 
-	ctx->dataFH = cfopen_write(fname, PG_BINARY_W, AH->compression_spec);
-
-	if (ctx->dataFH == NULL)
+	ctx->dataFH = InitCompressFileHandle(AH->compression_spec);
+	if (ctx->dataFH->open_write_func(fname, PG_BINARY_W, ctx->dataFH))
 		pg_fatal("could not open output file \"%s\": %m", fname);
 }
 
@@ -682,18 +685,19 @@ static void
 _EndLO(ArchiveHandle *AH, TocEntry *te, Oid oid)
 {
 	lclContext *ctx = (lclContext *) AH->formatData;
+	CompressFileHandle *CFH = ctx->LOsTocFH;
 	char		buf[50];
 	int			len;
 
-	/* Close the LO data file itself */
-	if (cfclose(ctx->dataFH) != 0)
-		pg_fatal("could not close LO data file: %m");
+	/* Close the BLOB data file itself */
+	if (EndCompressFileHandle(ctx->dataFH) != 0)
+		pg_fatal("could not close blob data file: %m");
 	ctx->dataFH = NULL;
 
 	/* register the LO in blobs.toc */
 	len = snprintf(buf, sizeof(buf), "%u blob_%u.dat\n", oid, oid);
-	if (cfwrite(buf, len, ctx->LOsTocFH) != len)
-		pg_fatal("could not write to LOs TOC file");
+	if (CFH->write_func(buf, len, CFH) != len)
+		pg_fatal("could not write to blobs TOC file");
 }
 
 /*
@@ -706,8 +710,8 @@ _EndLOs(ArchiveHandle *AH, TocEntry *te)
 {
 	lclContext *ctx = (lclContext *) AH->formatData;
 
-	if (cfclose(ctx->LOsTocFH) != 0)
-		pg_fatal("could not close LOs TOC file: %m");
+	if (EndCompressFileHandle(ctx->LOsTocFH) != 0)
+		pg_fatal("could not close blobs TOC file: %m");
 	ctx->LOsTocFH = NULL;
 }
 
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index d92247c915..78454928cc 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -94,7 +94,7 @@ my %pgdump_runs = (
 			command => [
 				'pg_restore', '-l', "$tempdir/compression_gzip_custom.dump",
 			],
-			expected => qr/Compression: 1/,
+			expected => qr/Compression: gzip/,
 			name     => 'data content is gzip-compressed'
 		},
 	},
@@ -239,8 +239,8 @@ my %pgdump_runs = (
 			command =>
 			  [ 'pg_restore', '-l', "$tempdir/defaults_custom_format.dump", ],
 			expected => $supports_gzip ?
-			qr/Compression: -1/ :
-			qr/Compression: 0/,
+			qr/Compression: gzip/ :
+			qr/Compression: none/,
 			name => 'data content is gzip-compressed by default if available',
 		},
 	},
@@ -264,8 +264,8 @@ my %pgdump_runs = (
 			command =>
 			  [ 'pg_restore', '-l', "$tempdir/defaults_dir_format", ],
 			expected => $supports_gzip ?
-			qr/Compression: -1/ :
-			qr/Compression: 0/,
+			qr/Compression: gzip/ :
+			qr/Compression: none/,
 			name => 'data content is gzip-compressed by default',
 		},
 		glob_patterns => [
diff --git a/src/tools/pginclude/cpluspluscheck b/src/tools/pginclude/cpluspluscheck
index e52fe9f509..db429474a2 100755
--- a/src/tools/pginclude/cpluspluscheck
+++ b/src/tools/pginclude/cpluspluscheck
@@ -150,7 +150,9 @@ do
 
 	# pg_dump is not C++-clean because it uses "public" and "namespace"
 	# as field names, which is unfortunate but we won't change it now.
+	test "$f" = src/bin/pg_dump/compress_gzip.h && continue
 	test "$f" = src/bin/pg_dump/compress_io.h && continue
+	test "$f" = src/bin/pg_dump/compress_none.h && continue
 	test "$f" = src/bin/pg_dump/parallel.h && continue
 	test "$f" = src/bin/pg_dump/pg_backup_archiver.h && continue
 	test "$f" = src/bin/pg_dump/pg_dump.h && continue
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 51484ca7e2..7e62c6ef3d 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -428,6 +428,7 @@ CompiledExprState
 CompositeIOData
 CompositeTypeStmt
 CompoundAffixFlag
+CompressFileHandle
 CompressionLocation
 CompressorState
 ComputeXidHorizonsResult
@@ -1034,6 +1035,7 @@ GucStack
 GucStackState
 GucStringAssignHook
 GucStringCheckHook
+GzipCompressorState
 HANDLE
 HASHACTION
 HASHBUCKET
-- 
2.34.1

From 978ed90ea8044dda8c04977c10a1b0c8a557ce76 Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Mon, 23 Jan 2023 14:51:06 +0000
Subject: [PATCH v23 3/3] Add LZ4 compression to pg_dump

This is mostly done within pg_dump's compression streaming and file APIs.
It is confined within the newly introduced compress_lz4.{c,h} files.

The first one, is aimed at inlined use cases and thus simple
lz4.h calls can be used directly. The second one is generating output, or is
parsing input, which can be read/generated via the lz4 utility.

Wherever the LZ4F api does not implement all the functionality corresponding
to fread(), fwrite(), fgets(), fgetc(), feof(), and fclose(). Where the
functionality was missing from the official API, it has been implemented
localy.
---
 doc/src/sgml/ref/pg_dump.sgml        |  13 +-
 src/bin/pg_dump/Makefile             |   2 +
 src/bin/pg_dump/compress_io.c        |  20 +-
 src/bin/pg_dump/compress_lz4.c       | 622 +++++++++++++++++++++++++++
 src/bin/pg_dump/compress_lz4.h       |  22 +
 src/bin/pg_dump/meson.build          |   8 +-
 src/bin/pg_dump/pg_backup_archiver.c |  14 +-
 src/bin/pg_dump/pg_dump.c            |   5 +-
 src/bin/pg_dump/t/002_pg_dump.pl     |  82 +++-
 src/tools/pginclude/cpluspluscheck   |   1 +
 src/tools/pgindent/typedefs.list     |   1 +
 11 files changed, 772 insertions(+), 18 deletions(-)
 create mode 100644 src/bin/pg_dump/compress_lz4.c
 create mode 100644 src/bin/pg_dump/compress_lz4.h

diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 2c938cd7e1..49d218905f 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -330,9 +330,10 @@ PostgreSQL documentation
            machine-readable format that <application>pg_restore</application>
            can read. A directory format archive can be manipulated with
            standard Unix tools; for example, files in an uncompressed archive
-           can be compressed with the <application>gzip</application> tool.
-           This format is compressed by default and also supports parallel
-           dumps.
+           can be compressed with the <application>gzip</application> or
+           <application>lz4</application>tool.
+           This format is compressed by default using <literal>gzip</literal>
+           and also supports parallel dumps.
           </para>
          </listitem>
         </varlistentry>
@@ -654,7 +655,7 @@ PostgreSQL documentation
        <para>
         Specify the compression method and/or the compression level to use.
         The compression method can be set to <literal>gzip</literal> or
-        <literal>none</literal> for no compression.
+        <literal>lz4</literal> or <literal>none</literal> for no compression.
         A compression detail string can optionally be specified.  If the
         detail string is an integer, it specifies the compression level.
         Otherwise, it should be a comma-separated list of items, each of the
@@ -675,8 +676,8 @@ PostgreSQL documentation
         individual table-data segments, and the default is to compress using
         <literal>gzip</literal> at a moderate level. For plain text output,
         setting a nonzero compression level causes the entire output file to be compressed,
-        as though it had been fed through <application>gzip</application>; but the default
-        is not to compress.
+        as though it had been fed through <application>gzip</application> or
+        <application>lz4</application>; but the default is not to compress.
        </para>
        <para>
         The tar archive format currently does not support compression at all.
diff --git a/src/bin/pg_dump/Makefile b/src/bin/pg_dump/Makefile
index 0013bc080c..eb8f59459a 100644
--- a/src/bin/pg_dump/Makefile
+++ b/src/bin/pg_dump/Makefile
@@ -17,6 +17,7 @@ top_builddir = ../../..
 include $(top_builddir)/src/Makefile.global
 
 export GZIP_PROGRAM=$(GZIP)
+export LZ4
 export with_icu
 
 override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
@@ -26,6 +27,7 @@ OBJS = \
 	$(WIN32RES) \
 	compress_gzip.o \
 	compress_io.o \
+	compress_lz4.o \
 	compress_none.o \
 	dumputils.o \
 	parallel.o \
diff --git a/src/bin/pg_dump/compress_io.c b/src/bin/pg_dump/compress_io.c
index ea4c266e08..4ec25e352c 100644
--- a/src/bin/pg_dump/compress_io.c
+++ b/src/bin/pg_dump/compress_io.c
@@ -56,7 +56,7 @@
  *	InitDiscoverCompressFileHandle tries to deffer the compression by the
  *	filename suffix. If the suffix is not yet known, then it tries to simply
  *	open the file, and if it fails, it tries to open the same file with the .gz
- *	suffix.
+ *	suffix, and then again with the .lz4 suffix.
  *
  * IDENTIFICATION
  *	   src/bin/pg_dump/compress_io.c
@@ -70,6 +70,7 @@
 
 #include "compress_gzip.h"
 #include "compress_io.h"
+#include "compress_lz4.h"
 #include "compress_none.h"
 #include "pg_backup_utils.h"
 
@@ -95,6 +96,8 @@ AllocateCompressor(const pg_compress_specification compression_spec,
 		InitCompressorNone(cs, compression_spec);
 	else if (compression_spec.algorithm == PG_COMPRESSION_GZIP)
 		InitCompressorGzip(cs, compression_spec);
+	else if (compression_spec.algorithm == PG_COMPRESSION_LZ4)
+		InitCompressorLZ4(cs, compression_spec);
 
 	return cs;
 }
@@ -159,6 +162,8 @@ InitCompressFileHandle(const pg_compress_specification compression_spec)
 		InitCompressNone(CFH, compression_spec);
 	else if (compression_spec.algorithm == PG_COMPRESSION_GZIP)
 		InitCompressGzip(CFH, compression_spec);
+	else if (compression_spec.algorithm == PG_COMPRESSION_LZ4)
+		InitCompressLZ4(CFH, compression_spec);
 
 	return CFH;
 }
@@ -172,7 +177,7 @@ InitCompressFileHandle(const pg_compress_specification compression_spec)
  * throughout. Otherwise the compression will be deferred by iteratively trying
  * to open the file at 'path', first as is, then by appending known compression
  * suffixes. So if you pass "foo" as 'path', this will open either "foo" or
- * "foo.gz", trying in that order.
+ * "foo.gz" or "foo.lz4", trying in that order.
  *
  * On failure, return NULL with an error code in errno.
  */
@@ -210,6 +215,17 @@ InitDiscoverCompressFileHandle(const char *path, const char *mode)
 			if (exists)
 				compression_spec.algorithm = PG_COMPRESSION_GZIP;
 		}
+#endif
+#ifdef USE_LZ4
+		if (!exists)
+		{
+			free_keep_errno(fname);
+			fname = psprintf("%s.lz4", path);
+			exists = (stat(fname, &st) == 0);
+
+			if (exists)
+				compression_spec.algorithm = PG_COMPRESSION_LZ4;
+		}
 #endif
 	}
 
diff --git a/src/bin/pg_dump/compress_lz4.c b/src/bin/pg_dump/compress_lz4.c
new file mode 100644
index 0000000000..e99893d777
--- /dev/null
+++ b/src/bin/pg_dump/compress_lz4.c
@@ -0,0 +1,622 @@
+/*-------------------------------------------------------------------------
+ *
+ * compress_lz4.c
+ *	 Routines for archivers to write a LZ4 compressed data stream.
+ *
+ * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * IDENTIFICATION
+ *	   src/bin/pg_dump/compress_lz4.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "postgres_fe.h"
+#include "pg_backup_utils.h"
+
+#include "compress_lz4.h"
+
+#ifdef USE_LZ4
+#include <lz4.h>
+#include <lz4frame.h>
+
+#define LZ4_OUT_SIZE	(4 * 1024)
+#define LZ4_IN_SIZE		(16 * 1024)
+
+/*
+ * LZ4F_HEADER_SIZE_MAX first appeared in v1.7.5 of the library.
+ * Redefine it for installations with a lesser version.
+ */
+#ifndef LZ4F_HEADER_SIZE_MAX
+#define LZ4F_HEADER_SIZE_MAX	32
+#endif
+
+/*----------------------
+ * Compressor API
+ *----------------------
+ */
+
+typedef struct LZ4CompressorState
+{
+	char	   *outbuf;
+	size_t		outsize;
+} LZ4CompressorState;
+
+/* Private routines that support LZ4 compressed data I/O */
+static void ReadDataFromArchiveLZ4(ArchiveHandle *AH, CompressorState *cs);
+static void WriteDataToArchiveLZ4(ArchiveHandle *AH, CompressorState *cs,
+								  const void *data, size_t dLen);
+static void EndCompressorLZ4(ArchiveHandle *AH, CompressorState *cs);
+
+static void
+ReadDataFromArchiveLZ4(ArchiveHandle *AH, CompressorState *cs)
+{
+	LZ4_streamDecode_t lz4StreamDecode;
+	char	   *buf;
+	char	   *decbuf;
+	size_t		buflen;
+	size_t		cnt;
+
+	buflen = LZ4_IN_SIZE;
+	buf = pg_malloc(buflen);
+	decbuf = pg_malloc(buflen);
+
+	LZ4_setStreamDecode(&lz4StreamDecode, NULL, 0);
+
+	while ((cnt = cs->readF(AH, &buf, &buflen)))
+	{
+		int			decBytes = LZ4_decompress_safe_continue(&lz4StreamDecode,
+															buf, decbuf,
+															cnt, buflen);
+
+		ahwrite(decbuf, 1, decBytes, AH);
+	}
+
+	pg_free(buf);
+	pg_free(decbuf);
+}
+
+static void
+WriteDataToArchiveLZ4(ArchiveHandle *AH, CompressorState *cs,
+					  const void *data, size_t dLen)
+{
+	LZ4CompressorState *LZ4cs = (LZ4CompressorState *) cs->private_data;
+	size_t		compressed;
+	size_t		requiredsize = LZ4_compressBound(dLen);
+
+	if (requiredsize > LZ4cs->outsize)
+	{
+		LZ4cs->outbuf = pg_realloc(LZ4cs->outbuf, requiredsize);
+		LZ4cs->outsize = requiredsize;
+	}
+
+	compressed = LZ4_compress_default(data, LZ4cs->outbuf,
+									  dLen, LZ4cs->outsize);
+
+	if (compressed <= 0)
+		pg_fatal("failed to LZ4 compress data");
+
+	cs->writeF(AH, LZ4cs->outbuf, compressed);
+}
+
+static void
+EndCompressorLZ4(ArchiveHandle *AH, CompressorState *cs)
+{
+	LZ4CompressorState *LZ4cs;
+
+	LZ4cs = (LZ4CompressorState *) cs->private_data;
+	if (LZ4cs)
+	{
+		pg_free(LZ4cs->outbuf);
+		pg_free(LZ4cs);
+		cs->private_data = NULL;
+	}
+}
+
+
+/*
+ * Public routines that support LZ4 compressed data I/O
+ */
+void
+InitCompressorLZ4(CompressorState *cs, const pg_compress_specification compression_spec)
+{
+	cs->readData = ReadDataFromArchiveLZ4;
+	cs->writeData = WriteDataToArchiveLZ4;
+	cs->end = EndCompressorLZ4;
+
+	cs->compression_spec = compression_spec;
+
+	/* Will be lazy init'd */
+	cs->private_data = pg_malloc0(sizeof(LZ4CompressorState));
+}
+
+/*----------------------
+ * Compress File API
+ *----------------------
+ */
+
+/*
+ * State needed for LZ4 (de)compression using the CompressFileHandle API.
+ */
+typedef struct LZ4File
+{
+	FILE	   *fp;
+
+	LZ4F_preferences_t prefs;
+
+	LZ4F_compressionContext_t ctx;
+	LZ4F_decompressionContext_t dtx;
+
+	bool		inited;
+	bool		compressing;
+
+	size_t		buflen;
+	char	   *buffer;
+
+	size_t		overflowalloclen;
+	size_t		overflowlen;
+	char	   *overflowbuf;
+
+	size_t		errcode;
+}	LZ4File;
+
+/*
+ * LZ4 equivalent to feof() or gzeof(). The end of file
+ * is reached if there is no decompressed output in the
+ * overflow buffer and the end of the file is reached.
+ */
+static int
+LZ4File_eof(CompressFileHandle *CFH)
+{
+	LZ4File    *fs = (LZ4File *) CFH->private_data;
+
+	return fs->overflowlen == 0 && feof(fs->fp);
+}
+
+static const char *
+LZ4File_get_error(CompressFileHandle *CFH)
+{
+	LZ4File    *fs = (LZ4File *) CFH->private_data;
+	const char *errmsg;
+
+	if (LZ4F_isError(fs->errcode))
+		errmsg = LZ4F_getErrorName(fs->errcode);
+	else
+		errmsg = strerror(errno);
+
+	return errmsg;
+}
+
+/*
+ * Prepare an already alloc'ed LZ4File struct for subsequent calls.
+ *
+ * It creates the nessary contexts for the operations. When compressing,
+ * it additionally writes the LZ4 header in the output stream.
+ */
+static int
+LZ4File_init(LZ4File * fs, int size, bool compressing)
+{
+	size_t		status;
+
+	if (fs->inited)
+		return 0;
+
+	fs->compressing = compressing;
+	fs->inited = true;
+
+	if (fs->compressing)
+	{
+		fs->buflen = LZ4F_compressBound(LZ4_IN_SIZE, &fs->prefs);
+		if (fs->buflen < LZ4F_HEADER_SIZE_MAX)
+			fs->buflen = LZ4F_HEADER_SIZE_MAX;
+
+		status = LZ4F_createCompressionContext(&fs->ctx, LZ4F_VERSION);
+		if (LZ4F_isError(status))
+		{
+			fs->errcode = status;
+			return 1;
+		}
+
+		fs->buffer = pg_malloc(fs->buflen);
+		status = LZ4F_compressBegin(fs->ctx, fs->buffer, fs->buflen,
+									&fs->prefs);
+
+		if (LZ4F_isError(status))
+		{
+			fs->errcode = status;
+			return 1;
+		}
+
+		if (fwrite(fs->buffer, 1, status, fs->fp) != status)
+		{
+			errno = errno ? : ENOSPC;
+			return 1;
+		}
+	}
+	else
+	{
+		status = LZ4F_createDecompressionContext(&fs->dtx, LZ4F_VERSION);
+		if (LZ4F_isError(status))
+		{
+			fs->errcode = status;
+			return 1;
+		}
+
+		fs->buflen = size > LZ4_OUT_SIZE ? size : LZ4_OUT_SIZE;
+		fs->buffer = pg_malloc(fs->buflen);
+
+		fs->overflowalloclen = fs->buflen;
+		fs->overflowbuf = pg_malloc(fs->overflowalloclen);
+		fs->overflowlen = 0;
+	}
+
+	return 0;
+}
+
+/*
+ * Read already decompressed content from the overflow buffer into 'ptr' up to
+ * 'size' bytes, if available. If the eol_flag is set, then stop at the first
+ * occurance of the new line char prior to 'size' bytes.
+ *
+ * Any unread content in the overflow buffer, is moved to the beginning.
+ */
+static int
+LZ4File_read_overflow(LZ4File * fs, void *ptr, int size, bool eol_flag)
+{
+	char	   *p;
+	int			readlen = 0;
+
+	if (fs->overflowlen == 0)
+		return 0;
+
+	if (fs->overflowlen >= size)
+		readlen = size;
+	else
+		readlen = fs->overflowlen;
+
+	if (eol_flag && (p = memchr(fs->overflowbuf, '\n', readlen)))
+		/* Include the line terminating char */
+		readlen = p - fs->overflowbuf + 1;
+
+	memcpy(ptr, fs->overflowbuf, readlen);
+	fs->overflowlen -= readlen;
+
+	if (fs->overflowlen > 0)
+		memmove(fs->overflowbuf, fs->overflowbuf + readlen, fs->overflowlen);
+
+	return readlen;
+}
+
+/*
+ * The workhorse for reading decompressed content out of an LZ4 compressed
+ * stream.
+ *
+ * It will read up to 'ptrsize' decompressed content, or up to the new line char
+ * if found first when the eol_flag is set. It is possible that the decompressed
+ * output generated by reading any compressed input via the LZ4F API, exceeds
+ * 'ptrsize'. Any exceeding decompressed content is stored at an overflow
+ * buffer within LZ4File. Of course, when the function is called, it will first
+ * try to consume any decompressed content already present in the overflow
+ * buffer, before decompressing new content.
+ */
+static int
+LZ4File_read_internal(LZ4File * fs, void *ptr, int ptrsize, bool eol_flag)
+{
+	size_t		dsize = 0;
+	size_t		rsize;
+	size_t		size = ptrsize;
+	bool		eol_found = false;
+
+	void	   *readbuf;
+
+	/* Lazy init */
+	if (!fs->inited && LZ4File_init(fs, size, false /* decompressing */ ))
+		return -1;
+
+	/* Verfiy that there is enough space in the outbuf */
+	if (size > fs->buflen)
+	{
+		fs->buflen = size;
+		fs->buffer = pg_realloc(fs->buffer, size);
+	}
+
+	/* use already decompressed content if available */
+	dsize = LZ4File_read_overflow(fs, ptr, size, eol_flag);
+	if (dsize == size || (eol_flag && memchr(ptr, '\n', dsize)))
+		return dsize;
+
+	readbuf = pg_malloc(size);
+
+	do
+	{
+		char	   *rp;
+		char	   *rend;
+
+		rsize = fread(readbuf, 1, size, fs->fp);
+		if (rsize < size && !feof(fs->fp))
+			return -1;
+
+		rp = (char *) readbuf;
+		rend = (char *) readbuf + rsize;
+
+		while (rp < rend)
+		{
+			size_t		status;
+			size_t		outlen = fs->buflen;
+			size_t		read_remain = rend - rp;
+
+			memset(fs->buffer, 0, outlen);
+			status = LZ4F_decompress(fs->dtx, fs->buffer, &outlen,
+									 rp, &read_remain, NULL);
+			if (LZ4F_isError(status))
+			{
+				fs->errcode = status;
+				return -1;
+			}
+
+			rp += read_remain;
+
+			/*
+			 * fill in what space is available in ptr if the eol flag is set,
+			 * either skip if one already found or fill up to EOL if present
+			 * in the outbuf
+			 */
+			if (outlen > 0 && dsize < size && eol_found == false)
+			{
+				char	   *p;
+				size_t		lib = (eol_flag == 0) ? size - dsize : size - 1 - dsize;
+				size_t		len = outlen < lib ? outlen : lib;
+
+				if (eol_flag == true &&
+					(p = memchr(fs->buffer, '\n', outlen)) &&
+					(size_t) (p - fs->buffer + 1) <= len)
+				{
+					len = p - fs->buffer + 1;
+					eol_found = true;
+				}
+
+				memcpy((char *) ptr + dsize, fs->buffer, len);
+				dsize += len;
+
+				/* move what did not fit, if any, at the begining of the buf */
+				if (len < outlen)
+					memmove(fs->buffer, fs->buffer + len, outlen - len);
+				outlen -= len;
+			}
+
+			/* if there is available output, save it */
+			if (outlen > 0)
+			{
+				while (fs->overflowlen + outlen > fs->overflowalloclen)
+				{
+					fs->overflowalloclen *= 2;
+					fs->overflowbuf = pg_realloc(fs->overflowbuf,
+												 fs->overflowalloclen);
+				}
+
+				memcpy(fs->overflowbuf + fs->overflowlen, fs->buffer, outlen);
+				fs->overflowlen += outlen;
+			}
+		}
+	} while (rsize == size && dsize < size && eol_found == 0);
+
+	pg_free(readbuf);
+
+	return (int) dsize;
+}
+
+/*
+ * Compress size bytes from ptr and write them to the stream.
+ */
+static size_t
+LZ4File_write(const void *ptr, size_t size, CompressFileHandle *CFH)
+{
+	LZ4File    *fs = (LZ4File *) CFH->private_data;
+	size_t		status;
+	int			remaining = size;
+
+	if (!fs->inited && LZ4File_init(fs, size, true))
+		return -1;
+
+	while (remaining > 0)
+	{
+		int			chunk = remaining < LZ4_IN_SIZE ? remaining : LZ4_IN_SIZE;
+
+		remaining -= chunk;
+
+		status = LZ4F_compressUpdate(fs->ctx, fs->buffer, fs->buflen,
+									 ptr, chunk, NULL);
+		if (LZ4F_isError(status))
+		{
+			fs->errcode = status;
+			return -1;
+		}
+
+		if (fwrite(fs->buffer, 1, status, fs->fp) != status)
+		{
+			errno = errno ? : ENOSPC;
+			return 1;
+		}
+	}
+
+	return size;
+}
+
+/*
+ * fread() equivalent implementation for LZ4 compressed files.
+ */
+static size_t
+LZ4File_read(void *ptr, size_t size, CompressFileHandle *CFH)
+{
+	LZ4File    *fs = (LZ4File *) CFH->private_data;
+	int			ret;
+
+	ret = LZ4File_read_internal(fs, ptr, size, false);
+	if (ret != size && !LZ4File_eof(CFH))
+		pg_fatal("could not read from input file: %s", LZ4File_get_error(CFH));
+
+	return ret;
+}
+
+/*
+ * fgetc() equivalent implementation for LZ4 compressed files.
+ */
+static int
+LZ4File_getc(CompressFileHandle *CFH)
+{
+	LZ4File    *fs = (LZ4File *) CFH->private_data;
+	unsigned char c;
+
+	if (LZ4File_read_internal(fs, &c, 1, false) != 1)
+	{
+		if (!LZ4File_eof(CFH))
+			pg_fatal("could not read from input file: %s", LZ4File_get_error(CFH));
+		else
+			pg_fatal("could not read from input file: end of file");
+	}
+
+	return c;
+}
+
+/*
+ * fgets() equivalent implementation for LZ4 compressed files.
+ */
+static char *
+LZ4File_gets(char *ptr, int size, CompressFileHandle *CFH)
+{
+	LZ4File    *fs = (LZ4File *) CFH->private_data;
+	size_t		dsize;
+
+	dsize = LZ4File_read_internal(fs, ptr, size, true);
+	if (dsize < 0)
+		pg_fatal("could not read from input file: %s", LZ4File_get_error(CFH));
+
+	/* Done reading */
+	if (dsize == 0)
+		return NULL;
+
+	return ptr;
+}
+
+/*
+ * Finalize (de)compression of a stream. When compressing it will write any
+ * remaining content and/or generated footer from the LZ4 API.
+ */
+static int
+LZ4File_close(CompressFileHandle *CFH)
+{
+	FILE	   *fp;
+	LZ4File    *fs = (LZ4File *) CFH->private_data;
+	size_t		status;
+	int			ret;
+
+	fp = fs->fp;
+	if (fs->inited)
+	{
+		if (fs->compressing)
+		{
+			status = LZ4F_compressEnd(fs->ctx, fs->buffer, fs->buflen, NULL);
+			if (LZ4F_isError(status))
+				pg_fatal("failed to end compression: %s",
+						 LZ4F_getErrorName(status));
+			else if ((ret = fwrite(fs->buffer, 1, status, fs->fp)) != status)
+			{
+				errno = errno ? : ENOSPC;
+				WRITE_ERROR_EXIT;
+			}
+
+			status = LZ4F_freeCompressionContext(fs->ctx);
+			if (LZ4F_isError(status))
+				pg_fatal("failed to end compression: %s",
+						 LZ4F_getErrorName(status));
+		}
+		else
+		{
+			status = LZ4F_freeDecompressionContext(fs->dtx);
+			if (LZ4F_isError(status))
+				pg_fatal("failed to end decompression: %s",
+						 LZ4F_getErrorName(status));
+			pg_free(fs->overflowbuf);
+		}
+
+		pg_free(fs->buffer);
+	}
+
+	pg_free(fs);
+
+	return fclose(fp);
+}
+
+static int
+LZ4File_open(const char *path, int fd, const char *mode,
+			 CompressFileHandle *CFH)
+{
+	FILE	   *fp;
+	LZ4File    *lz4fp = (LZ4File *) CFH->private_data;
+
+	if (fd >= 0)
+		fp = fdopen(fd, mode);
+	else
+		fp = fopen(path, mode);
+	if (fp == NULL)
+	{
+		lz4fp->errcode = errno;
+		return 1;
+	}
+
+	lz4fp->fp = fp;
+
+	return 0;
+}
+
+static int
+LZ4File_open_write(const char *path, const char *mode, CompressFileHandle *CFH)
+{
+	char	   *fname;
+	int			ret;
+
+	fname = psprintf("%s.lz4", path);
+	ret = CFH->open_func(fname, -1, mode, CFH);
+	pg_free(fname);
+
+	return ret;
+}
+
+/*
+ * Public routines
+ */
+void
+InitCompressLZ4(CompressFileHandle *CFH, const pg_compress_specification compression_spec)
+{
+	LZ4File    *lz4fp;
+
+	CFH->open_func = LZ4File_open;
+	CFH->open_write_func = LZ4File_open_write;
+	CFH->read_func = LZ4File_read;
+	CFH->write_func = LZ4File_write;
+	CFH->gets_func = LZ4File_gets;
+	CFH->getc_func = LZ4File_getc;
+	CFH->eof_func = LZ4File_eof;
+	CFH->close_func = LZ4File_close;
+	CFH->get_error_func = LZ4File_get_error;
+
+	CFH->compression_spec = compression_spec;
+	lz4fp = pg_malloc0(sizeof(*lz4fp));
+	if (CFH->compression_spec.level >= 0)
+		lz4fp->prefs.compressionLevel = CFH->compression_spec.level;
+
+	CFH->private_data = lz4fp;
+}
+#else							/* USE_LZ4 */
+void
+InitCompressorLZ4(CompressorState *cs, const pg_compress_specification compression_spec)
+{
+	pg_fatal("this build does not support compression with %s", "LZ4");
+}
+
+void
+InitCompressLZ4(CompressFileHandle *CFH, const pg_compress_specification compression_spec)
+{
+	pg_fatal("this build does not support compression with %s", "LZ4");
+}
+#endif							/* USE_LZ4 */
diff --git a/src/bin/pg_dump/compress_lz4.h b/src/bin/pg_dump/compress_lz4.h
new file mode 100644
index 0000000000..0fe8c4e524
--- /dev/null
+++ b/src/bin/pg_dump/compress_lz4.h
@@ -0,0 +1,22 @@
+/*-------------------------------------------------------------------------
+ *
+ * compress_lz4.h
+ *	 LZ4 interface to compress_io.c routines
+ *
+ * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * IDENTIFICATION
+ *	   src/bin/pg_dump/compress_lz4.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef _COMPRESS_LZ4_H_
+#define _COMPRESS_LZ4_H_
+
+#include "compress_io.h"
+
+extern void InitCompressorLZ4(CompressorState *cs, const pg_compress_specification compression_spec);
+extern void InitCompressLZ4(CompressFileHandle *CFH, const pg_compress_specification compression_spec);
+
+#endif							/* _COMPRESS_LZ4_H_ */
diff --git a/src/bin/pg_dump/meson.build b/src/bin/pg_dump/meson.build
index 84e9f0defa..0da476a4c3 100644
--- a/src/bin/pg_dump/meson.build
+++ b/src/bin/pg_dump/meson.build
@@ -3,6 +3,7 @@
 pg_dump_common_sources = files(
   'compress_gzip.c',
   'compress_io.c',
+  'compress_lz4.c',
   'compress_none.c',
   'dumputils.c',
   'parallel.c',
@@ -18,7 +19,7 @@ pg_dump_common_sources = files(
 pg_dump_common = static_library('libpgdump_common',
   pg_dump_common_sources,
   c_pch: pch_postgres_fe_h,
-  dependencies: [frontend_code, libpq, zlib],
+  dependencies: [frontend_code, libpq, lz4, zlib],
   kwargs: internal_lib_args,
 )
 
@@ -86,7 +87,10 @@ tests += {
   'sd': meson.current_source_dir(),
   'bd': meson.current_build_dir(),
   'tap': {
-    'env': {'GZIP_PROGRAM': gzip.path()},
+    'env': {
+      'GZIP_PROGRAM': gzip.path(),
+      'LZ4': program_lz4.found() ? program_lz4.path() : '',
+    },
     'tests': [
       't/001_basic.pl',
       't/002_pg_dump.pl',
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index febdd14885..4c5c4ed9be 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -398,6 +398,10 @@ RestoreArchive(Archive *AHX)
 #ifndef HAVE_LIBZ
 				if (AH->compression_spec.algorithm == PG_COMPRESSION_GZIP)
 					supports_compression = false;
+#endif
+#ifndef USE_LZ4
+				if (AH->compression_spec.algorithm == PG_COMPRESSION_LZ4)
+					supports_compression = false;
 #endif
 				if (supports_compression == false)
 					pg_fatal("cannot restore from compressed archive (compression not supported in this installation)");
@@ -2082,7 +2086,7 @@ _discoverArchiveFormat(ArchiveHandle *AH)
 
 		/*
 		 * Check if the specified archive is a directory. If so, check if
-		 * there's a "toc.dat" (or "toc.dat.gz") file in it.
+		 * there's a "toc.dat" (or "toc.dat.{gz,lz4}") file in it.
 		 */
 		if (stat(AH->fSpec, &st) == 0 && S_ISDIR(st.st_mode))
 		{
@@ -2092,6 +2096,10 @@ _discoverArchiveFormat(ArchiveHandle *AH)
 #ifdef HAVE_LIBZ
 			if (_fileExistsInDirectory(AH->fSpec, "toc.dat.gz"))
 				return AH->format;
+#endif
+#ifdef USE_LZ4
+			if (_fileExistsInDirectory(AH->fSpec, "toc.dat.lz4"))
+				return AH->format;
 #endif
 			pg_fatal("directory \"%s\" does not appear to be a valid archive (\"toc.dat\" does not exist)",
 					 AH->fSpec);
@@ -3755,6 +3763,10 @@ ReadHead(ArchiveHandle *AH)
 #ifndef HAVE_LIBZ
 		if (AH->compression_spec.algorithm == PG_COMPRESSION_GZIP)
 			unsupported = true;
+#endif
+#ifndef USE_LZ4
+		if (AH->compression_spec.algorithm == PG_COMPRESSION_LZ4)
+			unsupported = true;
 #endif
 		if (unsupported)
 			pg_fatal("archive is compressed, but this installation does not support compression");
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 527c7651ab..08105337b1 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -715,13 +715,12 @@ main(int argc, char **argv)
 		case PG_COMPRESSION_NONE:
 			/* fallthrough */
 		case PG_COMPRESSION_GZIP:
+			/* fallthrough */
+		case PG_COMPRESSION_LZ4:
 			break;
 		case PG_COMPRESSION_ZSTD:
 			pg_fatal("compression with %s is not yet supported", "ZSTD");
 			break;
-		case PG_COMPRESSION_LZ4:
-			pg_fatal("compression with %s is not yet supported", "LZ4");
-			break;
 	}
 
 	/*
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 78454928cc..1777a4e5ac 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -139,6 +139,80 @@ my %pgdump_runs = (
 			args    => [ '-d', "$tempdir/compression_gzip_plain.sql.gz", ],
 		},
 	},
+
+	# Do not use --no-sync to give test coverage for data sync.
+	compression_lz4_custom => {
+		test_key       => 'compression',
+		compile_option => 'lz4',
+		dump_cmd       => [
+			'pg_dump',      '--format=custom',
+			'--compress=lz4', "--file=$tempdir/compression_lz4_custom.dump",
+			'postgres',
+		],
+		restore_cmd => [
+			'pg_restore',
+			"--file=$tempdir/compression_lz4_custom.sql",
+			"$tempdir/compression_lz4_custom.dump",
+		],
+		command_like => {
+			command => [
+				'pg_restore',
+				'-l', "$tempdir/compression_lz4_custom.dump",
+			],
+			expected => qr/Compression: lz4/,
+			name => 'data content is lz4 compressed'
+		},
+	},
+
+	# Do not use --no-sync to give test coverage for data sync.
+	compression_lz4_dir => {
+		test_key       => 'compression',
+		compile_option => 'lz4',
+		dump_cmd       => [
+			'pg_dump',                              '--jobs=2',
+			'--format=directory',                   '--compress=lz4:1',
+			"--file=$tempdir/compression_lz4_dir", 'postgres',
+		],
+		# Give coverage for manually compressed blob.toc files during
+		# restore.
+		compress_cmd => {
+			program => $ENV{'LZ4'},
+			args    => [
+				'-z', '-f', '--rm',
+				"$tempdir/compression_lz4_dir/blobs.toc",
+				"$tempdir/compression_lz4_dir/blobs.toc.lz4",
+			],
+		},
+		# Verify that data files where compressed
+		glob_patterns => [
+			"$tempdir/compression_lz4_dir/toc.dat",
+		    "$tempdir/compression_lz4_dir/*.dat.lz4",
+		],
+		restore_cmd => [
+			'pg_restore', '--jobs=2',
+			"--file=$tempdir/compression_lz4_dir.sql",
+			"$tempdir/compression_lz4_dir",
+		],
+	},
+
+	compression_lz4_plain => {
+		test_key       => 'compression',
+		compile_option => 'lz4',
+		dump_cmd       => [
+			'pg_dump', '--format=plain', '--compress=lz4',
+			"--file=$tempdir/compression_lz4_plain.sql.lz4", 'postgres',
+		],
+		# Decompress the generated file to run through the tests.
+		compress_cmd => {
+			program => $ENV{'LZ4'},
+			args    => [
+				'-d', '-f',
+				"$tempdir/compression_lz4_plain.sql.lz4",
+				"$tempdir/compression_lz4_plain.sql",
+			],
+		},
+	},
+
 	clean => {
 		dump_cmd => [
 			'pg_dump',
@@ -4175,11 +4249,11 @@ foreach my $run (sort keys %pgdump_runs)
 	my $run_db   = 'postgres';
 
 	# Skip command-level tests for gzip if there is no support for it.
-	if (   defined($pgdump_runs{$run}->{compile_option})
-		&& $pgdump_runs{$run}->{compile_option} eq 'gzip'
-		&& !$supports_gzip)
+	if ($pgdump_runs{$run}->{compile_option} &&
+		($pgdump_runs{$run}->{compile_option} eq 'gzip' && !$supports_gzip) ||
+		($pgdump_runs{$run}->{compile_option} eq 'lz4' && !$supports_lz4))
 	{
-		note "$run: skipped due to no gzip support";
+		note "$run: skipped due to no $pgdump_runs{$run}->{compile_option} support";
 		next;
 	}
 
diff --git a/src/tools/pginclude/cpluspluscheck b/src/tools/pginclude/cpluspluscheck
index db429474a2..2c5042eb41 100755
--- a/src/tools/pginclude/cpluspluscheck
+++ b/src/tools/pginclude/cpluspluscheck
@@ -152,6 +152,7 @@ do
 	# as field names, which is unfortunate but we won't change it now.
 	test "$f" = src/bin/pg_dump/compress_gzip.h && continue
 	test "$f" = src/bin/pg_dump/compress_io.h && continue
+	test "$f" = src/bin/pg_dump/compress_lz4.h && continue
 	test "$f" = src/bin/pg_dump/compress_none.h && continue
 	test "$f" = src/bin/pg_dump/parallel.h && continue
 	test "$f" = src/bin/pg_dump/pg_backup_archiver.h && continue
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 7e62c6ef3d..692c5ceb0b 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -1383,6 +1383,7 @@ LWLock
 LWLockHandle
 LWLockMode
 LWLockPadded
+LZ4CompressorState
 LZ4F_compressionContext_t
 LZ4F_decompressOptions_t
 LZ4F_decompressionContext_t
-- 
2.34.1

---

From: Justin Pryzby <[email-masked]>
Date: Mon, 23 Jan 2023 12:00:14 -0600

> implementation in separate files.

Thanks for updating the patch.  Could you address the review comments I
sent here ?
https://www.postgresql.org/message-id/20230108194524.GA27637%40telsasoft.com

Thanks,
-- 
Justin

---

From: [email-masked]
Date: Tue, 24 Jan 2023 15:56:20 +0000

------- Original Message -------

> https://www.postgresql.org/message-id/20230108194524.GA27637%40telsasoft.com

Please find v24 attached.

Cheers,
//Georgios

> Thanks,

From af03b49cd4deedfcd05c468a74e40732650ef681 Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Tue, 24 Jan 2023 15:37:59 +0000
Subject: [PATCH v24 1/3] Prepare pg_dump internals for additional compression
 methods.

Commit  bf9aa490db introduced cfp in compress_io.{c,h} with the intent of
unifying compression related code and allow for the introduction of additional
archive formats. However, pg_backup_archiver.c was not using that API. This
commit teaches pg_backup_archiver.c about it and is using it throughout.
---
 src/bin/pg_dump/compress_io.c        |  66 +++++++++++---
 src/bin/pg_dump/compress_io.h        |   2 +
 src/bin/pg_dump/pg_backup_archiver.c | 132 ++++++++++-----------------
 src/bin/pg_dump/pg_backup_archiver.h |  27 +-----
 4 files changed, 103 insertions(+), 124 deletions(-)

+#ifdef HAVE_LIBZ
+#include <zlib.h>
+#endif
+
 /*----------------------
  * Compressor API
  *----------------------
 
 /*

 
 
+/*
 
 int
 
index ba5e6acbbb..5f52ea40e8 100644

 
 
 	/* Close the output */
 
 	if (res != 0)

@@ -391,17 +383,26 @@ RestoreArchive(Archive *AHX)
 	/*
 	 * Make sure we won't need (de)compression we haven't got
 	 */
-#ifndef HAVE_LIBZ
-	if (AH->compression_spec.algorithm == PG_COMPRESSION_GZIP &&
-		AH->PrintTocDataPtr != NULL)
+	supports_compression = false;
+	if (AH->compression_spec.algorithm == PG_COMPRESSION_NONE ||
+		AH->compression_spec.algorithm == PG_COMPRESSION_GZIP)
+		supports_compression = true;
+
+	if (AH->PrintTocDataPtr != NULL)
 	{
 		for (te = AH->toc->next; te != AH->toc; te = te->next)
 		{
 			if (te->hadDumper && (te->reqs & REQ_DATA) != 0)
-				pg_fatal("cannot restore from compressed archive (compression not supported in this installation)");
+			{
+#ifndef HAVE_LIBZ
+				if (AH->compression_spec.algorithm == PG_COMPRESSION_GZIP)
+					supports_compression = false;
+#endif
+				if (supports_compression == false)
+					pg_fatal("cannot restore from compressed archive (compression not supported in this installation)");
+			}
 		}
 	}
-#endif
 
 	/*
 	 * Prepare index arrays, so we can assume we have them throughout restore.
@@ -1133,7 +1134,7 @@ PrintTOCSummary(Archive *AHX)
 	TocEntry   *te;
 	pg_compress_specification out_compression_spec = {0};
 	teSection	curSection;
-	OutputContext sav;
+	cfp		   *sav;
 	const char *fmtName;
 	char		stamp_str[64];
 
@@ -1508,58 +1509,32 @@ static void
 SetOutput(ArchiveHandle *AH, const char *filename,
 		  const pg_compress_specification compression_spec)
 {
-	int			fn;
+	const char *mode;
+	int			fn = -1;
 
 	if (filename)
 	{

 
 	if (!AH->OF)
 	{
@@ -1570,33 +1545,24 @@ SetOutput(ArchiveHandle *AH, const char *filename,
 	}
 }
 
-static OutputContext
+static cfp *
 
 static void

 	if (res != 0)

@@ -1720,22 +1686,17 @@ ahwrite(const void *ptr, size_t size, size_t nmemb, ArchiveHandle *AH)

+	/*
 
 	if (bytes_written != size * nmemb)
 		WRITE_ERROR_EXIT;
@@ -2224,6 +2185,7 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,
 		 SetupWorkerPtrType setupWorkerPtr)
 {
 	ArchiveHandle *AH;
+	pg_compress_specification out_compress_spec = {0};
 
 	pg_log_debug("allocating AH for %s, format %d",
 				 FileSpec ? FileSpec : "(stdio)", fmt);
@@ -2277,8 +2239,8 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,
 	memset(&(AH->sqlparse), 0, sizeof(AH->sqlparse));

 	/*
 
 #define LOBBUFSIZE 16384
 
-#ifdef HAVE_LIBZ
-#include <zlib.h>

From 4900a66813144ca42ba343370c52259ca8afd491 Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Tue, 24 Jan 2023 15:40:12 +0000
Subject: [PATCH v24 2/3] Introduce Compress and Compressor API in pg_dump

 src/bin/pg_dump/compress_gzip.c       | 397 ++++++++++++++
 src/include/common/compression.h      |   4 +
 16 files changed, 1057 insertions(+), 752 deletions(-)

 OBJS = \
 	$(WIN32RES) \
+	compress_gzip.o \
 	compress_io.o \
+	compress_none.o \
 	dumputils.o \
 	parallel.o \
 	pg_backup_archiver.o \
index 0000000000..59da4a955e
@@ -0,0 +1,397 @@
+	return gzclose(gzfp);
 
+#include <sys/stat.h>
+#include <unistd.h>
+
 
-#ifdef HAVE_LIBZ
-#include <zlib.h>
-#endif
-
 /*----------------------
  * Compressor API
  *----------------------
  */

-#ifndef HAVE_LIBZ
 
-/*
 
-/*
 
 /*

 /*
 
-#ifdef HAVE_LIBZ

 /*
 
 /*

 
-	return fp;
+	return CFH;
 }
 
 /*
 
-int
 
-	if (size == 0)
-		return 0;
 
-#ifdef HAVE_LIBZ
-	if (fp->compressedfp)
-	{

 
-	return ret;
-}
-
-char *
 
-	return result;
+	return CFH;
 }
 
+/*
 
-const char *
-get_cfp_error(cfp *fp)
-{
-#ifdef HAVE_LIBZ
-	if (fp->compressedfp)
-	{
-		int			errnum;
 
-		if (errnum != Z_ERRNO)
-			return errmsg;
-	}
-#endif
 
-#ifdef HAVE_LIBZ
-static int

 /*

 
+/*
+ * Compress File Handle
+ */
 
-typedef struct cfp cfp;
+	/*

+	/*
 
index 5f52ea40e8..04cf887526 100644

 	/* Close the output */
 	errno = 0;
-	res = cfclose(AH->OF);
 
 	if (res != 0)

@@ -1134,7 +1134,7 @@ PrintTOCSummary(Archive *AHX)
 	TocEntry   *te;
 	pg_compress_specification out_compression_spec = {0};
 	teSection	curSection;
-	cfp		   *sav;
+	CompressFileHandle *sav;
 	const char *fmtName;
 	char		stamp_str[64];
 
@@ -1150,9 +1150,10 @@ PrintTOCSummary(Archive *AHX)
 		strcpy(stamp_str, "[unknown]");

 	switch (AH->format)
 	{
@@ -1509,6 +1510,7 @@ static void
 SetOutput(ArchiveHandle *AH, const char *filename,
 		  const pg_compress_specification compression_spec)
 {
+	CompressFileHandle *CFH;
 	const char *mode;
 	int			fn = -1;
 
@@ -1531,33 +1533,32 @@ SetOutput(ArchiveHandle *AH, const char *filename,
 	else
 		mode = PG_BINARY_W;
 
-	if (fn >= 0)
 
-	if (!AH->OF)
 
-static cfp *
 
 static void
 
 	errno = 0;
-	res = cfclose(AH->OF);
 
 	if (res != 0)
@@ -1696,7 +1697,11 @@ ahwrite(const void *ptr, size_t size, size_t nmemb, ArchiveHandle *AH)
 
 	if (bytes_written != size * nmemb)
 		WRITE_ERROR_EXIT;
@@ -2038,6 +2043,18 @@ ReadStr(ArchiveHandle *AH)
 	return buf;
 }
 
+static bool
@@ -2068,26 +2085,12 @@ _discoverArchiveFormat(ArchiveHandle *AH)
@@ -2185,6 +2188,7 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,
 
 	pg_log_debug("allocating AH for %s, format %d",
@@ -2240,7 +2244,10 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,

 	/*
@@ -3653,12 +3660,7 @@ WriteHead(ArchiveHandle *AH)
@@ -3729,10 +3731,11 @@ ReadHead(ArchiveHandle *AH)
 
@@ -3744,10 +3747,17 @@ ReadHead(ArchiveHandle *AH)

 

 
 /*
 
 	if (dLen > 0)
 
 /*

 	WriteInt(AH, oid);

 /*
 
 static void

 

 

 
 
 	errno = 0;

 	/* Close the file */
 
 	ctx->dataFH = NULL;
 
 	if (!filename)
 		return;

 
 
 	free(buf);
-	if (cfclose(cfp) != 0)

 

 

 
 	errno = 0;
 
 	return 1;

 /*
 
 	errno = 0;

 	/*

 

 

 

 

 /*

diff --git a/src/include/common/compression.h b/src/include/common/compression.h
index 9a471e40aa..d44ebb06cc 100644
--- a/src/include/common/compression.h
+++ b/src/include/common/compression.h
@@ -14,6 +14,10 @@
 #ifndef PG_COMPRESSION_H
 #define PG_COMPRESSION_H
 
+/*
+ * These values are stored in disc, for example in files generated by pg_dump.
+ * Create the necessary backwards compatibility layers if their order changes.
+ */
 typedef enum pg_compress_algorithm
 {
 	PG_COMPRESSION_NONE,
diff --git a/src/tools/pginclude/cpluspluscheck b/src/tools/pginclude/cpluspluscheck
index e52fe9f509..db429474a2 100755
--- a/src/tools/pginclude/cpluspluscheck
+++ b/src/tools/pginclude/cpluspluscheck
@@ -150,7 +150,9 @@ do

From c1d868f9ac967b800c20bb4c588a41883356a652 Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Tue, 24 Jan 2023 15:42:43 +0000
Subject: [PATCH v24 3/3] Add LZ4 compression to pg_dump

Wherever the LZ4F api does not implement all the functionality corresponding
to fread(), fwrite(), fgets(), fgetc(), feof(), and fclose(). Where the
functionality was missing from the official API, it has been implemented
localy.
---
 doc/src/sgml/ref/pg_dump.sgml         |  13 +-
 src/bin/pg_dump/Makefile              |   2 +
 src/bin/pg_dump/compress_io.c         |  20 +-
 src/bin/pg_dump/compress_lz4.c        | 623 ++++++++++++++++++++++++++
 src/bin/pg_dump/compress_lz4.h        |  22 +
 src/bin/pg_dump/meson.build           |   8 +-
 src/bin/pg_dump/pg_backup_archiver.c  |  17 +-
 src/bin/pg_dump/pg_backup_directory.c |   9 +-
 src/bin/pg_dump/pg_dump.c             |   5 +-
 src/bin/pg_dump/t/002_pg_dump.pl      |  82 +++-
 src/tools/pginclude/cpluspluscheck    |   1 +
 src/tools/pgindent/typedefs.list      |   1 +
 12 files changed, 781 insertions(+), 22 deletions(-)
 create mode 100644 src/bin/pg_dump/compress_lz4.c
 create mode 100644 src/bin/pg_dump/compress_lz4.h

 

 
 	return cs;
 }
 
 	return CFH;
 }
 
index 0000000000..644107f124
@@ -0,0 +1,623 @@
+ * It creates the necessary contexts for the operations. When compressing,
+			errno = (errno) ? errno : ENOSPC;
+ * occurrence of the new line char prior to 'size' bytes.
+	if (LZ4File_init(fs, size, false /* decompressing */ ))
+	/* Verify that there is enough space in the outbuf */
+				size_t		lib = (!eol_flag) ? size - dsize : size - 1 - dsize;
+				if (eol_flag &&
+				/* move what did not fit, if any, at the beginning of the buf */
+	if (LZ4File_init(fs, size, true))
+			errno = (errno) ? errno : ENOSPC;
+				errno = (errno) ? errno : ENOSPC;
 
@@ -86,7 +87,10 @@ tests += {
   'sd': meson.current_source_dir(),
   'bd': meson.current_build_dir(),
   'tap': {
-    'env': {'GZIP_PROGRAM': gzip.path()},
+    'env': {
+      'GZIP_PROGRAM': gzip.path(),
+      'LZ4': program_lz4.found() ? program_lz4.path() : '',
+    },
     'tests': [
       't/001_basic.pl',
       't/002_pg_dump.pl',
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 04cf887526..52c9a4a634 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -385,7 +385,8 @@ RestoreArchive(Archive *AHX)
 	 */
 	supports_compression = false;
 	if (AH->compression_spec.algorithm == PG_COMPRESSION_NONE ||
-		AH->compression_spec.algorithm == PG_COMPRESSION_GZIP)
+		AH->compression_spec.algorithm == PG_COMPRESSION_GZIP ||
+		AH->compression_spec.algorithm == PG_COMPRESSION_LZ4)
 		supports_compression = true;
 
 	if (AH->PrintTocDataPtr != NULL)
@@ -397,6 +398,10 @@ RestoreArchive(Archive *AHX)
 #ifndef HAVE_LIBZ
 				if (AH->compression_spec.algorithm == PG_COMPRESSION_GZIP)
 					supports_compression = false;
+#endif
+#ifndef USE_LZ4
+				if (AH->compression_spec.algorithm == PG_COMPRESSION_LZ4)
+					supports_compression = false;
 #endif
 				if (supports_compression == false)
 					pg_fatal("cannot restore from compressed archive (compression not supported in this installation)");
@@ -2081,7 +2086,7 @@ _discoverArchiveFormat(ArchiveHandle *AH)
 
 		/*
 		 * Check if the specified archive is a directory. If so, check if
-		 * there's a "toc.dat" (or "toc.dat.gz") file in it.
+		 * there's a "toc.dat" (or "toc.dat.{gz,lz4}") file in it.
 		 */
 		if (stat(AH->fSpec, &st) == 0 && S_ISDIR(st.st_mode))
 		{
@@ -2091,6 +2096,10 @@ _discoverArchiveFormat(ArchiveHandle *AH)
 #ifdef HAVE_LIBZ
 			if (_fileExistsInDirectory(AH->fSpec, "toc.dat.gz"))
 				return AH->format;
+#endif
+#ifdef USE_LZ4
+			if (_fileExistsInDirectory(AH->fSpec, "toc.dat.lz4"))
+				return AH->format;
 #endif
 			pg_fatal("directory \"%s\" does not appear to be a valid archive (\"toc.dat\" does not exist)",
 					 AH->fSpec);
@@ -3754,6 +3763,10 @@ ReadHead(ArchiveHandle *AH)
 #ifndef HAVE_LIBZ
 		if (AH->compression_spec.algorithm == PG_COMPRESSION_GZIP)
 			unsupported = true;
+#endif
+#ifndef USE_LZ4
+		if (AH->compression_spec.algorithm == PG_COMPRESSION_LZ4)
+			unsupported = true;
 #endif
 		if (unsupported)
 			pg_fatal("archive is compressed, but this installation does not support compression");
diff --git a/src/bin/pg_dump/pg_backup_directory.c b/src/bin/pg_dump/pg_backup_directory.c
index a2f88995c0..ce2a0838fa 100644
--- a/src/bin/pg_dump/pg_backup_directory.c
+++ b/src/bin/pg_dump/pg_backup_directory.c
@@ -779,10 +779,13 @@ _PrepParallelRestore(ArchiveHandle *AH)
 
 		if (stat(fname, &st) == 0)
 			te->dataLength = st.st_size;
-		else
+		else if (AH->compression_spec.algorithm != PG_COMPRESSION_NONE)
 		{
-			/* It might be compressed */
-			strlcat(fname, ".gz", sizeof(fname));
+			if (AH->compression_spec.algorithm == PG_COMPRESSION_GZIP)
+				strlcat(fname, ".gz", sizeof(fname));
+			else if (AH->compression_spec.algorithm == PG_COMPRESSION_LZ4)
+				strlcat(fname, ".lz4", sizeof(fname));
+
 			if (stat(fname, &st) == 0)
 				te->dataLength = st.st_size;
 		}
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 527c7651ab..08105337b1 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -715,13 +715,12 @@ main(int argc, char **argv)
 		case PG_COMPRESSION_NONE:
 			/* fallthrough */
 		case PG_COMPRESSION_GZIP:
+			/* fallthrough */
+		case PG_COMPRESSION_LZ4:
 			break;
 		case PG_COMPRESSION_ZSTD:
 			pg_fatal("compression with %s is not yet supported", "ZSTD");
 			break;
-		case PG_COMPRESSION_LZ4:
-			pg_fatal("compression with %s is not yet supported", "LZ4");
-			break;
 	}
 
 	/*
index 78454928cc..72b19ee6cd 100644
+		# Verify that data files were compressed

---

From: Justin Pryzby <[email-masked]>
Date: Tue, 24 Jan 2023 19:42:03 -0600

> Please find v24 attached.

Thanks for updating the patch.

> +                                       pg_fatal("cannot restore from compressed archive (compression not supported in this installation)");

This first checks if the algorithm is implemented, and then checks if
the algorithm is supported by the current build - that confused me for a
bit.  It seems unnecessary to check for unimplemented algorithms before
looping.  That also requires referencing both GZIP and LZ4 in two
places.

I think it could be written to avoid the need to change for added
compression algorithms:

+                       if (te->hadDumper && (te->reqs & REQ_DATA) != 0)
+                       {
+                               /* Check if the compression algorithm is supported */
+                               pg_compress_specification spec;
+                               parse_compress_specification(AH->compression_spec.algorithm, NULL, &spec);
+                               if (spec->parse_error != NULL)
+                                       pg_fatal(spec->parse_error);
+                       }

Or maybe add a new function to compression.c to indicate whether a given
algorithm is supported.

That would also indicate *which* compression library isn't supported.

Other than that, I think 001 is ready.

002/003 use these names, which I think are too similar - initially I
didn't even realize there were two separate functions (each with a
second stub function to handle the case of unsupported compression):

+extern void InitCompressorGzip(CompressorState *cs, const pg_compress_specification compression_spec);                                                                                                                      
+extern void InitCompressGzip(CompressFileHandle *CFH, const pg_compress_specification compression_spec);                                                                                                                        

typo:
s/not build with/not built with/

Should AllocateCompressor() set cs->compression_spec, rather than doing
it in each compressor ?

Thanks for considering.

-- 
Justin

---

From: [email-masked]
Date: Wed, 25 Jan 2023 15:37:12 +0000

------- Original Message -------
On Wednesday, January 25th, 2023 at 2:42 AM, Justin Pryzby <[email-masked]> wrote:

> > On Monday, January 23rd, 2023 at 7:00 PM, Justin Pryzby [email-masked] wrote:
> > 

I am not certain that it is unnecessary, at least not in the way that is
described. The idea is that new compression methods can be added, without
changing the archive's version number. It is very possible that it is
requested to restore an archive compressed with a method not implemented
in the current binary. The first check takes care of that and sets
supports_compression only for the supported versions. It is possible to
enter the loop with supports_compression already set to false, for example
because the archive was compressed with ZSTD, triggering the fatal error.

Of course, one can throw the error before entering the loop, yet I think
that it does not help the readability of the code. IMHO it is easier to
follow if the error is thrown once during that check.

> + pg_fatal(spec->parse_error);

I am not certain how that would work in the example with ZSTD above.
If I am not wrong, parse_compress_specification() will not throw an error
if the codebase supports ZSTD, yet this specific pg_dump binary will not
support it because ZSTD is not implemented. parse_compress_specification()
is not aware of that and should not be aware of it, should it?

> Or maybe add a new function to compression.c to indicate whether a given

I am not certain how this would help, as compression.c is supposed to be
used by multiple binaries while this is a pg_dump specific detail.

> That would also indicate which compression library isn't supported.

If anything, I can suggest to throw an error much earlier, i.e. in ReadHead(),
and remove altogether this check. On the other hand, I like the belts
and suspenders approach because there are no more checks after this point.

> Other than that, I think 001 is ready.

Thank you.

> +extern void InitCompressLZ4(CompressFileHandle *CFH, const pg_compress_specification compression_spec);

Fair enough. Names are now updated.

> s/not build with/not built with/

Thank you.

> Should AllocateCompressor() set cs->compression_spec, rather than doing

I think that compression_spec should be owned by each compressor. With that
in mind, it makes more sense to set it within each compressor. This is not
a hill I am willing to die on though.

Please find v25 attached.

> Thanks for considering.

From 5cf128bc49d47a1e5a763dd77a2245cb7e5680e9 Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Wed, 25 Jan 2023 14:43:23 +0000
Subject: [PATCH v25 1/3] Prepare pg_dump internals for additional compression
 methods.

 
+#ifdef HAVE_LIBZ
+#include <zlib.h>
+#endif
+
 /*----------------------
  * Compressor API
  *----------------------
 
 /*

 
 
+/*
 
 int

 

 	/* Close the output */
 
 	if (res != 0)

 
 	/*

 	if (filename)
 	{

 
 	if (!AH->OF)
 	{
 
-static OutputContext
+static cfp *
 
 static void

 	if (res != 0)

 
 
+	/*

 
 
 	/*
 
 #define LOBBUFSIZE 16384
 
-#ifdef HAVE_LIBZ
-#include <zlib.h>

From 8aa452d327c184cf54797430e4d912ac956ec187 Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Wed, 25 Jan 2023 14:45:50 +0000
Subject: [PATCH v25 2/3] Introduce Compress and Compressor API in pg_dump

 src/bin/pg_dump/compress_gzip.c       | 401 ++++++++++++++
 16 files changed, 1061 insertions(+), 752 deletions(-)

 OBJS = \
 	$(WIN32RES) \
+	compress_gzip.o \
 	compress_io.o \
+	compress_none.o \
 	dumputils.o \
 	parallel.o \
 	pg_backup_archiver.o \
index 0000000000..24e68fd022
@@ -0,0 +1,401 @@
+InitCompressorGzip(CompressorState *cs,
+InitCompressFileHandleGzip(CompressFileHandle *CFH,
+InitCompressorGzip(CompressorState *cs,
+InitCompressFileHandleGzip(CompressFileHandle *CFH,
index 0000000000..2392c697b4
+extern void InitCompressFileHandleGzip(CompressFileHandle *CFH,
index ef033914ba..f47ba5e615 100644
 
+#include <sys/stat.h>
+#include <unistd.h>
+
 
-#ifdef HAVE_LIBZ
-#include <zlib.h>
-#endif
-
 /*----------------------
  * Compressor API
  *----------------------
  */

-#ifndef HAVE_LIBZ
 
-/*
 
-/*
 
 /*

 /*
 
-#ifdef HAVE_LIBZ

 /*
 
 /*

-		if (fp->uncompressedfp == NULL)
-		{
-			free_keep_errno(fp);
-			fp = NULL;
-		}
-	}
+	if (compression_spec.algorithm == PG_COMPRESSION_NONE)
+		InitCompressFileHandleNone(CFH, compression_spec);
+	else if (compression_spec.algorithm == PG_COMPRESSION_GZIP)
+		InitCompressFileHandleGzip(CFH, compression_spec);
 
-	return fp;
+	return CFH;
 }
 
 /*
 
-int
 
-	if (size == 0)
-		return 0;
 
-#ifdef HAVE_LIBZ
-	if (fp->compressedfp)
-	{

+		/* avoid unused warning if it is not built with compression */
 
-	return ret;
-}
-
-char *
 
-	return result;
+	return CFH;
 }
 
+/*
 
-const char *
-get_cfp_error(cfp *fp)
-{
-#ifdef HAVE_LIBZ
-	if (fp->compressedfp)
-	{
-		int			errnum;
 
-		if (errnum != Z_ERRNO)
-			return errmsg;
-	}
-#endif
 
-#ifdef HAVE_LIBZ
-static int

 /*

 
+/*
+ * Compress File Handle
+ */
 
-typedef struct cfp cfp;
+	/*

+	/*
index 0000000000..ecbcf4b04a
+InitCompressFileHandleNone(CompressFileHandle *CFH,
index 0000000000..143e599819
+extern void InitCompressFileHandleNone(CompressFileHandle *CFH,

 
 	/* Close the output */
 	errno = 0;
-	res = cfclose(AH->OF);
 
 	if (res != 0)

 

 	switch (AH->format)
 	{

-	if (fn >= 0)
 
-	if (!AH->OF)
 
-static cfp *
 
 static void
 
 	errno = 0;
-	res = cfclose(AH->OF);
 
 	if (res != 0)

+static bool

 
 	/*

 

 
 
 /*
 
 	if (dLen > 0)
 
 /*

 	WriteInt(AH, oid);

 /*
 
 static void

 

 

 
 
 	errno = 0;

 	/* Close the file */
 
 	ctx->dataFH = NULL;
 
 	if (!filename)
 		return;

 
 
 	free(buf);
-	if (cfclose(cfp) != 0)

 

 

 
 	errno = 0;
 
 	return 1;

 /*
 
 	errno = 0;

 	/*

 

 

 

 

 /*

 
+/*

From a240440a9ec73d0e156aa68c1ec3b7eb476a4f99 Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Wed, 25 Jan 2023 14:46:07 +0000
Subject: [PATCH v25 3/3] Add LZ4 compression to pg_dump

 src/bin/pg_dump/compress_lz4.c        | 626 ++++++++++++++++++++++++++
 src/bin/pg_dump/compress_lz4.h        |  24 +
 12 files changed, 786 insertions(+), 22 deletions(-)

 
index f47ba5e615..55db63ac7a 100644

 
 	return cs;
 }
@@ -159,6 +162,8 @@ InitCompressFileHandle(const pg_compress_specification compression_spec)
 		InitCompressFileHandleNone(CFH, compression_spec);
 	else if (compression_spec.algorithm == PG_COMPRESSION_GZIP)
 		InitCompressFileHandleGzip(CFH, compression_spec);
+	else if (compression_spec.algorithm == PG_COMPRESSION_LZ4)
+		InitCompressFileHandleLZ4(CFH, compression_spec);
 
 	return CFH;
 }
 
index 0000000000..ee74cc8e28
@@ -0,0 +1,626 @@
+InitCompressFileHandleLZ4(CompressFileHandle *CFH,
+InitCompressorLZ4(CompressorState *cs,
+InitCompressFileHandleLZ4(CompressFileHandle *CFH,
index 0000000000..40dbe00d46
+extern void InitCompressorLZ4(CompressorState *cs,
+extern void InitCompressFileHandleLZ4(CompressFileHandle *CFH,

 
 		/*

 	/*

---

From: Tomas Vondra <[email-masked]>
Date: Wed, 25 Jan 2023 18:28:49 +0100

> follow if the error is thrown once during that check.

Actually, I don't understand why 0001 moves the check into the loop. I
mean, why not check HAVE_LIBZ before the loop?

> is not aware of that and should not be aware of it, should it?

Not sure. What happens in a similar situation now? That is, when trying
to deal with an archive gzip-compressed in a build without libz?

regards

---

From: Justin Pryzby <[email-masked]>
Date: Wed, 25 Jan 2023 12:00:20 -0600

> and suspenders approach because there are no more checks after this point.

While looking at this, I realized that commit 5e73a6048 introduced a
regression:

@@ -3740,19 +3762,24 @@ ReadHead(ArchiveHandle *AH)

-       if (AH->compression != 0)
-               pg_log_warning("archive is compressed, but this installation does not support compression -- no data will be available");
+       if (AH->compression_spec.algorithm == PG_COMPRESSION_GZIP)
+               pg_fatal("archive is compressed, but this installation does not support compression");

Before, it was possible to restore non-data chunks of a dump file, even
if the current build didn't support its compression.  But that's now
impossible - and it makes the code we're discussing in RestoreArchive()
unreachable.

I don't think we can currently test for that, since it requires creating a dump
using a build --with compression and then trying to restore using a build
--without compression.  The coverage report disagrees with me, though...
https://coverage.postgresql.org/src/bin/pg_dump/pg_backup_archiver.c.gcov.html#3901

...
> is not aware of that and should not be aware of it, should it?

You're right.

I think the 001 patch should try to remove hardcoded references to
LIBZ/GZIP, such that the later patches don't need to update those same
places for LZ4.  For example in ReadHead() and RestoreArchive(), and
maybe other places dealing with file extensions.  Maybe that could be
done by adding a function specific to pg_dump indicating whether or not
an algorithm is implemented and supported.

-- 
Justin

---

From: [email-masked]
Date: Wed, 25 Jan 2023 19:50:49 +0000

------- Original Message -------
On Wednesday, January 25th, 2023 at 6:28 PM, Tomas Vondra <[email-masked]> wrote:

> > On Wednesday, January 25th, 2023 at 2:42 AM, Justin Pryzby [email-masked] wrote:
> > 

The intention is to be able to restore archives that don't contain
data. In that case compression becomes irrelevant as only the data in
an archive is compressed.

> to deal with an archive gzip-compressed in a build without libz?

In case that there are no data chunks, the archive will be restored.

Cheers,
//Georgios

---

From: [email-masked]
Date: Wed, 25 Jan 2023 19:57:18 +0000

------- Original Message -------

> impossible - and it makes the code we're discussing in RestoreArchive()

Nice catch!

Cheers,
//Georgios

---

From: Michael Paquier <[email-masked]>
Date: Thu, 26 Jan 2023 13:46:54 +0900

> Nice catch!

Let me see..
--
Michael

---

From: Michael Paquier <[email-masked]>
Date: Thu, 26 Jan 2023 14:49:27 +0900

> impossible - and it makes the code we're discussing in RestoreArchive()

Right.  The impacts the possibility of looking at the header data,
which is useful with pg_restore -l for example.  On a dump that's been
compressed, pg_restore <= 15 would always print the TOC entries with
or without compression support.  On HEAD, this code prevents the
header lookup.  All *nix or BSD platforms should have support for
zlib, I hope..  Still that could be an issue on Windows, and this
would prevent folks to check the contents of the dumps after saving it
on a WIN32 host, so let's undo that.

So, I have been testing the attached with four sets of binaries from
15/HEAD and with[out] zlib support, and this brings HEAD back to the
pre-15 state (header information able to show up, still failure when
attempting to restore the dump's data without zlib).

> --without compression.

Right, the location of the data is in the header, and I don't see how
you would be able to do that without two sets of binaries at hand, but
our tests run under the assumption that you have only one.  Well,
that's not entirely true as well, as you could create a TAP test like
pg_upgrade that relies on a environment variable pointing to a second
set of binaries.  That's not worth the complication involved, IMO.

> The coverage report disagrees with me, though...

Isn't that one of the tests like compression_gzip_plain?

Thoughts?
--
Michael
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index ba5e6acbbb..cb4386f871 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -3784,7 +3784,7 @@ ReadHead(ArchiveHandle *AH)
 
 #ifndef HAVE_LIBZ
 	if (AH->compression_spec.algorithm == PG_COMPRESSION_GZIP)
-		pg_fatal("archive is compressed, but this installation does not support compression");
+		pg_log_warning("archive is compressed, but this installation does not support compression -- no data will be available");
 #endif

---

From: Justin Pryzby <[email-masked]>
Date: Thu, 26 Jan 2023 00:28:46 -0600

On Thu, Jan 26, 2023 at 02:49:27PM +0900, Michael Paquier wrote:
> which is useful with pg_restore -l for example.

It's not just header data - it's schema and (I think) everything other
than table data.

> Isn't that one of the tests like compression_gzip_plain?

I'm not sure what you mean.  Plain dump is restored with psql and not
with pg_restore.

My line number was wrong:
https://coverage.postgresql.org/src/bin/pg_dump/pg_backup_archiver.c.gcov.html#390

What test would hit that code without rebuilding ?

394             : #ifndef HAVE_LIBZ
395             :     if (AH->compression_spec.algorithm == PG_COMPRESSION_GZIP &&

> +		pg_log_warning("archive is compressed, but this installation does not support compression -- no data will be available");

Your patch is fine for now, but these errors should eventually specify
*which* compression algorithm is unavailable.  I think that should be a
part of the 001 patch, ideally in a way that minimizes the number of
places which need to be updated when adding an algorithm.

-- 
Justin

---

From: [email-masked]
Date: Thu, 26 Jan 2023 11:24:47 +0000

------- Original Message -------
On Thursday, January 26th, 2023 at 7:28 AM, Justin Pryzby <[email-masked]> wrote:

> which compression algorithm is unavailable. I think that should be a

I gave this a little bit of thought. I think that ReadHead should not
emit a warning, or at least not this warning as it is slightly misleading.
It implies that it will automatically turn off data restoration, which is
false. Further ahead, the code will fail with a conflicting error message
stating that the compression is not available.

Instead, it would be cleaner both for the user and the maintainer to
move the check in RestoreArchive and make it the sole responsible for
this logic.

Please find v26 attached. 0001 does the above and 0002 addresses Justin's
complaints regarding the code footprint.

//Cheers,
Georgios 

From 30ae227823baf3694e5bb9f0bac715082a317f09 Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Thu, 26 Jan 2023 11:11:14 +0000
Subject: [PATCH v26 2/4] Prepare pg_dump internals for additional compression
 methods.

Commit  bf9aa490db introduced cfp in compress_io.{c,h} with the intent of
unifying compression related code and allow for the introduction of additional
archive formats. However, pg_backup_archiver.c was not using that API. This
commit teaches pg_backup_archiver.c about it and is using it throughout.
---
 src/bin/pg_dump/compress_io.c        |  92 +++++++++++++++++---
 src/bin/pg_dump/compress_io.h        |   4 +
 src/bin/pg_dump/pg_backup_archiver.c | 125 +++++++++------------------
 src/bin/pg_dump/pg_backup_archiver.h |  27 +-----
 4 files changed, 124 insertions(+), 124 deletions(-)

index 7a2c80bbc4..e1733ce57c 100644
@@ -56,6 +56,36 @@
 
+#ifdef HAVE_LIBZ
+#include <zlib.h>
+#endif
+
+/*----------------------
+ * Generic functions
+ *----------------------
+ */
+
+char *
+supports_compression(const pg_compress_specification compression_spec)
+{
+	const pg_compress_algorithm	algorithm = compression_spec.algorithm;
+	bool						supported = false;
+
+	if (algorithm == PG_COMPRESSION_NONE)
+		supported = true;
+#ifdef HAVE_LIBZ
+	if (algorithm == PG_COMPRESSION_GZIP)
+		supported = true;
+#endif
+
+	if (!supported)
+		return psprintf("this build does not support compression with %s",
+						get_compress_algorithm_name(algorithm));
+
+	return NULL;
+}
+
+
 /*----------------------
  * Compressor API
  *----------------------
@@ -490,16 +520,19 @@ cfopen_write(const char *path, const char *mode,
 }
 
 /*
 
 	if (compression_spec.algorithm == PG_COMPRESSION_GZIP)
 	{
@@ -511,15 +544,20 @@ cfopen(const char *path, const char *mode,

-		fp->uncompressedfp = NULL;
 		if (fp->compressedfp == NULL)
 		{
 			free_keep_errno(fp);
@@ -531,10 +569,11 @@ cfopen(const char *path, const char *mode,
 	}
 	else
 	{
-#ifdef HAVE_LIBZ
-		fp->compressedfp = NULL;
-#endif
-		fp->uncompressedfp = fopen(path, mode);
+		if (fd >= 0)
+			fp->uncompressedfp = fdopen(fd, mode);
+		else
+			fp->uncompressedfp = fopen(path, mode);
+
 		if (fp->uncompressedfp == NULL)
 		{
 			free_keep_errno(fp);
@@ -545,6 +584,33 @@ cfopen(const char *path, const char *mode,
 	return fp;
 }
 
+/*
 
 int
index a429dc4789..8beb1058ec 100644
@@ -21,6 +21,8 @@
 
+extern char *supports_compression(const pg_compress_specification compression_spec);
+
 /* Prototype for callback function to WriteDataToArchive() */
 typedef void (*WriteFunc) (ArchiveHandle *AH, const char *buf, size_t len);
 
@@ -54,6 +56,8 @@ typedef struct cfp cfp;
 
index 25b1ea0026..f3b92b01d9 100644

 
 
 	/* Close the output */
 
 	if (res != 0)
 		pg_fatal("could not close output file: %m");
@@ -363,7 +354,7 @@ RestoreArchive(Archive *AHX)
 	RestoreOptions *ropt = AH->public.ropt;
 	bool		parallel_mode;
 	TocEntry   *te;
-	OutputContext sav;
+	cfp		   *sav;

@@ -391,17 +382,20 @@ RestoreArchive(Archive *AHX)
 	/*
 	 * Make sure we won't need (de)compression we haven't got
 	 */
-#ifndef HAVE_LIBZ
-	if (AH->compression_spec.algorithm == PG_COMPRESSION_GZIP &&
-		AH->PrintTocDataPtr != NULL)
+	if (AH->PrintTocDataPtr != NULL)
 	{
 		for (te = AH->toc->next; te != AH->toc; te = te->next)
 		{
 			if (te->hadDumper && (te->reqs & REQ_DATA) != 0)
-				pg_fatal("cannot restore data from compressed archive (compression not supported in this installation)");
+			{
+				char *errmsg = supports_compression(AH->compression_spec);
+				if (errmsg)	
+					pg_fatal("cannot restore data from compressed archive (%s)", errmsg);
+				else
+					break;
+			}
 		}
 	}
-#endif
 
 	/*
 	 * Prepare index arrays, so we can assume we have them throughout restore.
@@ -1133,7 +1127,7 @@ PrintTOCSummary(Archive *AHX)
 	TocEntry   *te;
 	pg_compress_specification out_compression_spec = {0};
 	teSection	curSection;
-	OutputContext sav;
+	cfp		   *sav;
 	const char *fmtName;
 	char		stamp_str[64];
 
@@ -1508,58 +1502,32 @@ static void
 SetOutput(ArchiveHandle *AH, const char *filename,
 		  const pg_compress_specification compression_spec)
 {
-	int			fn;
+	const char *mode;
+	int			fn = -1;
 
 	if (filename)
 	{

 
 	if (!AH->OF)
 	{
@@ -1570,33 +1538,24 @@ SetOutput(ArchiveHandle *AH, const char *filename,
 	}
 }
 
-static OutputContext
+static cfp *
 
 static void

 	if (res != 0)

@@ -1720,22 +1679,17 @@ ahwrite(const void *ptr, size_t size, size_t nmemb, ArchiveHandle *AH)

+	/*
 
 	if (bytes_written != size * nmemb)
 		WRITE_ERROR_EXIT;
@@ -2224,6 +2178,7 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,
 		 SetupWorkerPtrType setupWorkerPtr)
 {
 	ArchiveHandle *AH;
+	pg_compress_specification out_compress_spec = {0};
 
 	pg_log_debug("allocating AH for %s, format %d",
 				 FileSpec ? FileSpec : "(stdio)", fmt);
@@ -2277,8 +2232,8 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,
 	memset(&(AH->sqlparse), 0, sizeof(AH->sqlparse));

 	/*
 
 #define LOBBUFSIZE 16384
 
-#ifdef HAVE_LIBZ
-#include <zlib.h>

From 2c2db63ede38c2e61fb655ab38303f4160ec3f6c Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Thu, 26 Jan 2023 09:58:10 +0000
Subject: [PATCH v26 1/4] Address regression in pg_dump's ReadHead

Commit 5e73a6048 upgraded the check for supported compression while parsing an
archive's header, from warning to fatal. Prior to this commit, it was possible
to restore a compressed archive's schema even when the compression was not
supported by the binary.

Before the abovementioned commit, a warning message would be emmited:

	pg_log_warning("archive is compressed, but this installation does not support compression -- no data will be available");

This message is slightly miss-leading, as it can be interpreted that the binary
will actively switch into schema only mode, which is not the case. Instead a new
fatal message will appear informing that the compression is not available.

This commit chooses to remove the check in ReadHead altogether in favour of a
more comprehensive error message when checking the archive for data.

Regression spotted by: Justin Pryzby
---
 src/bin/pg_dump/pg_backup_archiver.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index ba5e6acbbb..25b1ea0026 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -398,7 +398,7 @@ RestoreArchive(Archive *AHX)
 		for (te = AH->toc->next; te != AH->toc; te = te->next)
 		{
 			if (te->hadDumper && (te->reqs & REQ_DATA) != 0)
-				pg_fatal("cannot restore from compressed archive (compression not supported in this installation)");
+				pg_fatal("cannot restore data from compressed archive (compression not supported in this installation)");
 		}
 	}
 #endif
@@ -3782,11 +3782,6 @@ ReadHead(ArchiveHandle *AH)
 	else
 		AH->compression_spec.algorithm = PG_COMPRESSION_GZIP;
 
-#ifndef HAVE_LIBZ
 		struct tm	crtm;
-- 
2.34.1

From f05feb5c446cb37b580629fa51e0b5757ad09c61 Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Wed, 25 Jan 2023 14:46:07 +0000
Subject: [PATCH v26 4/4] Add LZ4 compression to pg_dump

 src/bin/pg_dump/compress_io.c         |  24 +-
 src/bin/pg_dump/pg_backup_archiver.c  |   6 +-
 12 files changed, 780 insertions(+), 21 deletions(-)

 
index e4e008d2e1..6563f67689 100644

@@ -90,6 +91,10 @@ supports_compression(const pg_compress_specification compression_spec)
 	if (algorithm == PG_COMPRESSION_GZIP)
 		supported = true;
 #endif
+#ifdef USE_LZ4
+	if (algorithm == PG_COMPRESSION_LZ4)
+		supported = true;
+#endif
 
 	if (!supported)
 		return psprintf("this build does not support compression with %s",
@@ -121,6 +126,8 @@ AllocateCompressor(const pg_compress_specification compression_spec,
 		InitCompressorNone(cs, compression_spec);
 	else if (compression_spec.algorithm == PG_COMPRESSION_GZIP)
 		InitCompressorGzip(cs, compression_spec);
+	else if (compression_spec.algorithm == PG_COMPRESSION_LZ4)
+		InitCompressorLZ4(cs, compression_spec);
 
 	return cs;
 }
@@ -185,6 +192,8 @@ InitCompressFileHandle(const pg_compress_specification compression_spec)
 
 	return CFH;
 }
@@ -198,7 +207,7 @@ InitCompressFileHandle(const pg_compress_specification compression_spec)
@@ -236,6 +245,17 @@ InitDiscoverCompressFileHandle(const char *path, const char *mode)

index 1405459723..717002bf92 100644
@@ -2074,7 +2074,7 @@ _discoverArchiveFormat(ArchiveHandle *AH)
 
 		/*
@@ -2084,6 +2084,10 @@ _discoverArchiveFormat(ArchiveHandle *AH)

 	/*

From f523d2a2edee4b9b7804a2539230832b8d1bdf2c Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Wed, 25 Jan 2023 14:45:50 +0000
Subject: [PATCH v26 3/4] Introduce Compress and Compressor API in pg_dump

 src/bin/pg_dump/compress_io.c         | 744 +++++---------------------
 src/bin/pg_dump/pg_backup_archiver.c  |  91 ++--
 16 files changed, 1051 insertions(+), 749 deletions(-)

 OBJS = \
 	$(WIN32RES) \
+	compress_gzip.o \
 	compress_io.o \
+	compress_none.o \
 	dumputils.o \
 	parallel.o \
 	pg_backup_archiver.o \
index e1733ce57c..e4e008d2e1 100644
@@ -53,13 +65,14 @@
  */
 #include "postgres_fe.h"
 
+#include <sys/stat.h>
+#include <unistd.h>
+
 
-#ifdef HAVE_LIBZ
-#include <zlib.h>
-#endif
-
 /*----------------------
  * Generic functions
  *----------------------
@@ -91,110 +104,25 @@ supports_compression(const pg_compress_specification compression_spec)
  *----------------------
  */

-#ifndef HAVE_LIBZ
 
-/*
 
-/*
 
 /*
@@ -203,233 +131,31 @@ WriteDataToArchive(ArchiveHandle *AH, CompressorState *cs,
 void
 EndCompressor(ArchiveHandle *AH, CompressorState *cs)
 {
-#ifdef HAVE_LIBZ
-	if (cs->compression_spec.algorithm == PG_COMPRESSION_GZIP)
-		EndCompressorZlib(AH, cs);
-#endif
-	free(cs);
+	cs->end(AH, cs);
+	pg_free(cs);
 }

 /*
 
-#ifdef HAVE_LIBZ
 
 /* free() without changing errno; useful in several places below */
 static void
@@ -442,324 +168,102 @@ free_keep_errno(void *p)
 }
 
 /*
 
 /*

 
-	return fp;
+	return CFH;
 }
 
 /*
 
-int
 
-	if (size == 0)
-		return 0;
 
-#ifdef HAVE_LIBZ
-	if (fp->compressedfp)
-	{

 
-	return ret;
-}
-
-char *
 
-	return result;
+	return CFH;
 }
 
+/*
 
-const char *
-get_cfp_error(cfp *fp)
-{
-#ifdef HAVE_LIBZ
-	if (fp->compressedfp)
-	{
-		int			errnum;
 
-		if (errnum != Z_ERRNO)
-			return errmsg;
-	}
-#endif
 
-#ifdef HAVE_LIBZ
-static int
index 8beb1058ec..8280d7b0dc 100644
@@ -23,50 +23,160 @@
 
 extern char *supports_compression(const pg_compress_specification compression_spec);

 /*

 
+/*
+ * Compress File Handle
+ */
 
-typedef struct cfp cfp;
+	/*

+	/*
 
index f3b92b01d9..1405459723 100644

 	/* Close the output */
 	errno = 0;
-	res = cfclose(AH->OF);
 
 	if (res != 0)
 		pg_fatal("could not close output file: %m");
@@ -354,7 +354,7 @@ RestoreArchive(Archive *AHX)
 	RestoreOptions *ropt = AH->public.ropt;
 	bool		parallel_mode;
 	TocEntry   *te;
-	cfp		   *sav;
+	CompressFileHandle *sav;

@@ -1127,7 +1127,7 @@ PrintTOCSummary(Archive *AHX)
 	TocEntry   *te;
 	pg_compress_specification out_compression_spec = {0};
 	teSection	curSection;
-	cfp		   *sav;
+	CompressFileHandle *sav;
 	const char *fmtName;
 	char		stamp_str[64];
 
@@ -1143,9 +1143,10 @@ PrintTOCSummary(Archive *AHX)
 		strcpy(stamp_str, "[unknown]");

 	switch (AH->format)
 	{
@@ -1502,6 +1503,7 @@ static void
 SetOutput(ArchiveHandle *AH, const char *filename,
 		  const pg_compress_specification compression_spec)
 {
+	CompressFileHandle *CFH;
 	const char *mode;
 	int			fn = -1;
 
@@ -1524,33 +1526,32 @@ SetOutput(ArchiveHandle *AH, const char *filename,
 	else
 		mode = PG_BINARY_W;
 
-	if (fn >= 0)
 
-	if (!AH->OF)
 
-static cfp *
 
 static void
 
 	errno = 0;
-	res = cfclose(AH->OF);
 
 	if (res != 0)
@@ -1689,7 +1690,11 @@ ahwrite(const void *ptr, size_t size, size_t nmemb, ArchiveHandle *AH)
 
 	if (bytes_written != size * nmemb)
 		WRITE_ERROR_EXIT;
@@ -2031,6 +2036,18 @@ ReadStr(ArchiveHandle *AH)
 	return buf;
 }
 
+static bool
@@ -2061,26 +2078,12 @@ _discoverArchiveFormat(ArchiveHandle *AH)
@@ -2178,6 +2181,7 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,
 
 	pg_log_debug("allocating AH for %s, format %d",
@@ -2233,7 +2237,10 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,

 	/*
@@ -3646,12 +3653,7 @@ WriteHead(ArchiveHandle *AH)
@@ -3722,10 +3724,11 @@ ReadHead(ArchiveHandle *AH)

 

 /*
 
 	if (dLen > 0)
 
 /*

 	WriteInt(AH, oid);

 /*
 
 static void

 

 

 
 
 	errno = 0;

 	/* Close the file */
 
 	ctx->dataFH = NULL;
 
 	if (!filename)
 		return;

 
 
 	free(buf);
-	if (cfclose(cfp) != 0)

 

 

 
 	errno = 0;
 
 	return 1;

 /*
 
 	errno = 0;

 	/*

 

 

 

 

 /*

 
+/*

---

From: Michael Paquier <[email-masked]>
Date: Thu, 26 Jan 2023 20:53:28 +0900

-    pg_fatal("cannot restore from compressed archive (compression not supported in this installation)");
+    pg_fatal("cannot restore data from compressed archive (compression not supported in this installation)");
Hmm.  I don't mind changing this part as you suggest.

-#ifndef HAVE_LIBZ
-       if (AH->compression_spec.algorithm == PG_COMPRESSION_GZIP)
-               pg_fatal("archive is compressed, but this installation does not support compression");
-#endif
However I think that we'd better keep the warning, as it can offer a
hint when using pg_restore -l not built with compression support if
looking at a dump that has been compressed.
--
Michael

---

From: [email-masked]
Date: Thu, 26 Jan 2023 18:03:59 +0000

------- Original Message -------

> looking at a dump that has been compressed.

Fair enough. Please find v27 attached.

Cheers,
//Georgios

From d68ef7799f2cfb2d94402679157c8bcf6bca5273 Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Thu, 26 Jan 2023 17:49:06 +0000
Subject: [PATCH v27 1/4] Address regression in pg_dump's ReadHead

Regression spotted by: Justin Pryzby
---
 src/bin/pg_dump/pg_backup_archiver.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

index ba5e6acbbb..4d9114cdd8 100644
@@ -3784,9 +3784,10 @@ ReadHead(ArchiveHandle *AH)
 
 #ifndef HAVE_LIBZ
 	if (AH->compression_spec.algorithm == PG_COMPRESSION_GZIP)
-		pg_fatal("archive is compressed, but this installation does not support compression");
+		pg_log_warning("archive is compressed, but this installation does not support compression");
 #endif
 
+

From a6673dadf3f4e2f0ac08631181fecf98a8aa4948 Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Thu, 26 Jan 2023 18:00:23 +0000
Subject: [PATCH v27 4/4] Add LZ4 compression to pg_dump

 

 
 
 	if (!supported)
 
 	return cs;
 }
 
 	return CFH;
 }

index 02178bdc53..d368cacabb 100644
 
 		/*

 	/*

From 31142304229200fcb189f6852dad28a827160443 Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Thu, 26 Jan 2023 17:58:54 +0000
Subject: [PATCH v27 2/4] Prepare pg_dump internals for additional compression
 methods.

Commit  bf9aa490db introduced cfp in compress_io.{c,h} with the intent of
unifying compression related code and allow for the introduction of additional
archive formats. However, pg_backup_archiver.c was not using that API. This
commit teaches pg_backup_archiver.c about it and is using it throughout.
---
 src/bin/pg_dump/compress_io.c        |  92 +++++++++++++++---
 src/bin/pg_dump/compress_io.h        |   4 +
 src/bin/pg_dump/pg_backup_archiver.c | 138 ++++++++++-----------------
 src/bin/pg_dump/pg_backup_archiver.h |  27 +-----
 4 files changed, 132 insertions(+), 129 deletions(-)

+#ifdef HAVE_LIBZ
+#include <zlib.h>
+#endif
+
 
 /*

 
 
+/*
 
 int

 
index 4d9114cdd8..a4e5df0817 100644

 
 
 	/* Close the output */
 
 	if (res != 0)

 
 	/*

 	if (filename)
 	{

 
 	if (!AH->OF)
 	{
 
-static OutputContext
+static cfp *
 
 static void

 	if (res != 0)

 
 
+	/*

 
 
 	/*
 	 * On Windows, we need to use binary mode to read/write non-text files,
@@ -3713,6 +3668,7 @@ WriteHead(ArchiveHandle *AH)
 void
 ReadHead(ArchiveHandle *AH)
 {
+	char	   *errmsg;
 	char		vmaj,
 				vmin,
 				vrev;
@@ -3782,11 +3738,13 @@ ReadHead(ArchiveHandle *AH)
 	else
 		AH->compression_spec.algorithm = PG_COMPRESSION_GZIP;
 
-#ifndef HAVE_LIBZ
-	if (AH->compression_spec.algorithm == PG_COMPRESSION_GZIP)
-		pg_log_warning("archive is compressed, but this installation does not support compression");
-#endif
-
+	errmsg = supports_compression(AH->compression_spec);
+	if (errmsg)
+	{
+		pg_log_warning("archive is compressed but data cannot be restored (%s)",
+						errmsg);
+		pg_free(errmsg);
+	}

 #define LOBBUFSIZE 16384
 
-#ifdef HAVE_LIBZ
-#include <zlib.h>

From 06c6385d338d6b30390f4d125b1059148d4badd3 Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Thu, 26 Jan 2023 17:59:35 +0000
Subject: [PATCH v27 3/4] Introduce Compress and Compressor API in pg_dump

 
 OBJS = \
 	$(WIN32RES) \
+	compress_gzip.o \
 	compress_io.o \
+	compress_none.o \
 	dumputils.o \
 	parallel.o \
 	pg_backup_archiver.o \
 
+#include <sys/stat.h>
+#include <unistd.h>
+
 
-#ifdef HAVE_LIBZ
-#include <zlib.h>
-#endif
-
 /*----------------------
  * Generic functions
  *----------------------

-#ifndef HAVE_LIBZ
 
-/*
 
-/*
 
 /*

 /*
 
-#ifdef HAVE_LIBZ

 /*
 
 /*

 
-	return fp;
+	return CFH;
 }
 
 /*
 
-int
 
-	if (size == 0)
-		return 0;
 
-#ifdef HAVE_LIBZ
-	if (fp->compressedfp)
-	{

 
-	return ret;
-}
-
-char *
 
-	return result;
+	return CFH;
 }
 
+/*
 
-const char *
-get_cfp_error(cfp *fp)
-{
-#ifdef HAVE_LIBZ
-	if (fp->compressedfp)
-	{
-		int			errnum;
 
-		if (errnum != Z_ERRNO)
-			return errmsg;
-	}
-#endif
 
-#ifdef HAVE_LIBZ
-static int

 
 /*

 
+/*
+ * Compress File Handle
+ */
 
-typedef struct cfp cfp;
+	/*

+	/*
 
index a4e5df0817..02178bdc53 100644

 	/* Close the output */
 	errno = 0;
-	res = cfclose(AH->OF);
 
 	if (res != 0)

 

 	switch (AH->format)
 	{

-	if (fn >= 0)
 
-	if (!AH->OF)
 
-static cfp *
 
 static void
 
 	errno = 0;
-	res = cfclose(AH->OF);
 
 	if (res != 0)

+static bool

 
 	/*
@@ -3723,10 +3725,11 @@ ReadHead(ArchiveHandle *AH)

 

 /*
 
 	if (dLen > 0)
 
 /*

 	WriteInt(AH, oid);

 /*
 
 static void

 

 

 
 
 	errno = 0;

 	/* Close the file */
 
 	ctx->dataFH = NULL;
 
 	if (!filename)
 		return;

 
 
 	free(buf);
-	if (cfclose(cfp) != 0)

 

 

 
 	errno = 0;
 
 	return 1;

 /*
 
 	errno = 0;

 	/*

 

 

 

 

 /*

 
+/*

---

From: "[email-masked]" <[email-masked]>
Date: Wed, 15 Feb 2023 13:51:21 +0000

Hi,

I am interested in this feature and tried the patch. While reading the comments,
I noticed some minor things that could possibly be improved (in v27-0003 patch).

1.
+	/*

There is a redundant single quote in front of 'w'.

2.
/*
 * Callback function for WriteDataToArchive. Writes one block of (compressed)
 * data to the archive.
 */
/*
 * Callback function for ReadDataFromArchive. To keep things simple, we
 * always read one compressed block at a time.
 */

Should the function names in the comments be updated?

WriteDataToArchive
->
writeData

ReadDataFromArchive
->
readData

3.

Could we use PG_BINARY_R instead of "r" and "rb" here?

Regards,
Shi Yu

---

From: [email-masked]
Date: Thu, 16 Feb 2023 11:38:00 +0000

------- Original Message -------

> I noticed some minor things that could possibly be improved (in v27-0003 patch).

Thank you very much for the interest. Please find a rebased v28 attached. Due to
the rebase, 0001 of v27 is no longer relevant and has been removed. Your comments
are applied on v28-0002.

> + CompressFileHandle CFH);
> 

Fixed.

> /
> /
> /

Agreed. Fixed.

> Could we use PG_BINARY_R instead of "r" and "rb" here?

We could and we should. Using PG_BINARY_R has the added benefit
of needing only one strcmp() call. Fixed.

Cheers,
//Georgios

> Shi Yu

From d5a8e11d78e8ca9b592f9abb5f66396df4547727 Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Thu, 16 Feb 2023 11:00:35 +0000
Subject: [PATCH v28 3/3] Add LZ4 compression to pg_dump

 
 
index c507eeb7b3..929b38902b 100644

 
 	if (!supported)
 
 	return cs;
 }
 
 	return CFH;
 }

index 2bc5648ed6..027ded4bae 100644
@@ -2075,7 +2075,7 @@ _discoverArchiveFormat(ArchiveHandle *AH)
 
 		/*
@@ -2085,6 +2085,10 @@ _discoverArchiveFormat(ArchiveHandle *AH)

 	/*

index 1d59b82003..126b33af66 100644
@@ -1386,6 +1386,7 @@ LWLock

From 6094277728b21df5d9e48d958d2a46b66e95f103 Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Thu, 16 Feb 2023 11:00:25 +0000
Subject: [PATCH v28 2/3] Introduce Compress and Compressor API in pg_dump

 src/bin/pg_dump/pg_backup_custom.c    |  27 +-
 16 files changed, 1053 insertions(+), 751 deletions(-)

 OBJS = \
 	$(WIN32RES) \
+	compress_gzip.o \
 	compress_io.o \
+	compress_none.o \
 	dumputils.o \
 	parallel.o \
 	pg_backup_archiver.o \
index e1733ce57c..c507eeb7b3 100644
 
+#include <sys/stat.h>
+#include <unistd.h>
+
 
-#ifdef HAVE_LIBZ
-#include <zlib.h>
-#endif
-
 /*----------------------
  * Generic functions
  *----------------------

-#ifndef HAVE_LIBZ
 
-/*
 
-/*
 
 /*

 /*
 
-#ifdef HAVE_LIBZ

 /*
 
 /*

 
-	return fp;
+	return CFH;
 }
 
 /*
 
-int
 
-	if (size == 0)
-		return 0;
+	Assert(strcmp(mode, PG_BINARY_R) == 0);
 
-#ifdef HAVE_LIBZ
-	if (fp->compressedfp)
-	{

 
-	return ret;
-}
-
-char *
 
-	return result;
+	return CFH;
 }
 
+/*
 
-const char *
-get_cfp_error(cfp *fp)
-{
-#ifdef HAVE_LIBZ
-	if (fp->compressedfp)
-	{
-		int			errnum;
 
-		if (errnum != Z_ERRNO)
-			return errmsg;
-	}
-#endif
 
-#ifdef HAVE_LIBZ
-static int
index 8beb1058ec..d41c495025 100644

 
 /*

 
+/*
+ * Compress File Handle
+ */
+	 * 'mode' can be one of 'w', 'wb', 'a', and 'ab'. Requrires an already
 
-typedef struct cfp cfp;
+	/*

+	/*
 
index e02ce22db2..2bc5648ed6 100644

 	/* Close the output */
 	errno = 0;
-	res = cfclose(AH->OF);
 
 	if (res != 0)

@@ -1128,7 +1128,7 @@ PrintTOCSummary(Archive *AHX)
 	TocEntry   *te;
 	pg_compress_specification out_compression_spec = {0};
 	teSection	curSection;
-	cfp		   *sav;
+	CompressFileHandle *sav;
 	const char *fmtName;
 	char		stamp_str[64];
 
@@ -1144,9 +1144,10 @@ PrintTOCSummary(Archive *AHX)
 		strcpy(stamp_str, "[unknown]");

 	switch (AH->format)
 	{
@@ -1503,6 +1504,7 @@ static void
 SetOutput(ArchiveHandle *AH, const char *filename,
 		  const pg_compress_specification compression_spec)
 {
+	CompressFileHandle *CFH;
 	const char *mode;
 	int			fn = -1;
 
@@ -1525,33 +1527,32 @@ SetOutput(ArchiveHandle *AH, const char *filename,
 	else
 		mode = PG_BINARY_W;
 
-	if (fn >= 0)
 
-	if (!AH->OF)
 
-static cfp *
 
 static void
 
 	errno = 0;
-	res = cfclose(AH->OF);
 
 	if (res != 0)
@@ -1690,7 +1691,11 @@ ahwrite(const void *ptr, size_t size, size_t nmemb, ArchiveHandle *AH)
 
 	if (bytes_written != size * nmemb)
 		WRITE_ERROR_EXIT;
@@ -2032,6 +2037,18 @@ ReadStr(ArchiveHandle *AH)
 	return buf;
 }
 
+static bool
@@ -2062,26 +2079,12 @@ _discoverArchiveFormat(ArchiveHandle *AH)
@@ -2179,6 +2182,7 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,
 
 	pg_log_debug("allocating AH for %s, format %d",
@@ -2234,7 +2238,10 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,

 	/*

 
index 7529367a7b..b576b29924 100644

 /*
 
 	if (dLen > 0)
 
 /*

 	WriteInt(AH, oid);

 /*
 
 static void
@@ -977,7 +988,7 @@ _readBlockHeader(ArchiveHandle *AH, int *type, int *id)
 }
 
 /*
- * Callback function for WriteDataToArchive. Writes one block of (compressed)
+ * Callback function for writeData. Writes one block of (compressed)
  * data to the archive.
  */
 static void
@@ -992,7 +1003,7 @@ _CustomWriteFunc(ArchiveHandle *AH, const char *buf, size_t len)
 }
 
 /*
- * Callback function for ReadDataFromArchive. To keep things simple, we
+ * Callback function for readData. To keep things simple, we
  * always read one compressed block at a time.
  */
 static size_t
diff --git a/src/bin/pg_dump/pg_backup_directory.c b/src/bin/pg_dump/pg_backup_directory.c
index 6800c3ccee..a2f88995c0 100644
--- a/src/bin/pg_dump/pg_backup_directory.c
+++ b/src/bin/pg_dump/pg_backup_directory.c
@@ -50,9 +50,8 @@ typedef struct
 	 */
 	char	   *directory;

 

 

 
 
 	errno = 0;

 	/* Close the file */
 
 	ctx->dataFH = NULL;
 
 	if (!filename)
 		return;

 
 
 	free(buf);
-	if (cfclose(cfp) != 0)

 

 

 
 	errno = 0;
 
 	return 1;

 /*
 
 	errno = 0;

 	/*

 

 

 

 

 /*

 
+/*
 
index 45fc5759ce..1d59b82003 100644

From 1dc258e7d995145894b288d30ebd819de694ddfa Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Thu, 16 Feb 2023 10:47:59 +0000
Subject: [PATCH v28 1/3] Prepare pg_dump internals for additional compression
 methods.

 4 files changed, 133 insertions(+), 128 deletions(-)

+#ifdef HAVE_LIBZ
+#include <zlib.h>
+#endif
+
 
 /*

 
 
+/*
 
 int

 
index 269bfce019..e02ce22db2 100644

 
 
 	/* Close the output */
 
 	if (res != 0)

@@ -391,17 +382,21 @@ RestoreArchive(Archive *AHX)
 	/*
+					pg_fatal("cannot restore data from compressed archive (%s)",
+							  errmsg);
+				else
+					break;
+			}
 		}
 	}
-#endif
 
 	/*
 	 * Prepare index arrays, so we can assume we have them throughout restore.
@@ -1133,7 +1128,7 @@ PrintTOCSummary(Archive *AHX)
 	TocEntry   *te;
 	pg_compress_specification out_compression_spec = {0};
 	teSection	curSection;
-	OutputContext sav;
+	cfp		   *sav;
 	const char *fmtName;
 	char		stamp_str[64];
 
@@ -1508,58 +1503,32 @@ static void
 SetOutput(ArchiveHandle *AH, const char *filename,
 		  const pg_compress_specification compression_spec)
 {
-	int			fn;
+	const char *mode;
+	int			fn = -1;
 
 	if (filename)
 	{

 
 	if (!AH->OF)
 	{
@@ -1570,33 +1539,24 @@ SetOutput(ArchiveHandle *AH, const char *filename,
 	}
 }
 
-static OutputContext
+static cfp *
 
 static void

 	if (res != 0)

@@ -1720,22 +1680,17 @@ ahwrite(const void *ptr, size_t size, size_t nmemb, ArchiveHandle *AH)

+	/*
 
 	if (bytes_written != size * nmemb)
 		WRITE_ERROR_EXIT;
@@ -2224,6 +2179,7 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,
 		 SetupWorkerPtrType setupWorkerPtr)
 {
 	ArchiveHandle *AH;
+	pg_compress_specification out_compress_spec = {0};
 
 	pg_log_debug("allocating AH for %s, format %d",
 				 FileSpec ? FileSpec : "(stdio)", fmt);
@@ -2277,8 +2233,8 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,
 	memset(&(AH->sqlparse), 0, sizeof(AH->sqlparse));

 	/*
 	 * On Windows, we need to use binary mode to read/write non-text files,
@@ -3712,6 +3668,7 @@ WriteHead(ArchiveHandle *AH)
 void
 ReadHead(ArchiveHandle *AH)
 {
+	char	   *errmsg;
 	char		vmaj,
 				vmin,
 				vrev;
@@ -3781,10 +3738,13 @@ ReadHead(ArchiveHandle *AH)
 	else
 		AH->compression_spec.algorithm = PG_COMPRESSION_GZIP;
 
-#ifndef HAVE_LIBZ
-	if (AH->compression_spec.algorithm == PG_COMPRESSION_GZIP)
-		pg_log_warning("archive is compressed, but this installation does not support compression -- no data will be available");
-#endif
+	errmsg = supports_compression(AH->compression_spec);
+	if (errmsg)
+	{
+		pg_log_warning("archive is compressed, but this installation does not support compression (%s) -- no data will be available",
+						errmsg);
+		pg_free(errmsg);
+	}

 #define LOBBUFSIZE 16384
 
-#ifdef HAVE_LIBZ
-#include <zlib.h>

---

From: Tomas Vondra <[email-masked]>
Date: Sun, 19 Feb 2023 18:10:13 +0100

Hi Georgios,

I spent some time looking at the patch again, and IMO it's RFC. But I
need some help with the commit messages - I updated 0001 and 0002 but I
wasn't quite sure what some of the stuff meant to say and/or it seemed
maybe coming from an earlier patch version and obsolete.

Could you go over them and check if I got it right? Also feel free to
update the list of reviewers (I compiled that from substantial reviews
on the thread).

The 0003 commit message seems somewhat confusing - I added some XXX
lines asking about unclear stuff.

regards

From 1c0fdd6b294411368d4abd352c46bd31c58ba121 Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Thu, 16 Feb 2023 10:47:59 +0000
Subject: [PATCH 1/3] Prepare pg_dump internals for additional compression
 methods

Commit bf9aa490db a generic compression API in compress_io.{c,h} to make
reuse easier, and allow adding more compression algorithms. However,
pg_backup_archiver.c was not switched to this API and continued to call
the compression directly.

This commit teaches pg_backup_archiver.c about the compression API, so
that it can benefit from bf9aa490db (simpler code, easier addition of
new compression methods).

Most of the code was written by Georgios Kokolatos. Rachel Heaton provided
invaluable help out with expanding the testing coverage, testing on
different platforms and providing debug information on those, as well as
native speaker wording.

Author: Georgios Kokolatos, Rachel Heaton
Reviewed-by: Michael Paquier, Justin Pryzby, Daniel Gustafsson, Tomas
Vondra
Discussion: https://postgr.es/m/faUNEOpts9vunEaLnmxmG-DldLSg_ql137OC3JYDmgrOMHm1RvvWY2IdBkv_CRxm5spCCb_OmKNk2T03TMm0fBEWveFF9wA1WizPuAgB7Ss%3D%40protonmail.com
---
 src/bin/pg_dump/compress_io.c        |  92 +++++++++++++++---
 src/bin/pg_dump/compress_io.h        |   4 +
 src/bin/pg_dump/pg_backup_archiver.c | 138 ++++++++++-----------------
 src/bin/pg_dump/pg_backup_archiver.h |  27 +-----
 4 files changed, 133 insertions(+), 128 deletions(-)

index 7a2c80bbc4c..e1733ce57cf 100644
 
+#ifdef HAVE_LIBZ
+#include <zlib.h>
+#endif
+
 
 /*

 
 
+/*
 
 int
index a429dc4789d..8beb1058ec2 100644

 
index 269bfce019b..e02ce22db2e 100644

 
 
 	/* Close the output */
 
 	if (res != 0)

 
 	/*

 	if (filename)
 	{

 
 	if (!AH->OF)
 	{
 
-static OutputContext
+static cfp *
 
 static void

 	if (res != 0)

 
 
+	/*

 
 
 	/*
 
-#ifndef HAVE_LIBZ
 
index f72446ed5b4..4725e49747b 100644
 
 #define LOBBUFSIZE 16384
 
-#ifdef HAVE_LIBZ
-#include <zlib.h>

2.39.1

From f5002e563b48a0196143910748ce0d6b92a387c9 Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Thu, 16 Feb 2023 11:00:25 +0000
Subject: [PATCH 2/3] Introduce a generic pg_dump compression API

Switch pg_dump to use the Compression API, implemented by bf9aa490db.

The CompressFileHandle replaces the cfp* family of functions with a
struct of callbacks for accessing (compressed) files. This allows adding
new compression methods simply by introducing a new struct instance with
appropriate implementation of the callbacks.

Archives compressed using custom compression methods store an identifier
of the compression algorithm in their header instead of the compression
level. The header version is bumped.

Most of the code was written by Georgios Kokolatos. Rachel Heaton provided
invaluable help out with expanding the testing coverage, testing on
different platforms and providing debug information on those, as well as
native speaker wording.

Author: Georgios Kokolatos, Rachel Heaton
Reviewed-by: Michael Paquier, Justin Pryzby, Daniel Gustafsson, Tomas
Vondra
Discussion: https://postgr.es/m/faUNEOpts9vunEaLnmxmG-DldLSg_ql137OC3JYDmgrOMHm1RvvWY2IdBkv_CRxm5spCCb_OmKNk2T03TMm0fBEWveFF9wA1WizPuAgB7Ss%3D%40protonmail.com
---

index ef1ed0f3e51..0013bc080cf 100644
 
 OBJS = \
 	$(WIN32RES) \
+	compress_gzip.o \
 	compress_io.o \
+	compress_none.o \
 	dumputils.o \
 	parallel.o \
 	pg_backup_archiver.o \
index 00000000000..24e68fd0221
index 00000000000..2392c697b4c
index e1733ce57cf..c507eeb7b3d 100644
 
+#include <sys/stat.h>
+#include <unistd.h>
+
 
-#ifdef HAVE_LIBZ
-#include <zlib.h>
-#endif
-
 /*----------------------
  * Generic functions
  *----------------------

-#ifndef HAVE_LIBZ
 
-/*
 
-/*
 
 /*

 /*
 
-#ifdef HAVE_LIBZ

 /*
 
 /*

 
-	return fp;
+	return CFH;
 }
 
 /*
 
-int
 
-	if (size == 0)
-		return 0;
 
-#ifdef HAVE_LIBZ
-	if (fp->compressedfp)
-	{

 
-	return ret;
-}
-
-char *
 
-	return result;
+	return CFH;
 }
 
+/*
 
-const char *
-get_cfp_error(cfp *fp)
-{
-#ifdef HAVE_LIBZ
-	if (fp->compressedfp)
-	{
-		int			errnum;
 
-		if (errnum != Z_ERRNO)
-			return errmsg;
-	}
-#endif
 
-#ifdef HAVE_LIBZ
-static int
index 8beb1058ec2..d41c4950257 100644

 
 /*

 
+/*
+ * Compress File Handle
+ */
 
-typedef struct cfp cfp;
+	/*

+	/*
index 00000000000..ecbcf4b04a7
index 00000000000..143e599819d
index ca62f9a3740..84e9f0defa4 100644
 
index e02ce22db2e..2bc5648ed64 100644

 	/* Close the output */
 	errno = 0;
-	res = cfclose(AH->OF);
 
 	if (res != 0)

 

 	switch (AH->format)
 	{

-	if (fn >= 0)
 
-	if (!AH->OF)
 
-static cfp *
 
 static void
 
 	errno = 0;
-	res = cfclose(AH->OF);
 
 	if (res != 0)

+static bool

 
 	/*
 
index 4725e49747b..18b38c17abc 100644

index 7529367a7b9..b576b299240 100644

 /*
 
 	if (dLen > 0)
 
 /*

 	WriteInt(AH, oid);

 /*
 
 static void
 
 /*
 
 /*
index 6800c3cceef..a2f88995c00 100644

 

 

 
 
 	errno = 0;

 	/* Close the file */
 
 	ctx->dataFH = NULL;
 
 	if (!filename)
 		return;

 
 
 	free(buf);
-	if (cfclose(cfp) != 0)

 

 

 
 	errno = 0;
 
 	return 1;

 /*
 
 	errno = 0;

 	/*

 

 

 

 

 /*

index d92247c915c..78454928cca 100644
index 9a471e40aab..d44ebb06cc2 100644
 
+/*
index e52fe9f5099..db429474a25 100755
 
 	# pg_dump is not C++-clean because it uses "public" and "namespace"
 	# as field names, which is unfortunate but we won't change it now.
+	test "$f" = src/bin/pg_dump/compress_gzip.h && continue
 	test "$f" = src/bin/pg_dump/compress_io.h && continue
+	test "$f" = src/bin/pg_dump/compress_none.h && continue
 	test "$f" = src/bin/pg_dump/parallel.h && continue
 	test "$f" = src/bin/pg_dump/pg_backup_archiver.h && continue
 	test "$f" = src/bin/pg_dump/pg_dump.h && continue
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 22ea42c16b5..d4bb7442bec 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -429,6 +429,7 @@ CompiledExprState
 CompositeIOData
 CompositeTypeStmt
 CompoundAffixFlag
+CompressFileHandle
 CompressionLocation
 CompressorState
 ComputeXidHorizonsResult
@@ -1035,6 +1036,7 @@ GucStack
 GucStackState
 GucStringAssignHook
 GucStringCheckHook
+GzipCompressorState
 HANDLE
 HASHACTION
 HASHBUCKET
-- 
2.39.1

From 3a67132fa394b659c728f2748cd3aad33ff5cdd3 Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Thu, 16 Feb 2023 11:00:35 +0000
Subject: [PATCH 3/3] Add LZ4 compression to pg_dump

XXX The first what? File or streaming API?

XXX How is this related to lz4 utility?

XXX What us LZ4F? Does this rely on another library, not on plain lz4?

Most of the code was written by Georgios Kokolatos. Rachel Heaton provided
invaluable help out with expanding the testing coverage, testing on
different platforms and providing debug information on those, as well as
native speaker wording.

Author: Georgios Kokolatos, Rachel Heaton
Reviewed-by: Michael Paquier, Justin Pryzby, Daniel Gustafsson, Tomas
Vondra
Discussion: https://postgr.es/m/faUNEOpts9vunEaLnmxmG-DldLSg_ql137OC3JYDmgrOMHm1RvvWY2IdBkv_CRxm5spCCb_OmKNk2T03TMm0fBEWveFF9wA1WizPuAgB7Ss%3D%40protonmail.com
---

index 2c938cd7e14..49d218905fb 100644
index 0013bc080cf..eb8f59459a1 100644

index c507eeb7b3d..929b38902b2 100644

 
 	if (!supported)
 
 	return cs;
 }
 
 	return CFH;
 }
 
index 00000000000..ee74cc8e28a
index 00000000000..40dbe00d461
index 84e9f0defa4..0da476a4c34 100644
 
index 2bc5648ed64..027ded4baea 100644
 
 		/*
index a2f88995c00..ce2a0838fa7 100644
 
index 527c7651ab2..08105337b15 100644
 
 	/*
index 78454928cca..72b19ee6cde 100644

index db429474a25..2c5042eb417 100755
index d4bb7442bec..d53cff94fe7 100644
@@ -1387,6 +1387,7 @@ LWLock
2.39.1

---

From: Justin Pryzby <[email-masked]>
Date: Sun, 19 Feb 2023 13:43:45 -0600

Some little updates since I last checked:

=> this comment is obsolete.

s/deffer/infer/ ?
or determine ?
This typo occurs multiple times.

currently this includes only ".gz"
=> remove this phase from the 002 patch (or at least update it in 003).

deferred by iteratively
=> inferred?

s/Requrires/Requires/
twice.

s/occured/occurred/

s/disc/disk/ ?
Probably unimportant, but "disc" isn't used anywhere else.

"compress file handle"
=> maybe these should say "compressed"

supports_compression():
Since this is an exported function, it should probably be called
pgdump_supports_compresion.

---

From: [email-masked]
Date: Mon, 20 Feb 2023 13:09:22 +0000

------- Original Message -------

> maybe coming from an earlier patch version and obsolete.

Thank you very much Tomas! Indeed I have not being paying any attention
at the commit messages.

> update the list of reviewers (I compiled that from substantial reviews

Done. Rachel has been correctly identified as author in the relevant parts
up to commit 98fe74218d. After that, she had offered review comments and I
have taken the liberty to add her as a reviewer through out.

Also I think that Shi Yu should be credited as a reviewer of 0003.

> lines asking about unclear stuff.

Please find in the attached v30 an updated message, as well as an amended
reviewer list. Also v30 addresses the final comments raised by Justin.

Cheers,
//Georgios

> regards

From d34fb1856387c645dd0f172d636fd73ab8837ce4 Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Mon, 20 Feb 2023 12:13:04 +0000
Subject: [PATCH v30 1/3] Prepare pg_dump internals for additional compression
 methods

Commit bf9aa490db introduced a generic compression API in compress_io.{c,h} to
make reuse easier, and allow adding more compression algorithms. However,
pg_backup_archiver.c was not switched to this API and continued to call
the compression directly.

Author: Georgios Kokolatos
Reviewed-by: Michael Paquier, Rachel Heaton, Justin Pryzby, Tomas Vondra
Discussion:
https://postgr.es/m/faUNEOpts9vunEaLnmxmG-DldLSg_ql137OC3JYDmgrOMHm1RvvWY2IdBkv_CRxm5spCCb_OmKNk2T03TMm0fBEWveFF9wA1WizPuAgB7Ss%3D%40protonmail.com
---
 src/bin/pg_dump/compress_io.c        |  98 ++++++++++++++++---
 src/bin/pg_dump/compress_io.h        |   4 +
 src/bin/pg_dump/pg_backup_archiver.c | 138 ++++++++++-----------------
 src/bin/pg_dump/pg_backup_archiver.h |  27 +-----
 4 files changed, 139 insertions(+), 128 deletions(-)

index 7a2c80bbc4..4074cc031c 100644
@@ -56,6 +56,42 @@
 
+#ifdef HAVE_LIBZ
+#include <zlib.h>
+#endif
+
+/*----------------------
+ * Generic functions
+ *----------------------
+ */
+
+/*
+ * Checks wether a compression algorithm is supported.
+ *
+ * On success returns NULL, otherwise returns a malloc'ed string which can be
+ * used by the caller in an error message.
+ */
+char *
+supports_compression(const pg_compress_specification compression_spec)
+{
+	const pg_compress_algorithm	algorithm = compression_spec.algorithm;
+	bool						supported = false;
+
+	if (algorithm == PG_COMPRESSION_NONE)
+		supported = true;
+#ifdef HAVE_LIBZ
+	if (algorithm == PG_COMPRESSION_GZIP)
+		supported = true;
+#endif
+
+	if (!supported)
+		return psprintf("this build does not support compression with %s",
+						get_compress_algorithm_name(algorithm));
+
+	return NULL;
+}
+
+
 /*----------------------
  * Compressor API
  *----------------------
@@ -490,16 +526,19 @@ cfopen_write(const char *path, const char *mode,
 }
 
 /*
 
 	if (compression_spec.algorithm == PG_COMPRESSION_GZIP)
 	{
@@ -511,15 +550,20 @@ cfopen(const char *path, const char *mode,

-		fp->uncompressedfp = NULL;
 		if (fp->compressedfp == NULL)
 		{
 			free_keep_errno(fp);
@@ -531,10 +575,11 @@ cfopen(const char *path, const char *mode,
 	}
 	else
 	{
-#ifdef HAVE_LIBZ
-		fp->compressedfp = NULL;
-#endif
-		fp->uncompressedfp = fopen(path, mode);
+		if (fd >= 0)
+			fp->uncompressedfp = fdopen(fd, mode);
+		else
+			fp->uncompressedfp = fopen(path, mode);
+
 		if (fp->uncompressedfp == NULL)
 		{
 			free_keep_errno(fp);
@@ -545,6 +590,33 @@ cfopen(const char *path, const char *mode,
 	return fp;
 }
 
+/*
 
 int

 

 
 
 	/* Close the output */
 
 	if (res != 0)

 
 	/*

 	if (filename)
 	{

 
 	if (!AH->OF)
 	{
 
-static OutputContext
+static cfp *
 
 static void

 	if (res != 0)

 
 
+	/*

 
 
 	/*
 
-#ifndef HAVE_LIBZ

 #define LOBBUFSIZE 16384
 
-#ifdef HAVE_LIBZ
-#include <zlib.h>

From ad6743d75c3c467f981a6800e7152cf14982e0f4 Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Mon, 20 Feb 2023 12:19:15 +0000
Subject: [PATCH v30 3/3] Add LZ4 compression to pg_dump

This commit expands pg_dump's compression streaming and file APIs to support
PG_COMPRESSION_LZ4. It is implemented in the newly added compress_lz4.{c,h}
files which cover all the functionality of the aforementioned APIs. Minor
changes were also necessary in individual pg_backup_* files, where code for the
'lz4' file suffix has been added, as well as pg_dump's compression option
parsing.

Author: Georgios Kokolatos
Reviewed-by: Michael Paquier, Rachel Heaton, Justin Pryzby, Shi Yu, Tomas Vondra
Discussion:
https://postgr.es/m/faUNEOpts9vunEaLnmxmG-DldLSg_ql137OC3JYDmgrOMHm1RvvWY2IdBkv_CRxm5spCCb_OmKNk2T03TMm0fBEWveFF9wA1WizPuAgB7Ss%3D%40protonmail.com
---
 doc/src/sgml/ref/pg_dump.sgml         |  13 +-
 src/bin/pg_dump/Makefile              |   2 +
 src/bin/pg_dump/compress_io.c         |  26 +-
 src/bin/pg_dump/compress_lz4.c        | 626 ++++++++++++++++++++++++++
 src/bin/pg_dump/compress_lz4.h        |  24 +
 src/bin/pg_dump/meson.build           |   8 +-
 src/bin/pg_dump/pg_backup_archiver.c  |   6 +-
 src/bin/pg_dump/pg_backup_directory.c |   9 +-
 src/bin/pg_dump/pg_dump.c             |   5 +-
 src/bin/pg_dump/t/002_pg_dump.pl      |  82 +++-
 src/tools/pginclude/cpluspluscheck    |   1 +
 src/tools/pgindent/typedefs.list      |   1 +
 12 files changed, 781 insertions(+), 22 deletions(-)
 create mode 100644 src/bin/pg_dump/compress_lz4.c
 create mode 100644 src/bin/pg_dump/compress_lz4.h

 
 override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
@@ -26,6 +27,7 @@ OBJS = \
 	$(WIN32RES) \
 	compress_gzip.o \
 	compress_io.o \
+	compress_lz4.o \
 	compress_none.o \
 	dumputils.o \
 	parallel.o \
diff --git a/src/bin/pg_dump/compress_io.c b/src/bin/pg_dump/compress_io.c
index dae4dc01d5..1ff4aca650 100644
--- a/src/bin/pg_dump/compress_io.c
+++ b/src/bin/pg_dump/compress_io.c
@@ -53,7 +53,7 @@
  *	InitDiscoverCompressFileHandle tries to infer the compression by the
  *	filename suffix. If the suffix is not yet known then it tries to simply
  *	open the file and if it fails, it tries to open the same file with the .gz
- *	suffix.
+ *	suffix, and then again with the .lz4 suffix.
  *
  * IDENTIFICATION
  *	   src/bin/pg_dump/compress_io.c
@@ -67,6 +67,7 @@

@@ -93,6 +94,10 @@ supports_compression(const pg_compress_specification compression_spec)
 	if (algorithm == PG_COMPRESSION_GZIP)
 		supported = true;
 #endif
+#ifdef USE_LZ4
+	if (algorithm == PG_COMPRESSION_LZ4)
+		supported = true;
+#endif
 
 	if (!supported)
@@ -124,6 +129,8 @@ AllocateCompressor(const pg_compress_specification compression_spec,
 
 	return cs;
 }
@@ -188,6 +195,8 @@ InitCompressFileHandle(const pg_compress_specification compression_spec)
 
 	return CFH;
 }
@@ -197,11 +206,11 @@ InitCompressFileHandle(const pg_compress_specification compression_spec)
  * be either "r" or "rb".
  *
  * If the file at 'path' contains the suffix of a supported compression method,
- * currently this includes only ".gz", then this compression will be used
+ * currently this includes ".gz" and ".lz4", then this compression will be used
  * throughout. Otherwise the compression will be inferred by iteratively trying
  * to open the file at 'path', first as is, then by appending known compression
  * suffixes. So if you pass "foo" as 'path', this will open either "foo" or
- * "foo.gz", trying in that order.
+ * "foo.gz" or "foo.lz4", trying in that order.
  *
  * On failure, return NULL with an error code in errno.
  */
@@ -239,6 +248,17 @@ InitDiscoverCompressFileHandle(const char *path, const char *mode)
 			if (exists)
 				compression_spec.algorithm = PG_COMPRESSION_GZIP;
 		}
+#endif
+#ifdef USE_LZ4
+		if (!exists)
+		{
+			free_keep_errno(fname);
+			fname = psprintf("%s.lz4", path);
+			exists = (stat(fname, &st) == 0);
+
+			if (exists)
+				compression_spec.algorithm = PG_COMPRESSION_LZ4;
+		}
 #endif
 	}

 
 		/*

 	/*

index d4bb7442be..d53cff94fe 100644

From 30cd2d5f6f672252215b1e93c68580815cbc8b4c Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Mon, 20 Feb 2023 12:14:56 +0000
Subject: [PATCH v30 2/3] Introduce a generic pg_dump compression API

Author: Georgios Kokolatos
Reviewed-by: Michael Paquier, Rachel Heaton, Justin Pryzby, Tomas Vondra
Discussion:
https://postgr.es/m/faUNEOpts9vunEaLnmxmG-DldLSg_ql137OC3JYDmgrOMHm1RvvWY2IdBkv_CRxm5spCCb_OmKNk2T03TMm0fBEWveFF9wA1WizPuAgB7Ss%3D%40protonmail.com
---
 src/bin/pg_dump/Makefile              |   2 +
 src/bin/pg_dump/compress_gzip.c       | 401 ++++++++++++++
 src/bin/pg_dump/compress_gzip.h       |  24 +
 src/bin/pg_dump/compress_io.c         | 741 +++++---------------------
 src/bin/pg_dump/compress_io.h         | 166 +++++-
 src/bin/pg_dump/compress_none.c       | 206 +++++++
 src/bin/pg_dump/compress_none.h       |  24 +
 src/bin/pg_dump/meson.build           |   2 +
 src/bin/pg_dump/pg_backup_archiver.c  |  91 ++--
 src/bin/pg_dump/pg_backup_archiver.h  |   5 +-
 src/bin/pg_dump/pg_backup_custom.c    |  27 +-
 src/bin/pg_dump/pg_backup_directory.c |  94 ++--
 src/bin/pg_dump/t/002_pg_dump.pl      |  10 +-
 src/include/common/compression.h      |   4 +
 src/tools/pginclude/cpluspluscheck    |   2 +
 src/tools/pgindent/typedefs.list      |   2 +
 16 files changed, 1050 insertions(+), 751 deletions(-)
 create mode 100644 src/bin/pg_dump/compress_gzip.c
 create mode 100644 src/bin/pg_dump/compress_gzip.h
 create mode 100644 src/bin/pg_dump/compress_none.c
 create mode 100644 src/bin/pg_dump/compress_none.h

 OBJS = \
 	$(WIN32RES) \
+	compress_gzip.o \
 	compress_io.o \
+	compress_none.o \
 	dumputils.o \
 	parallel.o \
 	pg_backup_archiver.o \
index 4074cc031c..dae4dc01d5 100644
@@ -9,42 +9,51 @@
+ *	InitDiscoverCompressFileHandle tries to infer the compression by the
+ *	filename suffix. If the suffix is not yet known then it tries to simply
+ *	open the file and if it fails, it tries to open the same file with the .gz
@@ -53,13 +62,14 @@
  */
 #include "postgres_fe.h"
 
+#include <sys/stat.h>
+#include <unistd.h>
+
 
-#ifdef HAVE_LIBZ
-#include <zlib.h>
-#endif
-
 /*----------------------
  * Generic functions
  *----------------------
@@ -97,110 +107,25 @@ supports_compression(const pg_compress_specification compression_spec)
  *----------------------
  */

-#ifndef HAVE_LIBZ
 
-/*
 
-/*
 
 /*
@@ -209,233 +134,31 @@ WriteDataToArchive(ArchiveHandle *AH, CompressorState *cs,
 void
 
-
 /*----------------------
  * Compressed stream API
  *----------------------
  */
 
 /*
 
-#ifdef HAVE_LIBZ
 
 /* free() without changing errno; useful in several places below */
 static void
@@ -448,324 +171,102 @@ free_keep_errno(void *p)
 }
 
 /*
 
 /*

 
-	return fp;
+	return CFH;
 }
 
 /*
+ * throughout. Otherwise the compression will be inferred by iteratively trying
 
-int
 
-	if (size == 0)
-		return 0;
 
-#ifdef HAVE_LIBZ
-	if (fp->compressedfp)
-	{

 
-	return ret;
-}
-
-char *
 
-	return result;
+	return CFH;
 }
 
+/*
 
-const char *
-get_cfp_error(cfp *fp)
-{
-#ifdef HAVE_LIBZ
-	if (fp->compressedfp)
-	{
-		int			errnum;
 
-		if (errnum != Z_ERRNO)
-			return errmsg;
-	}
-#endif
 
-#ifdef HAVE_LIBZ
-static int
index 8beb1058ec..74ba5dda64 100644

 
 /*

 
+/*
+ * Compress File Handle
+ */
+	 * 'a', and 'ab'. Requires an already initialized CompressFileHandle.
+	 * 'mode' can be one of 'w', 'wb', 'a', and 'ab'. Requires an already
 
-typedef struct cfp cfp;
+	/*

+	/*
+	 * Get a pointer to a string that describes an error that occurred during a

 
 	/* Close the output */
 	errno = 0;
-	res = cfclose(AH->OF);
 
 	if (res != 0)

 

 	switch (AH->format)
 	{

-	if (fn >= 0)
 
-	if (!AH->OF)
 
-static cfp *
 
 static void
 
 	errno = 0;
-	res = cfclose(AH->OF);
 
 	if (res != 0)

+static bool

 
 	/*

 

 /*
 
 	if (dLen > 0)
 
 /*

 	WriteInt(AH, oid);

 /*
 
 static void
 
 /*
 
 /*

 

 

 
 
 	errno = 0;

 	/* Close the file */
 
 	ctx->dataFH = NULL;
 
 	if (!filename)
 		return;

 
 
 	free(buf);
-	if (cfclose(cfp) != 0)

 

 

 
 	errno = 0;
 
 	return 1;

 /*
 
 	errno = 0;

 	/*

 

 

 

 

 /*

index 9a471e40aa..b48c173022 100644
 
+/*
+ * These values are stored in disk, for example in files generated by pg_dump.
 
index 22ea42c16b..d4bb7442be 100644

---

From: Tomas Vondra <[email-masked]>
Date: Thu, 23 Feb 2023 16:26:35 +0100

Thanks for v30 with the updated commit messages. I've pushed 0001 after
fixing a comment typo and removing (I think) an unnecessary change in an
error message.

I'll give the buildfarm a bit of time before pushing 0002 and 0003.

regards

---

From: Tomas Vondra <[email-masked]>
Date: Thu, 23 Feb 2023 21:24:46 +0100

> I'll give the buildfarm a bit of time before pushing 0002 and 0003.

I've now pushed 0002 and 0003, after minor tweaks (a couple typos etc.),
and marked the CF entry as committed. Thanks for the patch!

I wonder how difficult would it be to add the zstd compression, so that
we don't have the annoying "unsupported" cases.

regards

---

From: Justin Pryzby <[email-masked]>
Date: Tue, 28 Feb 2023 17:58:34 -0600

> and marked the CF entry as committed. Thanks for the patch!

I found that e9960732a broke writing of empty gzip-compressed data,
specifically LOs.  pg_dump succeeds, but then the restore fails:

postgres=# SELECT lo_create(1234);
lo_create | 1234

$ time ./src/bin/pg_dump/pg_dump -h /tmp -d postgres -Fc |./src/bin/pg_dump/pg_restore -f /dev/null -v 
pg_restore: implied data-only restore
pg_restore: executing BLOB 1234
pg_restore: processing BLOBS
pg_restore: restoring large object with OID 1234
pg_restore: error: could not uncompress data: (null)

The inline patch below fixes it, but you won't be able to apply it
directly, as it's on top of other patches which rename the functions
back to "Zlib" and rearranges the functions to their original order, to
allow running:

git diff --diff-algorithm=minimal -w e9960732a~:./src/bin/pg_dump/compress_io.c ./src/bin/pg_dump/compress_gzip.c

The current function order avoids 3 lines of declarations, but it's
obviously pretty useful to be able to run that diff command.  I already
argued for not calling the functions "Gzip" on the grounds that the name
was inaccurate.

I'd want to create an empty large object in src/test/sql/largeobject.sql
to exercise this tested during pgupgrade.  But unfortunately that
doesn't use -Fc, so this isn't hit.  Empty input is an important enough
test case to justify a tap test, if there's no better way.

diff --git a/src/bin/pg_dump/compress_gzip.c b/src/bin/pg_dump/compress_gzip.c
index f3f5e87c9a8..68f3111b2fe 100644
--- a/src/bin/pg_dump/compress_gzip.c
+++ b/src/bin/pg_dump/compress_gzip.c
@@ -55,6 +55,32 @@ InitCompressorZlib(CompressorState *cs,
 	gzipcs = (ZlibCompressorState *) pg_malloc0(sizeof(ZlibCompressorState));
 
 	cs->private_data = gzipcs;
+
+	if (cs->writeF)
+	{
+		z_streamp	zp;
+		zp = gzipcs->zp = (z_streamp) pg_malloc0(sizeof(z_stream));
+		zp->zalloc = Z_NULL;
+		zp->zfree = Z_NULL;
+		zp->opaque = Z_NULL;
+
+		/*
+		 * outsize is the buffer size we tell zlib it can output to.  We
+		 * actually allocate one extra byte because some routines want to append a
+		 * trailing zero byte to the zlib output.
+		 */
+
+		gzipcs->outbuf = pg_malloc(ZLIB_OUT_SIZE + 1);
+		gzipcs->outsize = ZLIB_OUT_SIZE;
+
+		if (deflateInit(gzipcs->zp, cs->compression_spec.level) != Z_OK)
+			pg_fatal("could not initialize compression library: %s",
+					zp->msg);
+
+		/* Just be paranoid - maybe End is called after Start, with no Write */
+		zp->next_out = gzipcs->outbuf;
+		zp->avail_out = gzipcs->outsize;
+	}
 }
 
 static void
@@ -63,7 +89,7 @@ EndCompressorZlib(ArchiveHandle *AH, CompressorState *cs)
 	ZlibCompressorState *gzipcs = (ZlibCompressorState *) cs->private_data;
 	z_streamp	zp;
 
-	if (gzipcs->zp)
+	if (cs->writeF != NULL)
 	{
 		zp = gzipcs->zp;
 		zp->next_in = NULL;
@@ -131,29 +157,6 @@ WriteDataToArchiveZlib(ArchiveHandle *AH, CompressorState *cs,
 					   const void *data, size_t dLen)
 {
 	ZlibCompressorState *gzipcs = (ZlibCompressorState *) cs->private_data;
-	z_streamp	zp;
-
-	if (!gzipcs->zp)
-	{
-		zp = gzipcs->zp = (z_streamp) pg_malloc(sizeof(z_stream));
-		zp->zalloc = Z_NULL;
-		zp->zfree = Z_NULL;
-		zp->opaque = Z_NULL;
-
-		/*
-		 * outsize is the buffer size we tell zlib it can output to.  We
-		 * actually allocate one extra byte because some routines want to
-		 * append a trailing zero byte to the zlib output.
-		 */
-		gzipcs->outbuf = pg_malloc(ZLIB_OUT_SIZE + 1);
-		gzipcs->outsize = ZLIB_OUT_SIZE;
-
-		if (deflateInit(zp, cs->compression_spec.level) != Z_OK)
-			pg_fatal("could not initialize compression library: %s", zp->msg);
-
-		zp->next_out = gzipcs->outbuf;
-		zp->avail_out = gzipcs->outsize;
-	}
 
 	gzipcs->zp->next_in = (void *) unconstify(void *, data);
 	gzipcs->zp->avail_in = dLen;
>From 1c707279596f3cffde9c97b450dcbef0b6ddbd94 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <[email-masked]>
Date: Sun, 26 Feb 2023 17:12:03 -0600
Subject: [PATCH 1/3] Rename functions/structures/comments which don't use
 "gzip"

zlib's "deflate" functions use a header other than gzip's, so it's
misleading to use names that say gzip.

https://www.postgresql.org/message-id/20221217232615.GS1153%40telsasoft.com
https://www.postgresql.org/message-id/20230127172320.GZ22427%40telsasoft.com
---
 src/bin/pg_dump/compress_gzip.c | 42 ++++++++++++++++-----------------
 src/bin/pg_dump/compress_gzip.h |  2 +-
 src/bin/pg_dump/compress_io.c   |  2 +-
 3 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/src/bin/pg_dump/compress_gzip.c b/src/bin/pg_dump/compress_gzip.c
index 52f41c2e58c..dd769750c8f 100644
--- a/src/bin/pg_dump/compress_gzip.c
+++ b/src/bin/pg_dump/compress_gzip.c
@@ -1,50 +1,50 @@
 /*-------------------------------------------------------------------------
  *
  * compress_gzip.c
- *	 Routines for archivers to read or write a gzip compressed data stream.
+ *	 Routines for archivers to read or write a zlib/gzip compressed data streams.
  *
  * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
  *	   src/bin/pg_dump/compress_gzip.c
  *
  *-------------------------------------------------------------------------
  */
 #include "postgres_fe.h"
 #include <unistd.h>

 #ifdef HAVE_LIBZ
 #include "zlib.h"
 
 /*----------------------
  * Compressor API
  *----------------------
  */
-typedef struct GzipCompressorState
+typedef struct ZlibCompressorState
 {
 	z_streamp	zp;
 
 	void	   *outbuf;
 	size_t		outsize;
-} GzipCompressorState;
+} ZlibCompressorState;
 
-/* Private routines that support gzip compressed data I/O */
+/* Private routines that support zlib compressed data I/O */
 static void
-DeflateCompressorGzip(ArchiveHandle *AH, CompressorState *cs, bool flush)
+DeflateCompressorZlib(ArchiveHandle *AH, CompressorState *cs, bool flush)
 {
-	GzipCompressorState *gzipcs = (GzipCompressorState *) cs->private_data;
+	ZlibCompressorState *gzipcs = (ZlibCompressorState *) cs->private_data;
 	z_streamp	zp = gzipcs->zp;
 	void	   *out = gzipcs->outbuf;
 	int			res = Z_OK;
 
 	while (gzipcs->zp->avail_in != 0 || flush)
 	{
 		res = deflate(zp, flush ? Z_FINISH : Z_NO_FLUSH);
 		if (res == Z_STREAM_ERROR)
 			pg_fatal("could not compress data: %s", zp->msg);
 		if ((flush && (zp->avail_out < gzipcs->outsize))
 			|| (zp->avail_out == 0)
@@ -68,52 +68,52 @@ DeflateCompressorGzip(ArchiveHandle *AH, CompressorState *cs, bool flush)
 			}
 			zp->next_out = out;
 			zp->avail_out = gzipcs->outsize;
 		}
 
 		if (res == Z_STREAM_END)
 			break;
 	}
 }
 
 static void
-EndCompressorGzip(ArchiveHandle *AH, CompressorState *cs)
+EndCompressorZlib(ArchiveHandle *AH, CompressorState *cs)
 {
-	GzipCompressorState *gzipcs = (GzipCompressorState *) cs->private_data;
+	ZlibCompressorState *gzipcs = (ZlibCompressorState *) cs->private_data;
 	z_streamp	zp;
 
 	if (gzipcs->zp)
 	{
 		zp = gzipcs->zp;
 		zp->next_in = NULL;
 		zp->avail_in = 0;
 
 		/* Flush any remaining data from zlib buffer */
-		DeflateCompressorGzip(AH, cs, true);
+		DeflateCompressorZlib(AH, cs, true);
 
 		if (deflateEnd(zp) != Z_OK)
 			pg_fatal("could not close compression stream: %s", zp->msg);
 
 		pg_free(gzipcs->outbuf);
 		pg_free(gzipcs->zp);
 	}
 
 	pg_free(gzipcs);
 	cs->private_data = NULL;
 }
 
 static void
-WriteDataToArchiveGzip(ArchiveHandle *AH, CompressorState *cs,
+WriteDataToArchiveZlib(ArchiveHandle *AH, CompressorState *cs,
 					   const void *data, size_t dLen)
 {
-	GzipCompressorState *gzipcs = (GzipCompressorState *) cs->private_data;
+	ZlibCompressorState *gzipcs = (ZlibCompressorState *) cs->private_data;
 	z_streamp	zp;
 
 	if (!gzipcs->zp)
 	{
 		zp = gzipcs->zp = (z_streamp) pg_malloc(sizeof(z_stream));
 		zp->zalloc = Z_NULL;
 		zp->zfree = Z_NULL;
 		zp->opaque = Z_NULL;
 
 		/*
 		 * outsize is the buffer size we tell zlib it can output to.  We
@@ -124,27 +124,27 @@ WriteDataToArchiveGzip(ArchiveHandle *AH, CompressorState *cs,
 		gzipcs->outsize = ZLIB_OUT_SIZE;
 
 		if (deflateInit(zp, cs->compression_spec.level) != Z_OK)
 			pg_fatal("could not initialize compression library: %s", zp->msg);
 
 		zp->next_out = gzipcs->outbuf;
 		zp->avail_out = gzipcs->outsize;
 	}
 
 	gzipcs->zp->next_in = (void *) unconstify(void *, data);
 	gzipcs->zp->avail_in = dLen;
-	DeflateCompressorGzip(AH, cs, false);
+	DeflateCompressorZlib(AH, cs, false);
 }
 
 static void
-ReadDataFromArchiveGzip(ArchiveHandle *AH, CompressorState *cs)
+ReadDataFromArchiveZlib(ArchiveHandle *AH, CompressorState *cs)
 {
 	z_streamp	zp;
 	char	   *out;
 	int			res = Z_OK;
 	size_t		cnt;
 	char	   *buf;
 	size_t		buflen;
 
 	zp = (z_streamp) pg_malloc(sizeof(z_stream));
 	zp->zalloc = Z_NULL;
 	zp->zfree = Z_NULL;
@@ -193,36 +193,36 @@ ReadDataFromArchiveGzip(ArchiveHandle *AH, CompressorState *cs)
 		ahwrite(out, 1, ZLIB_OUT_SIZE - zp->avail_out, AH);
 	}
 
 	if (inflateEnd(zp) != Z_OK)
 		pg_fatal("could not close compression library: %s", zp->msg);
 
 	free(buf);
 	free(out);
 	free(zp);
 }
 
-/* Public routines that support gzip compressed data I/O */
+/* Public routines that support zlib compressed data I/O */
 void
-InitCompressorGzip(CompressorState *cs,
+InitCompressorZlib(CompressorState *cs,
 				   const pg_compress_specification compression_spec)
 {
-	GzipCompressorState *gzipcs;
+	ZlibCompressorState *gzipcs;
 
-	cs->readData = ReadDataFromArchiveGzip;
-	cs->writeData = WriteDataToArchiveGzip;
-	cs->end = EndCompressorGzip;
+	cs->readData = ReadDataFromArchiveZlib;
+	cs->writeData = WriteDataToArchiveZlib;
+	cs->end = EndCompressorZlib;
 
 	cs->compression_spec = compression_spec;
 
-	gzipcs = (GzipCompressorState *) pg_malloc0(sizeof(GzipCompressorState));
+	gzipcs = (ZlibCompressorState *) pg_malloc0(sizeof(ZlibCompressorState));
 
 	cs->private_data = gzipcs;
 }

 /*----------------------
  * Compress File API
  *----------------------
  */
 
 static size_t
@@ -370,23 +370,23 @@ InitCompressFileHandleGzip(CompressFileHandle *CFH,
 	CFH->getc_func = Gzip_getc;
 	CFH->close_func = Gzip_close;
 	CFH->eof_func = Gzip_eof;
 	CFH->get_error_func = Gzip_get_error;
 
 	CFH->compression_spec = compression_spec;
 
 	CFH->private_data = NULL;
 }
 #else							/* HAVE_LIBZ */
 void
-InitCompressorGzip(CompressorState *cs,
+InitCompressorZlib(CompressorState *cs,
 				   const pg_compress_specification compression_spec)
 {
 	pg_fatal("this build does not support compression with %s", "gzip");
 }
 
 void
 InitCompressFileHandleGzip(CompressFileHandle *CFH,
 						   const pg_compress_specification compression_spec)
 {
 	pg_fatal("this build does not support compression with %s", "gzip");
 }
diff --git a/src/bin/pg_dump/compress_gzip.h b/src/bin/pg_dump/compress_gzip.h
index 2392c697b4c..784a45edaae 100644
--- a/src/bin/pg_dump/compress_gzip.h
+++ b/src/bin/pg_dump/compress_gzip.h
@@ -8,17 +8,17 @@
  *
  * IDENTIFICATION
  *	   src/bin/pg_dump/compress_gzip.h
  *
  *-------------------------------------------------------------------------
  */
 #ifndef _COMPRESS_GZIP_H_
 #define _COMPRESS_GZIP_H_
 
 #include "compress_io.h"
 
-extern void InitCompressorGzip(CompressorState *cs,
+extern void InitCompressorZlib(CompressorState *cs,
 							   const pg_compress_specification compression_spec);
 extern void InitCompressFileHandleGzip(CompressFileHandle *CFH,
 									   const pg_compress_specification compression_spec);
 
 #endif							/* _COMPRESS_GZIP_H_ */
diff --git a/src/bin/pg_dump/compress_io.c b/src/bin/pg_dump/compress_io.c
index ce06f1eac9c..e8277a1e13a 100644
--- a/src/bin/pg_dump/compress_io.c
+++ b/src/bin/pg_dump/compress_io.c
@@ -119,23 +119,23 @@ AllocateCompressor(const pg_compress_specification compression_spec,
 				   ReadFunc readF, WriteFunc writeF)
 {
 	CompressorState *cs;
 
 	cs->readF = readF;
 	cs->writeF = writeF;
 
 	if (compression_spec.algorithm == PG_COMPRESSION_NONE)
 		InitCompressorNone(cs, compression_spec);
 	else if (compression_spec.algorithm == PG_COMPRESSION_GZIP)
-		InitCompressorGzip(cs, compression_spec);
+		InitCompressorZlib(cs, compression_spec);
 	else if (compression_spec.algorithm == PG_COMPRESSION_LZ4)
 		InitCompressorLZ4(cs, compression_spec);
 
 	return cs;
 }
 
 /*
  * Terminate compression library context and flush its buffers.
  */
 void
 EndCompressor(ArchiveHandle *AH, CompressorState *cs)
-- 
2.34.1

>From f8529ab684ab2957a775b6add1e6fc94a4a12476 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <[email-masked]>
Date: Tue, 28 Feb 2023 13:34:06 -0600
Subject: [PATCH 2/3] +also rearrange the functions to their original order..

This allows comparing like:
git diff --diff-algorithm=minimal -w e9960732a~:../src/bin/pg_dump/compress_io.c ../src/bin/pg_dump/compress_gzip.c
---
 src/bin/pg_dump/compress_gzip.c | 95 ++++++++++++++++++---------------
 1 file changed, 51 insertions(+), 44 deletions(-)

diff --git a/src/bin/pg_dump/compress_gzip.c b/src/bin/pg_dump/compress_gzip.c
index dd769750c8f..f3f5e87c9a8 100644
--- a/src/bin/pg_dump/compress_gzip.c
+++ b/src/bin/pg_dump/compress_gzip.c
@@ -24,22 +24,73 @@
  * Compressor API
  *----------------------
  */
 typedef struct ZlibCompressorState
 {
 	z_streamp	zp;
 
 	void	   *outbuf;
 	size_t		outsize;
 } ZlibCompressorState;
 
+static void ReadDataFromArchiveZlib(ArchiveHandle *AH, CompressorState *cs);
+static void WriteDataToArchiveZlib(ArchiveHandle *AH, CompressorState *cs,
+		const void *data, size_t dLen);
+static void EndCompressorZlib(ArchiveHandle *AH, CompressorState *cs);
+static void DeflateCompressorZlib(ArchiveHandle *AH, CompressorState *cs,
+		bool flush);
+
+/* Public routines that support zlib compressed data I/O */
+void
+InitCompressorZlib(CompressorState *cs,
+				   const pg_compress_specification compression_spec)
+{
+	ZlibCompressorState *gzipcs;
+
+	cs->readData = ReadDataFromArchiveZlib;
+	cs->writeData = WriteDataToArchiveZlib;
+	cs->end = EndCompressorZlib;
+
+	cs->compression_spec = compression_spec;
+
+	gzipcs = (ZlibCompressorState *) pg_malloc0(sizeof(ZlibCompressorState));
+
+	cs->private_data = gzipcs;
+}
+
+static void
+EndCompressorZlib(ArchiveHandle *AH, CompressorState *cs)
+{
+	ZlibCompressorState *gzipcs = (ZlibCompressorState *) cs->private_data;
+	z_streamp	zp;
+
+	if (gzipcs->zp)
+	{
+		zp = gzipcs->zp;
+		zp->next_in = NULL;
+		zp->avail_in = 0;
+
+		/* Flush any remaining data from zlib buffer */
+		DeflateCompressorZlib(AH, cs, true);
+
+		if (deflateEnd(zp) != Z_OK)
+			pg_fatal("could not close compression stream: %s", zp->msg);
+
+		pg_free(gzipcs->outbuf);
+		pg_free(gzipcs->zp);
+	}
+
+	pg_free(gzipcs);
+	cs->private_data = NULL;
+}
+
 /* Private routines that support zlib compressed data I/O */
 static void
 DeflateCompressorZlib(ArchiveHandle *AH, CompressorState *cs, bool flush)
 {
 	ZlibCompressorState *gzipcs = (ZlibCompressorState *) cs->private_data;
 	z_streamp	zp = gzipcs->zp;
 	void	   *out = gzipcs->outbuf;
 	int			res = Z_OK;
 
 	while (gzipcs->zp->avail_in != 0 || flush)
 	{
@@ -67,48 +118,22 @@ DeflateCompressorZlib(ArchiveHandle *AH, CompressorState *cs, bool flush)
 				cs->writeF(AH, (char *) out, len);
 			}
 			zp->next_out = out;
 			zp->avail_out = gzipcs->outsize;
 		}
 
 		if (res == Z_STREAM_END)
 			break;
 	}
 }
 
-static void
-EndCompressorZlib(ArchiveHandle *AH, CompressorState *cs)
-{
-	ZlibCompressorState *gzipcs = (ZlibCompressorState *) cs->private_data;
-	z_streamp	zp;
-
-	if (gzipcs->zp)
-	{
-		zp = gzipcs->zp;
-		zp->next_in = NULL;
-		zp->avail_in = 0;
-
-		/* Flush any remaining data from zlib buffer */
-		DeflateCompressorZlib(AH, cs, true);
-
-		if (deflateEnd(zp) != Z_OK)
-			pg_fatal("could not close compression stream: %s", zp->msg);
-
-		pg_free(gzipcs->outbuf);
-		pg_free(gzipcs->zp);
-	}
-
-	pg_free(gzipcs);
-	cs->private_data = NULL;
-}
-
 static void
 WriteDataToArchiveZlib(ArchiveHandle *AH, CompressorState *cs,
 					   const void *data, size_t dLen)
 {
 	ZlibCompressorState *gzipcs = (ZlibCompressorState *) cs->private_data;
 	z_streamp	zp;
 
 	if (!gzipcs->zp)
 	{
 		zp = gzipcs->zp = (z_streamp) pg_malloc(sizeof(z_stream));
 		zp->zalloc = Z_NULL;
@@ -193,40 +218,22 @@ ReadDataFromArchiveZlib(ArchiveHandle *AH, CompressorState *cs)
 		ahwrite(out, 1, ZLIB_OUT_SIZE - zp->avail_out, AH);
 	}
 
 	if (inflateEnd(zp) != Z_OK)
 		pg_fatal("could not close compression library: %s", zp->msg);
 
 	free(buf);
 	free(out);
 	free(zp);
 }
 
-/* Public routines that support zlib compressed data I/O */
-void
-InitCompressorZlib(CompressorState *cs,
-				   const pg_compress_specification compression_spec)
-{
-	ZlibCompressorState *gzipcs;
-
-	cs->readData = ReadDataFromArchiveZlib;
-	cs->writeData = WriteDataToArchiveZlib;
-	cs->end = EndCompressorZlib;
-
-	cs->compression_spec = compression_spec;
-
-	gzipcs = (ZlibCompressorState *) pg_malloc0(sizeof(ZlibCompressorState));
-
-	cs->private_data = gzipcs;
-}
-
 
 /*----------------------
  * Compress File API
  *----------------------
  */
 
 static size_t
 Gzip_read(void *ptr, size_t size, CompressFileHandle *CFH)
 {
 	gzFile		gzfp = (gzFile) CFH->private_data;
 	size_t		ret;
-- 
2.34.1

>From d27585f527365bdb32f032e789ed924d6bb2dca5 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <[email-masked]>
Date: Tue, 28 Feb 2023 17:06:59 -0600
Subject: [PATCH 3/3] pg_dump: call deflateInit() in Init() rather than in
 Write()..

This resolves an issue in e9960732a causing empty LOs to be dumped
incorrectly, resulting in errors during pg_restore.
---
 src/bin/pg_dump/compress_gzip.c | 51 +++++++++++++++++----------------
 1 file changed, 27 insertions(+), 24 deletions(-)

diff --git a/src/bin/pg_dump/compress_gzip.c b/src/bin/pg_dump/compress_gzip.c
index f3f5e87c9a8..68f3111b2fe 100644
--- a/src/bin/pg_dump/compress_gzip.c
+++ b/src/bin/pg_dump/compress_gzip.c
@@ -47,31 +47,57 @@ InitCompressorZlib(CompressorState *cs,
 	ZlibCompressorState *gzipcs;
 
 	cs->readData = ReadDataFromArchiveZlib;
 	cs->writeData = WriteDataToArchiveZlib;
 	cs->end = EndCompressorZlib;
 
 	cs->compression_spec = compression_spec;
 
 	gzipcs = (ZlibCompressorState *) pg_malloc0(sizeof(ZlibCompressorState));
 
 	cs->private_data = gzipcs;
+
+	if (cs->writeF)
+	{
+		z_streamp	zp;
+		zp = gzipcs->zp = (z_streamp) pg_malloc0(sizeof(z_stream));
+		zp->zalloc = Z_NULL;
+		zp->zfree = Z_NULL;
+		zp->opaque = Z_NULL;
+
+		/*
+		 * outsize is the buffer size we tell zlib it can output to.  We
+		 * actually allocate one extra byte because some routines want to append a
+		 * trailing zero byte to the zlib output.
+		 */
+
+		gzipcs->outbuf = pg_malloc(ZLIB_OUT_SIZE + 1);
+		gzipcs->outsize = ZLIB_OUT_SIZE;
+
+		if (deflateInit(gzipcs->zp, cs->compression_spec.level) != Z_OK)
+			pg_fatal("could not initialize compression library: %s",
+					zp->msg);
+
+		/* Just be paranoid - maybe End is called after Start, with no Write */
+		zp->next_out = gzipcs->outbuf;
+		zp->avail_out = gzipcs->outsize;
+	}
 }
 
 static void
 EndCompressorZlib(ArchiveHandle *AH, CompressorState *cs)
 {
 	ZlibCompressorState *gzipcs = (ZlibCompressorState *) cs->private_data;
 	z_streamp	zp;
 
-	if (gzipcs->zp)
+	if (cs->writeF != NULL)
 	{
 		zp = gzipcs->zp;
 		zp->next_in = NULL;
 		zp->avail_in = 0;
 
 		/* Flush any remaining data from zlib buffer */
 		DeflateCompressorZlib(AH, cs, true);
 
 		if (deflateEnd(zp) != Z_OK)
 			pg_fatal("could not close compression stream: %s", zp->msg);
 
@@ -123,45 +149,22 @@ DeflateCompressorZlib(ArchiveHandle *AH, CompressorState *cs, bool flush)
 
 		if (res == Z_STREAM_END)
 			break;
 	}
 }
 
 static void
 WriteDataToArchiveZlib(ArchiveHandle *AH, CompressorState *cs,
 					   const void *data, size_t dLen)
 {
 	ZlibCompressorState *gzipcs = (ZlibCompressorState *) cs->private_data;
-	z_streamp	zp;
-
-	if (!gzipcs->zp)
-	{
-		zp = gzipcs->zp = (z_streamp) pg_malloc(sizeof(z_stream));
-		zp->zalloc = Z_NULL;
-		zp->zfree = Z_NULL;
-		zp->opaque = Z_NULL;
-
-		/*
-		 * outsize is the buffer size we tell zlib it can output to.  We
-		 * actually allocate one extra byte because some routines want to
-		 * append a trailing zero byte to the zlib output.
-		 */
-		gzipcs->outbuf = pg_malloc(ZLIB_OUT_SIZE + 1);
-		gzipcs->outsize = ZLIB_OUT_SIZE;
-
-		if (deflateInit(zp, cs->compression_spec.level) != Z_OK)
-			pg_fatal("could not initialize compression library: %s", zp->msg);
-
-		zp->next_out = gzipcs->outbuf;
-		zp->avail_out = gzipcs->outsize;
-	}
 
 	gzipcs->zp->next_in = (void *) unconstify(void *, data);
 	gzipcs->zp->avail_in = dLen;
 	DeflateCompressorZlib(AH, cs, false);
 }
 
 static void
 ReadDataFromArchiveZlib(ArchiveHandle *AH, CompressorState *cs)
 {
 	z_streamp	zp;
 	char	   *out;
-- 
2.34.1

---

From: Michael Paquier <[email-masked]>
Date: Wed, 1 Mar 2023 16:24:59 +0900

> specifically LOs.  pg_dump succeeds, but then the restore fails:

The number of issues you have been reporting here begins to worries
me..  How many of them have you found?  Is it right to assume that all
of them have basically 03d02f5 as oldest origin point?
--
Michael

---

From: [email-masked]
Date: Wed, 01 Mar 2023 13:39:14 +0000

------- Original Message -------

> pg_restore: error: could not uncompress data: (null)

Thank you for looking. This was an untested case.

> git diff --diff-algorithm=minimal -w e9960732a~:./src/bin/pg_dump/compress_io.c ./src/bin/pg_dump/compress_gzip.c

Please find a patch attached that can be applied directly.

> argued for not calling the functions "Gzip" on the grounds that the name

I have no idea why we are back on the naming issue. I stand by the name
because in my humble opinion helps the code reader. There is a certain
uniformity when the compression_spec.algorithm and the compressor
functions match as the following code sample shows.

When the reader wants to see what happens when the PG_COMPRESSION_XXX
is set, has to simply search for the XXX part. I think that this is
justification enough for the use of the names.

> test case to justify a tap test, if there's no better way.

Please find in the attached a test case that exercises this codepath.

Cheers,
//Georgios

From 95450f0e7e90f0a1a3cdfc12c760a9520bd2995f Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Wed, 1 Mar 2023 12:42:32 +0000
Subject: [PATCH vX] Properly gzip compress when no data is available

When creating dumps with the Compressor API, it is possible to only call the
Allocate and End compressor functions without ever writing any data. The gzip
implementation wrongly assumed that the write function would be called and
defered the initialization of the internal compression system for the first
write call. The End call would only finilize the internal compression system if
that was ever initialized.

The problem with that approach is that it violated the expectations of the
internal compression system during decompression.

This commit initializes the internal compression system during the Allocate
call, under the condition that a write function was provided by the caller.
Given that decompression does not need to keep track of any state, the
compressor's private_data member is now populated only during compression.

Tests are added to cover this scenario.

Initial patch by Justin Pruzby.

Reported-by: Justin Pryzby
---
 src/bin/pg_dump/compress_gzip.c  | 118 +++++++++++++++++--------------
 src/bin/pg_dump/t/002_pg_dump.pl |  27 ++++++-
 2 files changed, 91 insertions(+), 54 deletions(-)

diff --git a/src/bin/pg_dump/compress_gzip.c b/src/bin/pg_dump/compress_gzip.c
index 0af65afeb4..f5d32cf059 100644
--- a/src/bin/pg_dump/compress_gzip.c
+++ b/src/bin/pg_dump/compress_gzip.c
@@ -32,9 +32,48 @@ typedef struct GzipCompressorState
 	size_t		outsize;
 } GzipCompressorState;
 
+
 /* Private routines that support gzip compressed data I/O */
 static void
-DeflateCompressorGzip(ArchiveHandle *AH, CompressorState *cs, bool flush)
+DeflateCompressorInit(CompressorState *cs)
+{
+	GzipCompressorState *gzipcs;
+	z_streamp	zp;
+
+	gzipcs = (GzipCompressorState *) pg_malloc0(sizeof(GzipCompressorState));
+	zp = gzipcs->zp = (z_streamp) pg_malloc(sizeof(z_stream));
+	zp->zalloc = Z_NULL;
+	zp->zfree = Z_NULL;
+	zp->opaque = Z_NULL;
+
+	/*
+	 * outsize is the buffer size we tell zlib it can output to.  We
+	 * actually allocate one extra byte because some routines want to
+	 * append a trailing zero byte to the zlib output.
+	 */
+	gzipcs->outbuf = pg_malloc(ZLIB_OUT_SIZE + 1);
+	gzipcs->outsize = ZLIB_OUT_SIZE;
+
+	/*
+	 * A level of zero simply copies the input one block at the time. This
+	 * is probably not what the user wanted when calling this interface.
+	 */
+	if (cs->compression_spec.level == 0)
+		pg_fatal("requested to compress the archive yet no level was specified");
+
+	if (deflateInit(zp, cs->compression_spec.level) != Z_OK)
+		pg_fatal("could not initialize compression library: %s", zp->msg);
+
+	/* Just be paranoid - maybe End is called after Start, with no Write */
+	zp->next_out = gzipcs->outbuf;
+	zp->avail_out = gzipcs->outsize;
+
+	/* Keep track of gzipcs */
+	cs->private_data = gzipcs;
+}
+
+static void
+DeflateCompressorData(ArchiveHandle *AH, CompressorState *cs, bool flush)
 {
 	GzipCompressorState *gzipcs = (GzipCompressorState *) cs->private_data;
 	z_streamp	zp = gzipcs->zp;
@@ -76,71 +115,44 @@ DeflateCompressorGzip(ArchiveHandle *AH, CompressorState *cs, bool flush)
 }
 
 static void
-EndCompressorGzip(ArchiveHandle *AH, CompressorState *cs)
+DeflateCompressorEnd(ArchiveHandle *AH, CompressorState *cs)
 {
 	GzipCompressorState *gzipcs = (GzipCompressorState *) cs->private_data;
 	z_streamp	zp;
 
-	if (gzipcs->zp)
-	{
-		zp = gzipcs->zp;
-		zp->next_in = NULL;
-		zp->avail_in = 0;
-
 
-		if (deflateEnd(zp) != Z_OK)
-			pg_fatal("could not close compression stream: %s", zp->msg);
+	/* Flush any remaining data from zlib buffer */
+	DeflateCompressorData(AH, cs, true);

 
+static void
+EndCompressorGzip(ArchiveHandle *AH, CompressorState *cs)
+{
+	/* If deflation was initialized, finalize it */
+	if (cs->private_data)
+		DeflateCompressorEnd(AH, cs);
+}
+
 static void
 WriteDataToArchiveGzip(ArchiveHandle *AH, CompressorState *cs,
 					   const void *data, size_t dLen)
 {
 	GzipCompressorState *gzipcs = (GzipCompressorState *) cs->private_data;
-	z_streamp	zp;
-
-	if (!gzipcs->zp)
-	{
-		zp = gzipcs->zp = (z_streamp) pg_malloc(sizeof(z_stream));
-		zp->zalloc = Z_NULL;
-		zp->zfree = Z_NULL;
-		zp->opaque = Z_NULL;
-
-		/*
-		 * outsize is the buffer size we tell zlib it can output to.  We
-		 * actually allocate one extra byte because some routines want to
-		 * append a trailing zero byte to the zlib output.
-		 */
-		gzipcs->outbuf = pg_malloc(ZLIB_OUT_SIZE + 1);
-		gzipcs->outsize = ZLIB_OUT_SIZE;
-
-		/*
-		 * A level of zero simply copies the input one block at the time. This
-		 * is probably not what the user wanted when calling this interface.
-		 */
-		if (cs->compression_spec.level == 0)
-			pg_fatal("requested to compress the archive yet no level was specified");
-
-		if (deflateInit(zp, cs->compression_spec.level) != Z_OK)
-			pg_fatal("could not initialize compression library: %s", zp->msg);
-
-		/* Just be paranoid - maybe End is called after Start, with no Write */
-		zp->next_out = gzipcs->outbuf;
-		zp->avail_out = gzipcs->outsize;
-	}
 
 	gzipcs->zp->next_in = (void *) unconstify(void *, data);
 	gzipcs->zp->avail_in = dLen;
-	DeflateCompressorGzip(AH, cs, false);
+	DeflateCompressorData(AH, cs, false);
 }
 
 static void
@@ -214,17 +226,19 @@ void
 InitCompressorGzip(CompressorState *cs,
 				   const pg_compress_specification compression_spec)
 {
-	GzipCompressorState *gzipcs;
-
 	cs->readData = ReadDataFromArchiveGzip;
 	cs->writeData = WriteDataToArchiveGzip;
 	cs->end = EndCompressorGzip;

-	gzipcs = (GzipCompressorState *) pg_malloc0(sizeof(GzipCompressorState));
-
-	cs->private_data = gzipcs;
+	/*
+	 * If the caller has defined a write function, prepare the necessary state.
+	 * Avoid initializing during the first write call, because End may be called
+	 * without ever writing any data.
+	 */
+	if (cs->writeF)
+		DeflateCompressorInit(cs);
 }

diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 72b19ee6cd..7b5a6e190c 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -1194,6 +1194,29 @@ my %tests = (
 		},
 	},
 
+	'LO create (with no data)' => {
+		create_sql =>
+		  'SELECT pg_catalog.lo_create(0);',
+		regexp => qr/^
+			\QSELECT pg_catalog.lo_open\E \('\d+',\ \d+\);\n
+			\QSELECT pg_catalog.lo_close(0);\E
+			/xm,
+		like   => {
+			%full_runs,
+			column_inserts         => 1,
+			data_only              => 1,
+			inserts                => 1,
+			section_data           => 1,
+			test_schema_plus_large_objects => 1,
+		},
+		unlike => {
+			binary_upgrade         => 1,
+			no_large_objects       => 1,
+			schema_only            => 1,
+			section_pre_data       => 1,
+		},
+	},
+
 	'COMMENT ON DATABASE postgres' => {
 		regexp => qr/^COMMENT ON DATABASE postgres IS .+;/m,
 
@@ -4250,8 +4273,8 @@ foreach my $run (sort keys %pgdump_runs)
 
 	# Skip command-level tests for gzip if there is no support for it.
 	if ($pgdump_runs{$run}->{compile_option} &&
-		($pgdump_runs{$run}->{compile_option} eq 'gzip' && !$supports_gzip) ||
-		($pgdump_runs{$run}->{compile_option} eq 'lz4' && !$supports_lz4))
+		(($pgdump_runs{$run}->{compile_option} eq 'gzip' && !$supports_gzip) ||
+		 ($pgdump_runs{$run}->{compile_option} eq 'lz4' && !$supports_lz4)))
 	{
 		note "$run: skipped due to no $pgdump_runs{$run}->{compile_option} support";
 		next;
-- 
2.34.1

---

From: Tomas Vondra <[email-masked]>
Date: Wed, 1 Mar 2023 15:33:00 +0100

> of them have basically 03d02f5 as oldest origin point?

AFAICS a lot of the issues are more a discussion about wording in a
couple places, whether it's nicer to do A or B, name the functions
differently or what.

I'm aware of three genuine issues that I intend to fix shortly:

1) incorrect "if" condition in a TAP test

2) failure when compressing empty LO (which we had no test for)

3) change in handling "compression level = 0" (which I believe should be
made to behave like before)

regards

---

From: Tomas Vondra <[email-masked]>
Date: Wed, 1 Mar 2023 16:52:49 +0100

> Thank you for looking. This was an untested case.

Yeah :-(

> justification enough for the use of the names.

I don't recall the previous discussion about the naming, but I'm not
sure why would it be inaccurate. We call it 'gzip' pretty much
everywhere, and I agree with Georgios there's it helps to make this
consistent with the PG_COMPRESSION_ stuff.

The one thing that concerned me while reviewing it earlier was that it
might make the backpatcheing harder. But that's mostly irrelevant due to
all the other changes I think.

> Please find in the attached a test case that exercises this codepath.

Thanks. That seems correct to me, but I find it somewhat confusing,
because we now have

 DeflateCompressorInit vs. InitCompressorGzip

 DeflateCompressorEnd vs. EndCompressorGzip

 DeflateCompressorData - The name doesn't really say what it does (would
                         be better to have a verb in there, I think).

I wonder if we can make this somehow clearer?

Also, InitCompressorGzip says this:

   /*
    * If the caller has defined a write function, prepare the necessary
    * state. Avoid initializing during the first write call, because End
    * may be called without ever writing any data.
    */
    if (cs->writeF)
        DeflateCompressorInit(cs);

Does it actually make sense to not have writeF defined in some cases?

regards

---

From: Justin Pryzby <[email-masked]>
Date: Tue, 7 Mar 2023 19:45:20 -0600

> I wonder if we can make this somehow clearer?

To move things along, I updated Georgios' patch:

Rename DeflateCompressorData() to DeflateCompressorCommon();
Rearrange functions to their original order allowing a cleaner diff to the prior code;
Change pg_fatal() to an assertion+comment;
Update the commit message and fix a few typos;

> Does it actually make sense to not have writeF defined in some cases?

InitCompressor is being called for either reading or writing, either of
which could be null:

src/bin/pg_dump/pg_backup_custom.c:     ctx->cs = AllocateCompressor(AH->compression_spec,
src/bin/pg_dump/pg_backup_custom.c-                                                              NULL,
src/bin/pg_dump/pg_backup_custom.c-                                                              _CustomWriteFunc);
--
src/bin/pg_dump/pg_backup_custom.c:     cs = AllocateCompressor(AH->compression_spec,
src/bin/pg_dump/pg_backup_custom.c-                                                     _CustomReadFunc, NULL);

It's confusing that the comment says "Avoid initializing...".  What it
really means is "Initialize eagerly...".  But that makes more sense in
the context of the commit message for this bugfix than in a comment.  So
I changed that too.

Maybe it'd be more clear if this used "if (cs->writeF)", like in the
init function ?

-- 
Justin
>From 5c027aa86e8591db140093c48a58aafba7a6c28c Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Wed, 1 Mar 2023 12:42:32 +0000
Subject: [PATCH] pg_dump: fix gzip compression of empty data

When creating dumps with the Compressor API, it is possible to only call
the Allocate and End compressor functions without ever writing any data.
Since e9960732a, the gzip implementation wrongly assumed that the write
function would always be called and deferred the initialization of the
internal compression system until the first write call.

The problem with that approach is that the End call would not finalize
the internal compression system if it hadn't been initialized.

This commit initializes the internal compression system during the
Allocate call, whenever a write function was provided by the caller.
Given that decompression does not need to keep track of any state, the
compressor's private_data member is now populated only during
compression.

In passing, rearrange the functions to their original order, to allow
usefully comparing with the previous code, like:
git diff --diff-algorithm=minimal -w e9960732a~:src/bin/pg_dump/compress_io.c src/bin/pg_dump/compress_gzip.c

Also replace an unreachable pg_fatal() with an assert+comment.  I
(Justin) argued that the new fatal shouldn't have been introduced in a
refactoring commit, so this is a compromise.

Report and initial patch by Justin Pryzby, test case by Georgios
Kokolatos.

https://www.postgresql.org/message-id/20230228235834.GC30529%40telsasoft.com
---
 src/bin/pg_dump/compress_gzip.c  | 137 ++++++++++++++++++-------------
 src/bin/pg_dump/t/002_pg_dump.pl |  23 ++++++
 2 files changed, 101 insertions(+), 59 deletions(-)

diff --git a/src/bin/pg_dump/compress_gzip.c b/src/bin/pg_dump/compress_gzip.c
index 0af65afeb4e..3c9ac55c266 100644
--- a/src/bin/pg_dump/compress_gzip.c
+++ b/src/bin/pg_dump/compress_gzip.c
@@ -32,9 +32,75 @@ typedef struct GzipCompressorState
 	size_t		outsize;
 } GzipCompressorState;
 
+
 /* Private routines that support gzip compressed data I/O */
+static void DeflateCompressorInit(CompressorState *cs);
+static void DeflateCompressorEnd(ArchiveHandle *AH, CompressorState *cs);
+static void DeflateCompressorCommon(ArchiveHandle *AH, CompressorState *cs,
+									bool flush);
+static void EndCompressorGzip(ArchiveHandle *AH, CompressorState *cs);
+static void WriteDataToArchiveGzip(ArchiveHandle *AH, CompressorState *cs,
+								   const void *data, size_t dLen);
+static void ReadDataFromArchiveGzip(ArchiveHandle *AH, CompressorState *cs);
+
 static void
-DeflateCompressorGzip(ArchiveHandle *AH, CompressorState *cs, bool flush)
+DeflateCompressorInit(CompressorState *cs)
+{
+	GzipCompressorState *gzipcs;
+	z_streamp	zp;
+
+	gzipcs = (GzipCompressorState *) pg_malloc0(sizeof(GzipCompressorState));
+	zp = gzipcs->zp = (z_streamp) pg_malloc(sizeof(z_stream));
+	zp->zalloc = Z_NULL;
+	zp->zfree = Z_NULL;
+	zp->opaque = Z_NULL;
+
+	/*
+	 * outsize is the buffer size we tell zlib it can output to.  We actually
+	 * allocate one extra byte because some routines want to append a trailing
+	 * zero byte to the zlib output.
+	 */
+	gzipcs->outbuf = pg_malloc(ZLIB_OUT_SIZE + 1);
+	gzipcs->outsize = ZLIB_OUT_SIZE;
+
+	/* -Z 0 uses the "None" compressor -- not zlib with no compression */
+	Assert(cs->compression_spec.level != 0);
+
+	if (deflateInit(zp, cs->compression_spec.level) != Z_OK)
+		pg_fatal("could not initialize compression library: %s", zp->msg);
+
+	/* Just be paranoid - maybe End is called after Start, with no Write */
+	zp->next_out = gzipcs->outbuf;
+	zp->avail_out = gzipcs->outsize;
+
+	/* Keep track of gzipcs */
+	cs->private_data = gzipcs;
+}
+
+static void
+DeflateCompressorEnd(ArchiveHandle *AH, CompressorState *cs)
+{
+	GzipCompressorState *gzipcs = (GzipCompressorState *) cs->private_data;
+	z_streamp	zp;
+
+	zp = gzipcs->zp;
+	zp->next_in = NULL;
+	zp->avail_in = 0;
+
+	/* Flush any remaining data from zlib buffer */
+	DeflateCompressorCommon(AH, cs, true);
+
+	if (deflateEnd(zp) != Z_OK)
+		pg_fatal("could not close compression stream: %s", zp->msg);
+
+	pg_free(gzipcs->outbuf);
+	pg_free(gzipcs->zp);
+	pg_free(gzipcs);
+	cs->private_data = NULL;
+}
+
+static void
+DeflateCompressorCommon(ArchiveHandle *AH, CompressorState *cs, bool flush)
 {
 	GzipCompressorState *gzipcs = (GzipCompressorState *) cs->private_data;
 	z_streamp	zp = gzipcs->zp;
@@ -78,27 +144,9 @@ DeflateCompressorGzip(ArchiveHandle *AH, CompressorState *cs, bool flush)
 static void
 EndCompressorGzip(ArchiveHandle *AH, CompressorState *cs)
 {
-	GzipCompressorState *gzipcs = (GzipCompressorState *) cs->private_data;
-	z_streamp	zp;
-
-	if (gzipcs->zp)
-	{
-		zp = gzipcs->zp;
-		zp->next_in = NULL;
-		zp->avail_in = 0;
-
-		/* Flush any remaining data from zlib buffer */
-		DeflateCompressorGzip(AH, cs, true);
-
-		if (deflateEnd(zp) != Z_OK)
-			pg_fatal("could not close compression stream: %s", zp->msg);
-
-		pg_free(gzipcs->outbuf);
-		pg_free(gzipcs->zp);
-	}
-
-	pg_free(gzipcs);
-	cs->private_data = NULL;
+	/* If deflation was initialized, finalize it */
+	if (cs->private_data)
+		DeflateCompressorEnd(AH, cs);
 }
 
 static void
@@ -106,41 +154,10 @@ WriteDataToArchiveGzip(ArchiveHandle *AH, CompressorState *cs,
 
 	gzipcs->zp->next_in = (void *) unconstify(void *, data);
 	gzipcs->zp->avail_in = dLen;
-	DeflateCompressorGzip(AH, cs, false);
+	DeflateCompressorCommon(AH, cs, false);
 }
 
 static void
@@ -214,17 +231,19 @@ void

-	gzipcs = (GzipCompressorState *) pg_malloc0(sizeof(GzipCompressorState));
-
-	cs->private_data = gzipcs;
+	/*
+	 * If the caller has defined a write function, prepare the necessary
+	 * state.  Note that if the data is empty, End may be called immediately
+	 * after Init, without ever calling Write.
+	 */
+	if (cs->writeF)
+		DeflateCompressorInit(cs);
 }

index 187e4b8d07d..14cd0d2d503 100644

-- 
2.34.1

---

From: Alexander Lakhin <[email-masked]>
Date: Sat, 11 Mar 2023 09:00:00 +0300

Hello,
> we don't have the annoying "unsupported" cases.

With the patch 0003 committed, a single warning -Wtype-limits appeared in the
master branch:
$ CPPFLAGS="-Og -Wtype-limits" ./configure --with-lz4 -q && make -s -j8
compress_lz4.c: In function ‘LZ4File_gets’:
compress_lz4.c:492:19: warning: comparison of unsigned expression in ‘< 0’ is 
always false [-Wtype-limits]
   492 |         if (dsize < 0)
       |
(I wonder, is it accidental that there no other places that triggers
the warning, or some buildfarm animals had this check enabled before?)

It is not a false positive as can be proved by the 002_pg_dump.pl modified as
follows:
-                       program => $ENV{'LZ4'},
+                       program => 'mv',
                         args    => [
-                               '-z', '-f', '--rm',
"$tempdir/compression_lz4_dir/blobs.toc",
"$tempdir/compression_lz4_dir/blobs.toc.lz4",
                         ],
                 },
A diagnostic logging added shows:
LZ4File_gets() after LZ4File_read_internal; dsize: 18446744073709551615

and pg_restore fails with:
error: invalid line in large object TOC file 
".../src/bin/pg_dump/tmp_check/tmp_test_22ri/compression_lz4_dir/blobs.toc": "????"

Best regards,
Alexander

---

From: [email-masked]
Date: Sat, 11 Mar 2023 10:50:06 +0000

------- Original Message -------

> always false [-Wtype-limits]

Thank you Alexander. Please find attached an attempt to address it.

> the warning, or some buildfarm animals had this check enabled before?)

I can not answer about the buildfarms. Do you think that adding an explicit
check for this warning in meson would help? I am a bit uncertain as I think
that type-limits are included in extra.

@@ -1748,6 +1748,7 @@ common_warning_flags = [
   '-Wshadow=compatible-local',
   # This was included in -Wall/-Wformat in older GCC versions
   '-Wformat-security',
+  '-Wtype-limits',
 ]

> "$tempdir/compression_lz4_dir/blobs.toc.lz4",

Correct, it is not a false positive. The existing testing framework provides
limited support for exercising error branches. Especially so when those are
dependent on generated output. 

> ".../src/bin/pg_dump/tmp_check/tmp_test_22ri/compression_lz4_dir/blobs.toc": "????"

It is a good thing that the restore fails with bad input. Yet it should
have failed earlier. The attached makes certain it does fail earlier. 

Cheers,
//Georgios

> Alexander

From b80bb52ef6546aee8c8431d7cc126fa4a76b543c Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <[email-masked]>
Date: Sat, 11 Mar 2023 09:54:40 +0000
Subject: [PATCH v1] Respect return type of LZ4File_read_internal

The function LZ4File_gets() was storing the return value of
LZ4File_read_internal in a variable of the wrong type, disregarding sign-es.
As a consequence, LZ4File_gets() would not take the error path when it should.

In an attempt to improve readability, spell out the significance of a negative
return value of LZ4File_read_internal() in LZ4File_read().

Reported-by: Alexander Lakhin <[email-masked]>
---
 src/bin/pg_dump/compress_lz4.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/bin/pg_dump/compress_lz4.c b/src/bin/pg_dump/compress_lz4.c
index 63e794cdc6..cc039f0b47 100644
--- a/src/bin/pg_dump/compress_lz4.c
+++ b/src/bin/pg_dump/compress_lz4.c
@@ -453,7 +453,7 @@ LZ4File_read(void *ptr, size_t size, CompressFileHandle *CFH)
 	int			ret;
 
 	ret = LZ4File_read_internal(fs, ptr, size, false);
-	if (ret != size && !LZ4File_eof(CFH))
+	if (ret < 0 || (ret != size && !LZ4File_eof(CFH)))
 		pg_fatal("could not read from input file: %s", LZ4File_get_error(CFH));
 
 	return ret;
@@ -486,14 +486,14 @@ static char *
 LZ4File_gets(char *ptr, int size, CompressFileHandle *CFH)
 {
 	LZ4File    *fs = (LZ4File *) CFH->private_data;
-	size_t		dsize;
+	int			ret;
 
-	dsize = LZ4File_read_internal(fs, ptr, size, true);
-	if (dsize < 0)
+	ret = LZ4File_read_internal(fs, ptr, size, true);
+	if (ret < 0)
 		pg_fatal("could not read from input file: %s", LZ4File_get_error(CFH));
 
 	/* Done reading */
-	if (dsize == 0)
+	if (ret == 0)
 		return NULL;
 
 	return ptr;
-- 
2.34.1

---

From: Alexander Lakhin <[email-masked]>
Date: Sat, 11 Mar 2023 21:00:00 +0300

Hi Georgios,

>     # This was included in -Wall/-Wformat in older GCC versions
I'm not sure that I can promote additional checks (or determine where
to put them), but if some patch introduces a warning of a type that wasn't
present before, I think it's worth to eliminate the warning (if it is
sensible) to keep the source code check baseline at the same level
or even lift it up gradually.
I've also found that the same commit introduced a single instance of
the analyzer-possible-null-argument warning:
CPPFLAGS="-Og -fanalyzer -Wno-analyzer-malloc-leak -Wno-analyzer-file-leak 
-Wno-analyzer-null-dereference -Wno-analyzer-shift-count-overflow 
-Wno-analyzer-free-of-non-heap -Wno-analyzer-null-argument 
-Wno-analyzer-double-free -Wanalyzer-possible-null-argument" ./configure 
--with-lz4 -q && make -s -j8
compress_io.c: In function ‘hasSuffix’:
compress_io.c:158:47: warning: use of possibly-NULL ‘filename’ where non-null 
expected [CWE-690] [-Wanalyzer-possible-null-argument]
   158 |         int                     filenamelen = strlen(filename);
       | ^~~~~~~~~~~~~~~~
   ‘InitDiscoverCompressFileHandle’: events 1-3
...

(I use gcc-11.3.)
As I can see, many existing uses of strdup() are followed by a check for
null result, so maybe it's a common practice and a similar check should
be added in InitDiscoverCompressFileHandle().
(There also a couple of other warnings introduced with the lz4 compression
patches, but those ones are not unique, so I maybe they aren't worth fixing.)

>> have failed earlier. The attached makes certain it does fail earlier.
Thanks! Your patch definitely fixes the issue.

Best regards,
Alexander

---

From: Peter Eisentraut <[email-masked]>
Date: Sun, 12 Mar 2023 11:07:33 +0100

On 11.03.23 07:00, Alexander Lakhin wrote:
> compress_lz4.c:492:19: warning: comparison of unsigned expression in ‘< 
> 0’ is always false [-Wtype-limits]
>    492 |         if (dsize < 0)
>        |

I think there is an underlying problem in this code that it dances back 
and forth between size_t and int in an unprincipled way.

In the code that triggers the warning, dsize is size_t.  dsize is the 
return from LZ4File_read_internal(), which is declared to return int. 
The variable that LZ4File_read_internal() returns in the success case is 
size_t, but in case of an error it returns -1.  (So the code that is 
warning is meaning to catch this error case, but it won't ever work.) 
Further below LZ4File_read_internal() calls LZ4File_read_overflow(), 
which is declared to return int, but in some cases it returns 
fs->overflowlen, which is size_t.

This should be cleaned up.

AFAICT, the upstream API in lz4.h uses int for size values, but 
lz4frame.h uses size_t, so I don't know what the correct approach is.

---

From: Tomas Vondra <[email-masked]>
Date: Sun, 12 Mar 2023 19:34:02 +0100

On 3/12/23 11:07, Peter Eisentraut wrote:
>> compress_lz4.c:492:19: warning: comparison of unsigned expression in
>> ‘< 0’ is always false [-Wtype-limits]
>>    492 |         if (dsize < 0)
>>        |

I agree. I just got home so I looked at this only very briefly, but I
think it's clearly wrong to assign the LZ4File_read_internal() result to
a size_t variable (and it seems to me LZ4File_gets does the same mistake
with LZ4File_read_internal() result).

I'll get this fixed early next week, I'm too tired to do that now
without likely causing further issues.

> lz4frame.h uses size_t, so I don't know what the correct approach is.

Yeah, that's a good point. I think Justin is right we should be using
the LZ4F stuff, so ultimately we'll probably switch to size_t. But IMO
it's definitely better to correct the current code first, and only then
switch to LZ4F (from one correct state to another).

regards

---

From: Tomas Vondra <[email-masked]>
Date: Mon, 13 Mar 2023 22:47:12 +0100

Hi Justin,

Thanks for the patch.

> Rename DeflateCompressorData() to DeflateCompressorCommon();

Hmmm, I don't find "common" any clearer than "data" :-( There needs to
at least be a comment explaining what "common" does.

> Rearrange functions to their original order allowing a cleaner diff to the prior code;

OK. I wasn't very enthusiastic about this initially, but after thinking
about it a bit I think it's meaningful to make diffs clearer. But I
don't see much difference with/without the patch. The

git diff --diff-algorithm=minimal -w
e9960732a~:src/bin/pg_dump/compress_io.c src/bin/pg_dump/compress_gzip.c

Produces ~25k diff with/without the patch. What am I doing wrong?

> Change pg_fatal() to an assertion+comment;

Yeah, that's reasonable. I'd even ditch the assert/comment, TBH. We
could add such protections against "impossible" stuff to a zillion other
places and the confusion likely outweighs the benefits.

> Update the commit message and fix a few typos;

Thanks. I don't want to annoy you too much, but could you split the
patch into the "empty-data" fix and all the other changes (rearranging
functions etc.)? I'd rather not mix those in the same commit.

> src/bin/pg_dump/pg_backup_custom.c-                                                              _CustomWriteFunc);

Yeah, if the two checks are equivalent, it'd be better to stick to the
same check everywhere.

regards

---

From: [email-masked]
Date: Tue, 14 Mar 2023 11:07:25 +0000

------- Original Message -------

> places and the confusion likely outweighs the benefits.

A minor note to add is to not ignore the lessons learned from a7885c9bb.

For example, as the testing framework stands, one can not test that the
contents of the custom format are indeed compressed. One can infer it by
examining the header of the produced dump and searching for the
compression flag. The code responsible for writing the header and the
code responsible for actually dealing with data, is not the same. Also,
the compression library itself will happily read and write uncompressed
data.

A pg_fatal, assertion, or similar, is the only guard rail against this
kind of error. Without it, the tests will continue passing even after
e.g. a wrong initialization of the API. It was such a case that lead to
a7885c9bb in the first place. I do think that we wish it to be an
"impossible" case. Also it will be an untested case with some history
without such a guard rail.

Of course I will not object to removing it, if you think that is more
confusing than useful.

Cheers,
//Georgios

---

From: Tomas Vondra <[email-masked]>
Date: Tue, 14 Mar 2023 16:52:20 +0100

> without such a guard rail.

So is the pg_fatal() a dead code or not? My understanding was it's not
really reachable, and the main purpose is to remind people this is not
possible. Or am I mistaken/confused?

If it's reachable, can we test it? AFAICS we don't, per the coverage
reports.

If it's just a protection against incorrect API initialization, then an
assert is the right solution, I think. With proper comment. But can't we
actually verify that *during* the initialization?

Also, how come WriteDataToArchiveLZ4() doesn't need this protection too?
Or is that due to gzip being the default compression method?

> Of course I will not object to removing it, if you think that is more

Not sure, I have a feeling I don't quite understand in what situation
this actually helps.

regards