v13-0004-Add-tests-for-pipe.patch

application/x-patch

Filename: v13-0004-Add-tests-for-pipe.patch
Type: application/x-patch
Part: 2
Message: Re: Adding pg_dump flag for parallel export to pipes

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: format-patch
Series: patch v13-0004
Subject: Add tests for pipe
File+
src/bin/pg_dump/t/001_basic.pl 70 2
src/bin/pg_dump/t/002_pg_dump.pl 243 0
src/bin/pg_dump/t/004_pg_dump_parallel.pl 34 0
src/bin/pg_dump/t/005_pg_dump_filterfile.pl 18 0
From 43918db8e304e1492298c20dde6cc13153b827c9 Mon Sep 17 00:00:00 2001
From: Nitin Motiani <nitinmotiani@google.com>
Date: Sat, 15 Feb 2025 04:29:17 +0000
Subject: [PATCH v13 4/5] Add tests for pipe

* These tests include the invalid usages of --pipe-command with other flags.

* Also test pg_dump and pg_restore with pipe command along with various other flags.
---
 src/bin/pg_dump/t/001_basic.pl              |  72 +++++-
 src/bin/pg_dump/t/002_pg_dump.pl            | 243 ++++++++++++++++++++
 src/bin/pg_dump/t/004_pg_dump_parallel.pl   |  34 +++
 src/bin/pg_dump/t/005_pg_dump_filterfile.pl |  18 ++
 4 files changed, 365 insertions(+), 2 deletions(-)

diff --git a/src/bin/pg_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl
index 509f4f9ce7d..878ebf33271 100644
--- a/src/bin/pg_dump/t/001_basic.pl
+++ b/src/bin/pg_dump/t/001_basic.pl
@@ -74,6 +74,48 @@ command_fails_like(
 	'pg_dump: options --statistics-only and --no-statistics cannot be used together'
 );
 
