------- Original Message -------
On Monday, May 8th, 2023 at 3:16 AM, Tom Lane <t...@sss.pgh.pa.us> wrote:


> 
> 
> I wrote:
> 
> > Michael Paquier mich...@paquier.xyz writes:
> > 
> > > While testing this patch, I have triggered an error pointing out that
> > > the decompression path of LZ4 is broken for table data. I can
> > > reproduce that with a dump of the regression database, as of:
> > > make installcheck
> > > pg_dump --format=d --file=dump_lz4 --compress=lz4 regression
> 
> > Ugh. Reproduced here ... so we need an open item for this.
> 
> 
> BTW, it seems to work with --format=c.
> 

Thank you for the extra tests. It seems that exists a gap in the test
coverage. Please find a patch attached that is addressing the issue
and attempt to provide tests for it.

Cheers,
//Georgios

> regards, tom lane
From 8c6c86c362820e93f066992ede6e5ca23f128807 Mon Sep 17 00:00:00 2001
From: Georgios Kokolatos <gkokola...@pm.me>
Date: Mon, 8 May 2023 15:25:25 +0000
Subject: [PATCH v1] Advance input pointer when LZ4 compressing data

LZ4File_write() did not advance the input pointer on subsequent invocations of
LZ4F_compressUpdate(). As a result the generated compressed output would be a
compressed version of the same input chunk.

WriteDataToArchiveLZ4() which is also using LZ4F_compressUpdate() did not suffer
from this omission. Tests failed to catch this error because all of their input
would comfortably fit within the same input chunk. Tests have been added to
provide adequate coverage.
---
 src/bin/pg_dump/compress_lz4.c   |  5 +++-
 src/bin/pg_dump/t/002_pg_dump.pl | 44 ++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/src/bin/pg_dump/compress_lz4.c b/src/bin/pg_dump/compress_lz4.c
index f97b7550d1..76211c82f0 100644
--- a/src/bin/pg_dump/compress_lz4.c
+++ b/src/bin/pg_dump/compress_lz4.c
@@ -564,6 +564,7 @@ LZ4Stream_write(const void *ptr, size_t size, CompressFileHandle *CFH)
 	LZ4State   *state = (LZ4State *) CFH->private_data;
 	size_t		status;
 	int			remaining = size;
+	const void *in = ptr;
 
 	/* Lazy init */
 	if (!LZ4Stream_init(state, size, true))
@@ -576,7 +577,7 @@ LZ4Stream_write(const void *ptr, size_t size, CompressFileHandle *CFH)
 		remaining -= chunk;
 
 		status = LZ4F_compressUpdate(state->ctx, state->buffer, state->buflen,
-									 ptr, chunk, NULL);
+									 in, chunk, NULL);
 		if (LZ4F_isError(status))
 		{
 			state->errcode = status;
@@ -588,6 +589,8 @@ LZ4Stream_write(const void *ptr, size_t size, CompressFileHandle *CFH)
 			errno = (errno) ? errno : ENOSPC;
 			return false;
 		}
+
+		in = ((const char *) in) + chunk;
 	}
 
 	return true;
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 93e24d5145..c6b1225815 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -3108,6 +3108,50 @@ my %tests = (
 		},
 	},
 
+	'CREATE TABLE test_compression_method' => {
+		create_order => 110,
+		create_sql   => 'CREATE TABLE dump_test.test_compression_method (
+						   col1 text
+					   );',
+		regexp => qr/^
+			\QCREATE TABLE dump_test.test_compression_method (\E\n
+			\s+\Qcol1 text\E\n
+			\Q);\E
+			/xm,
+		like => {
+			%full_runs,
+			%dump_test_schema_runs,
+			section_pre_data     => 1,
+		},
+		unlike => {
+			exclude_dump_test_schema => 1,
+			only_dump_measurement    => 1,
+		},
+	},
+
+	'COPY test_compression_method' => {
+		create_order => 111,
+		create_sql   => 'INSERT INTO dump_test.test_compression_method (col1) '
+		  . 'SELECT string_agg(a::text, \'\') FROM generate_series(1,4096) a;',
+		regexp => qr/^
+			\QCOPY dump_test.test_compression_method (col1) FROM stdin;\E
+			\n(?:\d{15277}\n){1}\\\.\n
+			/xm,
+		like => {
+			%full_runs,
+			data_only                => 1,
+			section_data             => 1,
+			only_dump_test_schema    => 1,
+			test_schema_plus_large_objects    => 1,
+		},
+		unlike => {
+			binary_upgrade           => 1,
+			exclude_dump_test_schema => 1,
+			schema_only              => 1,
+			only_dump_measurement    => 1,
+		},
+	},
+
 	'CREATE TABLE fk_reference_test_table' => {
 		create_order => 21,
 		create_sql   => 'CREATE TABLE dump_test.fk_reference_test_table (
-- 
2.34.1

Reply via email to