Hi All,
In [1] we found that having a test to dump and restore objects left
behind by regression test is missing. Such a test would cover many
dump restore scenarios without much effort. It will also help identity
problems described in the same thread [2] during development itself.
I am starting a new thread to discuss such a test. Attached is a WIP
version of the test. The test does fail at the restore step when
commit 74563f6b90216180fc13649725179fc119dddeb5 is reverted
reintroducing the problem.
Attached WIP test is inspired from
src/bin/pg_upgrade/t/002_pg_upgrade.pl which tests binary-upgrade
dumps. Attached test tests the non-binary-upgrade dumps.
Similar to 0002_pg_upgrade.pl the test uses SQL dumps before and after
dump and restore to make sure that the objects are restored correctly.
The test has some shortcomings
1. Objects which are not dumped at all are never tested.
2. Since the rows are dumped in varying order by the two clusters, the
test only tests schema dump and restore.
3. The order of columns of the inheritance child table differs
depending upon the DDLs used to reach a given state. This introduces
diffs in the SQL dumps before and after restore. The test ignores
these diffs by hardcoding the diff in the test.
Even with 1 and 2 the test is useful to detect dump/restore anomalies.
I think we should improve 3, but I don't have a good and simpler
solution. I didn't find any way to compare two given clusters in our
TAP test framework. Building it will be a lot of work. Not sure if
it's worth it.
Suggestions welcome.
[1]
https://www.postgresql.org/message-id/CAExHW5vyqv%3DXLTcNMzCNccOrHiun_XhYPjcRqeV6dLvZSamriQ%40mail.gmail.com
[2] https://www.postgresql.org/message-id/3462358.1708107856%40sss.pgh.pa.us
--
Best Wishes,
Ashutosh Bapat
From 78903d2cad4e94e05db74b2473f82aabb498f987 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Date: Wed, 21 Feb 2024 11:02:40 +0530
Subject: [PATCH 1/2] WIP Test to dump and restore object left behind by
regression
Regression run leaves many database objects behind in a variety of states.
Dumping and restoring these objects covers many dump/restore scenarios not
covered elsewhere. src/bin/pg_upgrade/t/002_pg_upgrade.pl test pg_upgrade this
way. But it does not cover non-binary-upgrade dump and restore.
The test takes a dump of regression database from one cluster and restores it
on another cluster. It compares the dumps from both the clusters in SQL format
to make sure that the objects dumped were restored properly. Obviously if some
objects were not dumped, those remain untested. Hence this isn't a complete
test.
The order in which data rows are dumped varies a lot between dump and restore,
hence the tests only schema dumps.
The order in which columns of an inherited table are dumped varies depending
upon the DDLs used to set up inheritance. This introduces some difference in
the SQL dumps taken from the two clusters. Those differences are explicitly
ignored in the test.
TODO:
1. We could test formats other than -Fc
Ashutosh Bapat
---
src/bin/pg_dump/Makefile | 4 +
src/bin/pg_dump/t/006_pg_dump_regress.pl | 180 +++++++++++++++++++++++
2 files changed, 184 insertions(+)
create mode 100644 src/bin/pg_dump/t/006_pg_dump_regress.pl
diff --git a/src/bin/pg_dump/Makefile b/src/bin/pg_dump/Makefile
index 930c741c95..29a3d67953 100644
--- a/src/bin/pg_dump/Makefile
+++ b/src/bin/pg_dump/Makefile
@@ -42,6 +42,10 @@ OBJS = \
pg_backup_tar.o \
pg_backup_utils.o
+# required for 006_pg_dump_regress.pl
+REGRESS_SHLIB=$(abs_top_builddir)/src/test/regress/regress$(DLSUFFIX)
+export REGRESS_SHLIB
+
all: pg_dump pg_restore pg_dumpall
pg_dump: pg_dump.o common.o pg_dump_sort.o $(OBJS) | submake-libpq submake-libpgport submake-libpgfeutils
diff --git a/src/bin/pg_dump/t/006_pg_dump_regress.pl b/src/bin/pg_dump/t/006_pg_dump_regress.pl
new file mode 100644
index 0000000000..c3016f975d
--- /dev/null
+++ b/src/bin/pg_dump/t/006_pg_dump_regress.pl
@@ -0,0 +1,180 @@
+# Copyright (c) 2022-2024, PostgreSQL Global Development Group
+
+# Test dump and restore of regression data. This is expected to cover dump and
+# restore of most of the types of objects left behind in different states by
+# the regression test.
+use strict;
+use warnings FATAL => 'all';
+
+use Cwd qw(abs_path);
+use File::Basename qw(dirname);
+use File::Compare;
+use File::Find qw(find);
+use File::Path qw(rmtree);
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use PostgreSQL::Test::AdjustUpgrade;
+use Test::More;
+
+sub generate_regress_data
+{
+ my ($node) = @_;
+
+ # The default location of the source code is the root of this directory.
+ my $srcdir = abs_path("../../..");
+
+ # Grab any regression options that may be passed down by caller.
+ my $extra_opts = $ENV{EXTRA_REGRESS_OPTS} || "";
+
+ # --dlpath is needed to be able to find the location of regress.so
+ # and any libraries the regression tests require.
+ my $dlpath = dirname($ENV{REGRESS_SHLIB});
+
+ # --outputdir points to the path where to place the output files.
+ my $outputdir = $PostgreSQL::Test::Utils::tmp_check;
+
+ # --inputdir points to the path of the input files.
+ my $inputdir = "$srcdir/src/test/regress";
+
+ note 'running regression tests in old instance';
+ my $rc =
+ system($ENV{PG_REGRESS}
+ . " $extra_opts "
+ . "--dlpath=\"$dlpath\" "
+ . "--bindir= "
+ . "--host="
+ . $node->host . " "
+ . "--port="
+ . $node->port . " "
+ . "--schedule=$srcdir/src/test/regress/parallel_schedule "
+ . "--max-concurrent-tests=20 "
+ . "--inputdir=\"$inputdir\" "
+ . "--outputdir=\"$outputdir\"");
+ if ($rc != 0)
+ {
+ # Dump out the regression diffs file, if there is one
+ my $diffs = "$outputdir/regression.diffs";
+ if (-e $diffs)
+ {
+ print "=== dumping $diffs ===\n";
+ print slurp_file($diffs);
+ print "=== EOF ===\n";
+ }
+ }
+ is($rc, 0, 'regression tests pass');
+}
+
+# Paths to the dumps taken during the tests.
+my $tempdir = PostgreSQL::Test::Utils::tempdir;
+my $dump1_file = "$tempdir/dump1.sql";
+my $dump2_file = "$tempdir/dump2.sql";
+my $dump_file = "$tempdir/dump.out";
+
+# Initialize source node
+my $src_node =
+ PostgreSQL::Test::Cluster->new('src_node');
+
+$src_node->init();
+$src_node->start;
+generate_regress_data($src_node);
+
+# Initialize a new node for the upgrade.
+my $dst_node = PostgreSQL::Test::Cluster->new('new_node');
+
+$dst_node->init();
+
+# In a VPATH build, we'll be started in the source directory, but we want
+# to run pg_upgrade in the build directory so that any files generated finish
+# in it, like delete_old_cluster.{sh,bat}.
+chdir ${PostgreSQL::Test::Utils::tmp_check};
+
+# Dump source database for comparison later
+command_ok(
+ [
+ 'pg_dump', '-s', '-d', 'regression',
+ '-h', $src_node->host,
+ '-p', $src_node->port,
+ '-f', $dump1_file
+ ],
+ 'pg_dump on source instance');
+
+# Dump to be restored
+command_ok(
+ [
+ 'pg_dump', '-Fc', '-d', 'regression',
+ '-h', $src_node->host,
+ '-p', $src_node->port,
+ '-f', $dump_file
+ ],
+ 'pg_dump on source instance');
+
+$dst_node->start;
+$dst_node->command_ok(
+ [ 'createdb', 'regression' ],
+ "created destination database");
+
+# Restore into destination database
+command_ok(
+ [
+ 'pg_restore', '-d', 'regression',
+ '-h', $dst_node->host,
+ '-p', $dst_node->port,
+ $dump_file
+ ],
+ 'pg_restore on destination instance');
+
+# Dump from destination database for comparison
+command_ok(
+ [
+ 'pg_dump', '-s', '-d', 'regression',
+ '-h', $dst_node->host,
+ '-p', $dst_node->port,
+ '-f', $dump2_file
+ ],
+ 'pg_dump on destination instance');
+
+# Compare the two dumps. Usually there is no difference except a difference in
+# column order caused because of the way the tables are created in regression
+# tests and the way they are dumped. Treat that as an exception.
+my $expected_diff = " --
+ CREATE TABLE public.gtestxx_4 (
+- b integer,
+- a integer NOT NULL
++ a integer NOT NULL,
++ b integer
+ )
+ INHERITS (public.gtest1);
+ --
+ CREATE TABLE public.test_type_diff2_c1 (
++ int_two smallint,
+ int_four bigint,
+- int_eight bigint,
+- int_two smallint
++ int_eight bigint
+ )
+ INHERITS (public.test_type_diff2);
+ --
+ CREATE TABLE public.test_type_diff2_c2 (
+- int_eight bigint,
+ int_two smallint,
+- int_four bigint
++ int_four bigint,
++ int_eight bigint
+ )
+ INHERITS (public.test_type_diff2);
+ ";
+my ($stdout, $stderr) =
+ run_command([ 'diff', '-u', $dump1_file, $dump2_file]);
+# Clear file names, line numbersfrom the diffs; those are not going to remain
+# the same always. Also clear empty lines and normalize new line characters
+# across platforms.
+$stdout =~ s/^\@\@.*$//mg;
+$stdout =~ s/^.*$dump1_file.*$//mg;
+$stdout =~ s/^.*$dump2_file.*$//mg;
+$stdout =~ s/^\s*\n//mg;
+$stdout =~ s/\r\n/\n/g;
+$expected_diff =~ s/\r\n/\n/g;
+is($stdout, $expected_diff, 'old and new dumps match after dump and restore');
+
+done_testing();
\ No newline at end of file
--
2.25.1