+command_fails_like(
+	[ 'pg_dump', '-Fd', '--pipe="cat"', '-f', 'testdir', 'test'],
+	qr/\Qpg_dump: error: options -f\/--file and --pipe cannot be used together\E/,
+	'pg_dump: options -f/--file and --pipe cannot be used together'
+);
+
+command_fails_like(
+	[ 'pg_dump', '-Fd', '--pipe="cat"', '-Z', 'gzip', 'test'],
+	qr/\Qpg_dump: error: option --pipe is not supported with any compression type\E/,
+	'pg_dump: option --pipe is not supported with any compression type'
+);
+
+command_fails_like(
+	[ 'pg_dump', '-Fd', '--pipe="cat"', '--compress=lz4', 'test'],
+	qr/\Qpg_dump: error: option --pipe is not supported with any compression type\E/,
+	'pg_dump: option --pipe is not supported with any compression type'
+);
+
+command_fails_like(
+	[ 'pg_dump', '-Fd', '--pipe="cat"', '--compress=gzip', 'test'],
+	qr/\Qpg_dump: error: option --pipe is not supported with any compression type\E/,
+	'pg_dump: option --pipe is not supported with any compression type'
+);
+
+command_fails_like(
+	[ 'pg_dump', '-Fd', '--pipe="cat"', '-Z', '1', 'test'],
+	qr/\Qpg_dump: error: option --pipe is not supported with any compression type\E/,
+	'pg_dump: option --pipe is not supported with any compression type'
+);
+
+command_fails_like(
+	[ 'pg_dump', '-Fc', '--pipe="cat"', 'test'],
+	qr/\Qpg_dump: error: option --pipe is only supported with directory format\E/,
+	'pg_dump: option --pipe is only supported with directory format'
+);
+
+command_fails_like(
+	[ 'pg_dump', '--format=tar', '--pipe="cat"', 'test'],
+	qr/\Qpg_dump: error: option --pipe is only supported with directory format\E/,
+	'pg_dump: option --pipe is only supported with directory format'
+);
+
 command_fails_like(
 	[ 'pg_dump', '-j2', '--include-foreign-data=xxx' ],
 	qr/\Qpg_dump: error: option --include-foreign-data is not supported with parallel backup\E/,
@@ -94,12 +136,38 @@ command_fails_like(
 command_fails_like(
 	[ 'pg_restore', '-d', 'xxx', '-f', 'xxx' ],
 	qr/\Qpg_restore: error: options -d\/--dbname and -f\/--file cannot be used together\E/,
-	'pg_restore: options -d/--dbname and -f/--file cannot be used together');
+	'pg_restore: options -d/--dbname and -f/--file cannot be used together'
+);
+
+command_fails_like(
+	[ 'pg_restore', '-f', '-', '--pipe="cat"', 'dumpdir' ],
+	qr/\Qpg_restore: error: cannot specify both an input file and --pipe\E/,
+	'pg_restore: cannot specify both an input file and --pipe'
+);
+
+command_fails_like(
+	[ 'pg_restore', '-Fd', '-f', '-', '--pipe="cat"', 'dumpdir' ],
+	qr/\Qpg_restore: error: cannot specify both an input file and --pipe\E/,
+	'pg_restore: cannot specify both an input file and --pipe'
+);
+
+command_fails_like(
+	[ 'pg_restore', '-Fc', '-f', '-', '--pipe="cat"' ],
+	qr/\Qpg_restore: error: option --pipe is only supported with directory format\E/,
+	'pg_restore: option --pipe is only supported with directory format'
+);
+
+command_fails_like(
+	[ 'pg_restore', '--format=tar', '-f', '-', '--pipe="cat"' ],
+	qr/\Qpg_restore: error: option --pipe is only supported with directory format\E/,
+	'pg_restore: option --pipe is only supported with directory format'
+);
 
 command_fails_like(
 	[ 'pg_dump', '-c', '-a' ],
 	qr/\Qpg_dump: error: options -c\/--clean and -a\/--data-only cannot be used together\E/,
-	'pg_dump: options -c/--clean and -a/--data-only cannot be used together');
+	'pg_dump: options -c/--clean and -a/--data-only cannot be used together'
+);
 
 command_fails_like(
 	[ 'pg_dumpall', '-c', '-a' ],
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 3bc8e51561d..79c601371ab 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -7,6 +7,7 @@ use warnings FATAL => 'all';
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
 use Test::More;
+use File::Spec;
 
 my $tempdir = PostgreSQL::Test::Utils::tempdir;
 
@@ -46,6 +47,28 @@ my $tempdir = PostgreSQL::Test::Utils::tempdir;
 my $supports_icu = ($ENV{with_icu} eq 'yes');
 my $supports_gzip = check_pg_config("#define HAVE_LIBZ 1");
 
+# Use perl one-liner as a portable 'cat' replacement for Windows compatibility.
+# On Windows, perl opens file handles in text mode by default, which corrupts
+# binary archive data by translating newlines and interpreting EOF characters.
+# We use -Mopen=IO,:raw to force raw binary mode. We use -pe 1 instead of
+# -pe '' to avoid shell quoting issues with empty strings on Windows cmd.exe.
+my $perlbin = $^X;
+$perlbin =~ s!\\!/!g if $PostgreSQL::Test::Utils::windows_os;
+my $perl_cat = "$perlbin -Mopen=IO,:raw -pe 1";
+
+# Check for external gzip program for pipe tests.
+my $gzip_bin = $ENV{GZIP_PROGRAM} || 'gzip';
+my $has_gzip_bin = (system("$gzip_bin --version >" . File::Spec->devnull() . " 2>&1") == 0);
+
+# Create output directories for pipe tests
+mkdir "$tempdir/pipe_out_dir_parallel";
+mkdir "$tempdir/pipe_out_dir_parallel_8";
+mkdir "$tempdir/pipe_out_dir_complex";
+mkdir "$tempdir/pipe_out_dir_lo";
+mkdir "$tempdir/pipe_cross_dump";
+mkdir "$tempdir/pipe_cross_restore";
+mkdir "$tempdir/schema_only_pipe_dir";
+
 my %pgdump_runs = (
 	binary_upgrade => {
 		dump_cmd => [
@@ -223,6 +246,140 @@ my %pgdump_runs = (
 		],
 	},
 
+	defaults_dir_format_pipe => {
+		test_key => 'defaults',
+		dump_cmd => [
+			'pg_dump',
+			'--format' => 'directory',
+			'--pipe' => "$perl_cat > $tempdir/defaults_dir_format/%f",
+			'--statistics',
+			'postgres',
+		],
+	restore_cmd => [
+			'pg_restore',
+			'--format' => 'directory',
+			'--file' => "$tempdir/defaults_dir_format_pipe.sql",
+			'--pipe' => "$perl_cat $tempdir/defaults_dir_format/%f",
+			'--statistics',
+		],
+	},
+
+	defaults_dir_format_pipe_dump_only => {
+		test_key => 'defaults',
+		dump_cmd => [
+			'pg_dump',
+			'--format' => 'directory',
+			'--pipe' => "$perl_cat > $tempdir/pipe_cross_dump/%f",
+			'--statistics',
+			'postgres',
+		],
+	restore_cmd => [
+			'pg_restore',
+			'--format' => 'directory',
+			'--file' => "$tempdir/defaults_dir_format_pipe_dump_only.sql",
+			'--statistics',
+			"$tempdir/pipe_cross_dump",
+		],
+	},
+
+	defaults_dir_format_pipe_restore_only => {
+		test_key => 'defaults',
+		dump_cmd => [
+			'pg_dump',
+			'--format' => 'directory',
+			'--file' => "$tempdir/pipe_cross_restore",
+			'--statistics',
+			'postgres',
+		],
+	restore_cmd => [
+			'pg_restore',
+			'--format' => 'directory',
+			'--file' => "$tempdir/defaults_dir_format_pipe_restore_only.sql",
+			'--pipe' => ($supports_gzip && !$PostgreSQL::Test::Utils::windows_os)
+			? "if [ -f $tempdir/pipe_cross_restore/%f.gz ]; then $gzip_bin -d -c $tempdir/pipe_cross_restore/%f.gz; else $perl_cat $tempdir/pipe_cross_restore/%f; fi"
+			: "$perl_cat $tempdir/pipe_cross_restore/%f",
+			'--statistics',
+		],
+	},
+
+	defaults_parallel_pipe => {
+		test_key => 'defaults',
+		dump_cmd => [
+			'pg_dump',
+			'--format' => 'directory',
+			'--jobs' => 2,
+			'--pipe' => "$perl_cat > $tempdir/pipe_out_dir_parallel/%f",
+			'--statistics',
+			'postgres',
+		],
+		restore_cmd => [
+			'pg_restore',
+			'--format' => 'directory',
+			'--file' => "$tempdir/defaults_parallel_pipe.sql",
+			'--pipe' => "$perl_cat $tempdir/pipe_out_dir_parallel/%f",
+			'--statistics',
+		],
+	},
+
+	defaults_parallel_8_pipe => {
+		test_key => 'defaults',
+		dump_cmd => [
+			'pg_dump',
+			'--format' => 'directory',
+			'--jobs' => 8,
+			'--pipe' => "$perl_cat > $tempdir/pipe_out_dir_parallel_8/%f",
+			'--statistics',
+			'postgres',
+		],
+		restore_cmd => [
+			'pg_restore',
+			'--format' => 'directory',
+			'--file' => "$tempdir/defaults_parallel_8_pipe.sql",
+			'--pipe' => "$perl_cat $tempdir/pipe_out_dir_parallel_8/%f",
+			'--statistics',
+		],
+	},
+
+	defaults_complex_pipe => {
+		test_key => 'defaults',
+		skip_unless => \$has_gzip_bin,
+		dump_cmd => [
+			'pg_dump',
+			'--format' => 'directory',
+			'--pipe' => "$gzip_bin | $perl_cat > $tempdir/pipe_out_dir_complex/%f.gz",
+			'--statistics',
+			'postgres',
+		],
+		restore_cmd => [
+			'pg_restore',
+			'--format' => 'directory',
+			'--file' => "$tempdir/defaults_complex_pipe.sql",
+			'--pipe' => "$perl_cat $tempdir/pipe_out_dir_complex/%f.gz | $gzip_bin -d",
+			'--statistics',
+		],
+	},
+	defaults_lo_pipe => {
+		test_key => 'defaults',
+		dump_cmd => [
+			'pg_dump',
+			'--format' => 'directory',
+			'--statistics',
+			'--pipe' => "$perl_cat > $tempdir/pipe_out_dir_lo/%f",
+			'postgres',
+		],
+		restore_cmd => [
+			'pg_restore',
+			'--format' => 'directory',
+			'--file' => "$tempdir/defaults_lo_pipe.sql",
+			'--statistics',
+			'--pipe' => "$perl_cat $tempdir/pipe_out_dir_lo/%f",
+		],
+		glob_patterns => [
+			"$tempdir/pipe_out_dir_lo/toc.dat",
+			"$tempdir/pipe_out_dir_lo/blobs_*.toc",
+		],
+	},
+
 	# Do not use --no-sync to give test coverage for data sync.
 	defaults_parallel => {
 		test_key => 'defaults',
@@ -527,6 +684,22 @@ my %pgdump_runs = (
 			'postgres',
 		],
 	},
+	schema_only_pipe => {
+		test_key => 'schema_only',
+		dump_cmd => [
+			'pg_dump', '--no-sync',
+			'--format' => 'directory',
+			'--schema-only',
+			'--pipe' => "$perl_cat > $tempdir/schema_only_pipe_dir/%f",
+			'postgres',
+		],
+		restore_cmd => [
+			'pg_restore',
+			'--format' => 'directory',
+			'--file' => "$tempdir/schema_only_pipe.sql",
+			'--pipe' => "$perl_cat $tempdir/schema_only_pipe_dir/%f",
+		],
+	},
 	section_pre_data => {
 		dump_cmd => [
 			'pg_dump', '--no-sync',
@@ -5217,6 +5390,13 @@ foreach my $run (sort keys %pgdump_runs)
 	my $test_key = $run;
 	my $run_db = 'postgres';
 
+	if (defined($pgdump_runs{$run}->{skip_unless}) &&
+		!${ $pgdump_runs{$run}->{skip_unless} })
+	{
+		note "skipping run $run";
+		next;
+	}
+
 	$node->command_ok(\@{ $pgdump_runs{$run}->{dump_cmd} },
 		"$run: pg_dump runs");
 
@@ -5334,6 +5514,69 @@ foreach my $run (sort keys %pgdump_runs)
 	}
 }
 
+#########################################
+# Test error reporting for a failing pipe command.
+# We use a perl one-liner that exits with 1 after processing input.
+# This ensures we test the error handling in pclose() at the end of the dump,
+# verifying that the child's exit status is correctly captured and reported.
+my $failing_perl_cat = "$perlbin -Mopen=IO,:raw -pe \"END { exit 1 }\"";
+
+$node->command_fails_like(
+	[ 'pg_dump', '-Fd', "--pipe=$failing_perl_cat > %f", 'postgres' ],
+	qr/pipe command failed/,
+	'pg_dump pipe command error reporting'
+);
+
+$node->command_fails_like(
+	[ 'pg_restore', '-Fd', '-l', "--pipe=$failing_perl_cat " . "$tempdir/pipe_cross_dump" . '/%f' ],
+	qr/pipe command failed/,
+	'pg_restore pipe command error reporting'
+);
+
+# Targeted Edge Case Tests
+$node->command_fails_like(
+	[ 'pg_dump', '-Fd', '--pipe=/nonexistent/binary', 'postgres' ],
+	qr/could not write to file: (?:Broken pipe|The pipe has been ended)|Permission denied/,
+	'pg_dump early pipe command execution failure'
+);
+
+$node->command_fails_like(
+	[ 'pg_dump', '-Fd', '--pipe=no_such_command_at_all', 'postgres' ],
+	qr/could not write to file: (?:Broken pipe|The pipe has been ended)|not found|not recognized/,
+	'pg_dump command not found error reporting'
+);
+
+$node->command_fails_like(
+	[ 'pg_dump', '-Fd', '-f', '-', "--pipe=$perl_cat > %f", 'postgres' ],
+	qr/options -f\/--file and --pipe cannot be used together/,
+	'pg_dump options -f/--file and --pipe conflict check'
+);
+
+# Test that pg_restore rejects a positional argument when --pipe is used.
+# We create a dummy cluster archive (containing toc.glo) to verify that
+# even in cluster mode, the mutual exclusivity holds.
+mkdir "$tempdir/dummy_cluster_archive";
+open my $fh, '>', "$tempdir/dummy_cluster_archive/toc.glo";
+close $fh;
+
+$node->command_fails_like(
+	[ 'pg_restore', '-Fd', '-l', "--pipe=$perl_cat %f", "$tempdir/dummy_cluster_archive" ],
+	qr/cannot specify both an input file and --pipe/,
+	'pg_restore --pipe rejects positional argument even for cluster archive'
+);
+
+# Test that pg_dump --pipe bypasses local directory existence check.
+# We use a pipe command that writes to a subdirectory that hasn't been created.
+# The dump itself will fail when the pipe command tries to write to the
+# non-existent directory, but the error should come from the pipe command/write
+# failure, not from pg_dump's directory initialization.
+my $remote_dir = "$tempdir/non_existent_remote_dir";
+$node->command_fails_like(
+	[ 'pg_dump', '-Fd', "--pipe=$perl_cat > $remote_dir/%f", 'postgres' ],
+	qr/could not write to file: (?:Broken pipe|The pipe has been ended)|pipe command failed/,
+	'pg_dump --pipe bypasses local directory existence check'
+);
+
 #########################################
 # Stop the database instance, which will be removed at the end of the tests.
 
diff --git a/src/bin/pg_dump/t/004_pg_dump_parallel.pl b/src/bin/pg_dump/t/004_pg_dump_parallel.pl
index 738f34b1c1b..d7f02a6e142 100644
--- a/src/bin/pg_dump/t/004_pg_dump_parallel.pl
+++ b/src/bin/pg_dump/t/004_pg_dump_parallel.pl
@@ -8,9 +8,19 @@ use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
 use Test::More;
 
+# Use perl one-liner as a portable 'cat' replacement for Windows compatibility.
+# On Windows, perl opens file handles in text mode by default, which corrupts
+# binary archive data by translating newlines and interpreting EOF characters.
+# We use -Mopen=IO,:raw to force raw binary mode. We use -pe 1 instead of
+# -pe '' to avoid shell quoting issues with empty strings on Windows cmd.exe.
+my $perlbin = $^X;
+$perlbin =~ s!\\!/!g if $PostgreSQL::Test::Utils::windows_os;
+my $perl_cat = "$perlbin -Mopen=IO,:raw -pe 1";
+
 my $dbname1 = 'regression_src';
 my $dbname2 = 'regression_dest1';
 my $dbname3 = 'regression_dest2';
+my $dbname4 = 'regression_dest3';
 
 my $node = PostgreSQL::Test::Cluster->new('main');
 $node->init;
@@ -21,6 +31,7 @@ my $backupdir = $node->backup_dir;
 $node->run_log([ 'createdb', $dbname1 ]);
 $node->run_log([ 'createdb', $dbname2 ]);
 $node->run_log([ 'createdb', $dbname3 ]);
+$node->run_log([ 'createdb', $dbname4 ]);
 
 $node->safe_psql(
 	$dbname1,
@@ -87,4 +98,27 @@ $node->command_ok(
 	],
 	'parallel restore as inserts');
 
+mkdir "$backupdir/dump_pipe";
+
+$node->command_ok(
+	[
+		'pg_dump',
+		'--format' => 'directory',
+		'--no-sync',
+		'--jobs' => 2,
+		'--pipe' => "$perl_cat > $backupdir/dump_pipe/%f",
+		$node->connstr($dbname1),
+	],
+	'parallel dump with pipe');
+
+$node->command_ok(
+	[
+		'pg_restore', '--verbose',
+		'--dbname' => $node->connstr($dbname4),
+		'--format' => 'directory',
+		'--jobs' => 3,
+		'--pipe' => "$perl_cat $backupdir/dump_pipe/%f",
+	],
+	'parallel restore with pipe');
+
 done_testing();
diff --git a/src/bin/pg_dump/t/005_pg_dump_filterfile.pl b/src/bin/pg_dump/t/005_pg_dump_filterfile.pl
index b2630ef2897..83358696e18 100644
--- a/src/bin/pg_dump/t/005_pg_dump_filterfile.pl
+++ b/src/bin/pg_dump/t/005_pg_dump_filterfile.pl
@@ -8,6 +8,11 @@ use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
 use Test::More;
 
+# Use perl one-liner as a portable 'cat' replacement for Windows compatibility.
+my $perlbin = $^X;
+$perlbin =~ s!\\!/!g if $PostgreSQL::Test::Utils::windows_os;
+my $perl_cat = "$perlbin -pe ''";
+
 my $tempdir = PostgreSQL::Test::Utils::tempdir;
 my $inputfile;
 
@@ -98,6 +103,19 @@ command_ok(
 	],
 	"filter file without patterns");
 
+mkdir "$backupdir/dump_pipe_filter";
+
+command_ok(
+	[
+		'pg_dump',
+		'--port' => $port,
+		'--format' => 'directory',
+		'--pipe' => "$perl_cat > $backupdir/dump_pipe_filter/%f",
+		'--filter' => "$tempdir/inputfile.txt",
+		'postgres'
+	],
+	"filter file without patterns with pipe");
+
 my $dump = slurp_file($plainfile);
 
 like($dump, qr/^CREATE TABLE public\.table_one/m, "table one dumped");
-- 
2.54.0.563.g4f69b47b94-goog