On Fri, Jul 12, 2024 at 10:42 AM Ashutosh Bapat <ashutosh.bapat....@gmail.com> wrote: > > I have merged the two patches now. >
894be11adfa60ad1ce5f74534cf5f04e66d51c30 changed the schema in which objects in test genereated_stored.sql are created. Because of this the new test added by the patch was failing. Fixed the failure in the attached. -- Best Wishes, Ashutosh Bapat
From 3b4573b0d3bb59fd21e01c3887a3d9cab8643238 Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com> Date: Thu, 27 Jun 2024 10:03:53 +0530 Subject: [PATCH 2/2] Test pg_dump/restore of regression objects 002_pg_upgrade.pl tests pg_upgrade on the regression database left behind by regression run. Modify it to test dump and restore of the regression database as well. Regression database created by regression run contains almost all the database objects supported by PostgreSQL in various states. The test thus covers wider dump and restore scenarios. When PG_TEST_EXTRA has 'regress_dump_formats' in it, test dump and restore in all supported formats. Otherwise test only "plain" format so that the test finishes quickly. Author: Ashutosh Bapat Reviewed by: Michael Pacquire Discussion: https://www.postgresql.org/message-id/CAExHW5uF5V=Cjecx3_Z=7xfh4rg2wf61pt+hfquzjbqourz...@mail.gmail.com --- doc/src/sgml/regress.sgml | 13 ++ src/bin/pg_upgrade/t/002_pg_upgrade.pl | 173 ++++++++++++++++++-- src/test/perl/PostgreSQL/Test/AdjustDump.pm | 109 ++++++++++++ 3 files changed, 277 insertions(+), 18 deletions(-) create mode 100644 src/test/perl/PostgreSQL/Test/AdjustDump.pm diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml index d1042e02228..8c1a9ddc403 100644 --- a/doc/src/sgml/regress.sgml +++ b/doc/src/sgml/regress.sgml @@ -336,6 +336,19 @@ make check-world PG_TEST_EXTRA='kerberos ldap ssl load_balance libpq_encryption' </para> </listitem> </varlistentry> + + <varlistentry> + <term><literal>regress_dump_formats</literal></term> + <listitem> + <para> + When enabled, + <filename>src/bin/pg_upgrade/t/002_pg_upgrade.pl</filename> tests dump + and restore of regression database using all dump formats. Otherwise + tests only <literal>plain</literal> format. Not enabled by default + because it is resource intensive. + </para> + </listitem> + </varlistentry> </variablelist> Tests for features that are not supported by the current build diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl index 17af2ce61ef..613512ffe7d 100644 --- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl +++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl @@ -13,6 +13,7 @@ use File::Path qw(rmtree); use PostgreSQL::Test::Cluster; use PostgreSQL::Test::Utils; use PostgreSQL::Test::AdjustUpgrade; +use PostgreSQL::Test::AdjustDump; use Test::More; # Can be changed to test the other modes. @@ -36,9 +37,9 @@ sub generate_db "created database with ASCII characters from $from_char to $to_char"); } -# Filter the contents of a dump before its use in a content comparison. -# This returns the path to the filtered dump. -sub filter_dump +# Filter the contents of a dump before its use in a content comparison for +# upgrade testing. This returns the path to the filtered dump. +sub filter_dump_for_upgrade { my ($is_old, $old_version, $dump_file) = @_; my $dump_contents = slurp_file($dump_file); @@ -61,6 +62,44 @@ sub filter_dump return $dump_file_filtered; } +# Test that the given two files match. The files usually contain pg_dump +# output in "plain" format. Output the difference if any. +sub compare_dumps +{ + my ($dump1, $dump2, $testname) = @_; + + my $compare_res = compare($dump1, $dump2); + is($compare_res, 0, $testname); + + # Provide more context if the dumps do not match. + if ($compare_res != 0) + { + my ($stdout, $stderr) = + run_command([ 'diff', '-u', $dump1, $dump2 ]); + print "=== diff of $dump1 and $dump2\n"; + print "=== stdout ===\n"; + print $stdout; + print "=== stderr ===\n"; + print $stderr; + print "=== EOF ===\n"; + } +} + +# Adjust the contents of a dump before its use in a content comparison for dump +# and restore testing. This returns the path to the adjusted dump. +sub adjust_dump_for_restore +{ + my ($dump_file, $is_original) = @_; + my $dump_adjusted = "${dump_file}_adjusted"; + + open(my $dh, '>', $dump_adjusted) + || die "opening $dump_adjusted "; + print $dh adjust_regress_dumpfile(slurp_file($dump_file), $is_original); + close($dh); + + return $dump_adjusted; +} + # The test of pg_upgrade requires two clusters, an old one and a new one # that gets upgraded. Before running the upgrade, a logical dump of the # old cluster is taken, and a second logical dump of the new one is taken @@ -258,6 +297,12 @@ else } } is($rc, 0, 'regression tests pass'); + + # Test dump/restore of the objects left behind by regression. Ideally it + # should be done in a separate test, but doing it here saves us one full + # regression run. Do this while the old cluster remains usable before + # upgrading it. + test_regression_dump_restore($oldnode, %node_params); } # Initialize a new node for the upgrade. @@ -502,24 +547,116 @@ push(@dump_command, '--extra-float-digits', '0') $newnode->command_ok(\@dump_command, 'dump after running pg_upgrade'); # Filter the contents of the dumps. -my $dump1_filtered = filter_dump(1, $oldnode->pg_version, $dump1_file); -my $dump2_filtered = filter_dump(0, $oldnode->pg_version, $dump2_file); +my $dump1_filtered = + filter_dump_for_upgrade(1, $oldnode->pg_version, $dump1_file); +my $dump2_filtered = + filter_dump_for_upgrade(0, $oldnode->pg_version, $dump2_file); # Compare the two dumps, there should be no differences. -my $compare_res = compare($dump1_filtered, $dump2_filtered); -is($compare_res, 0, 'old and new dumps match after pg_upgrade'); - -# Provide more context if the dumps do not match. -if ($compare_res != 0) +compare_dumps($dump1_filtered, $dump2_filtered, + 'old and new dumps match after pg_upgrade'); + +# Test dump and restore of objects left behind regression run. +# +# It is expected that regression tests, which create `regression` database, are +# run on `src_node`, which in turn is left in running state. The dump is +# restored on a fresh node created using given `node_params`. Plain dumps from +# both the nodes are compared to make sure that all the dumped objects are +# restored faithfully. +sub test_regression_dump_restore { - my ($stdout, $stderr) = - run_command([ 'diff', '-u', $dump1_filtered, $dump2_filtered ]); - print "=== diff of $dump1_filtered and $dump2_filtered\n"; - print "=== stdout ===\n"; - print $stdout; - print "=== stderr ===\n"; - print $stderr; - print "=== EOF ===\n"; + my ($src_node, %node_params) = @_; + my $dst_node = PostgreSQL::Test::Cluster->new('dst_node'); + + # Dump the original database in "plain" format for comparison later. The + # order of columns in COPY statements dumped from the original database and + # that from the restored database differs. These differences are hard to + # adjust. Hence we compare only schema dumps for now. + my $src_dump_file = "$tempdir/src_dump.sql"; + command_ok( + [ + 'pg_dump', '-s', + '--no-sync', '-d', + $src_node->connstr('regression'), '-f', + $src_dump_file + ], + 'pg_dump on original instance'); + my $src_adjusted_dump = adjust_dump_for_restore($src_dump_file, 1); + + # Setup destination database + $dst_node->init(%node_params); + $dst_node->start; + + # Testing all dump formats takes longer. Do it only when explicitly + # requested. + my @formats; + if ( $ENV{PG_TEST_EXTRA} + && $ENV{PG_TEST_EXTRA} =~ /\bregress_dump_formats\b/) + { + @formats = ('tar', 'directory', 'custom', 'plain'); + } + else + { + @formats = ('plain'); + } + + for my $format (@formats) + { + my $dump_file = "$tempdir/regression_dump.$format"; + my $format_spec = substr($format, 0, 1); + my $restored_db = 'regression_' . $format; + + # Even though we compare only schema from the original and the restored + # database, we dump and restore data as well to catch any errors while + # doing so. + command_ok( + [ + 'pg_dump', "-F$format_spec", '--no-sync', + '-d', $src_node->connstr('regression'), + '-f', $dump_file + ], + "pg_dump on source instance in '$format' format"); + + $dst_node->command_ok([ 'createdb', $restored_db ], + "created destination database '$restored_db'"); + + # Restore into destination database. + my @restore_command; + if ($format eq 'plain') + { + # Restore dump in "plain" format using `psql`. + @restore_command = [ + 'psql', '-d', $dst_node->connstr($restored_db), + '-f', $dump_file + ]; + } + else + { + @restore_command = [ + 'pg_restore', '-d', + $dst_node->connstr($restored_db), $dump_file + ]; + } + command_ok(@restore_command, + "pg_restore on destination instance in '$format' format"); + + # Dump restored database for comparison + my $dst_dump_file = "$tempdir/dest_dump.$format.sql"; + command_ok( + [ + 'pg_dump', '-s', + '--no-sync', '-d', + $dst_node->connstr($restored_db), '-f', + $dst_dump_file + ], + "pg_dump on instance restored with '$format' format"); + my $dst_adjusted_dump = adjust_dump_for_restore($dst_dump_file, 0); + + # Compare adjusted dumps, there should be no differences. + compare_dumps($src_adjusted_dump, $dst_adjusted_dump, + "dump outputs of original and restored regression database, using format '$format', match" + ); + } } done_testing(); diff --git a/src/test/perl/PostgreSQL/Test/AdjustDump.pm b/src/test/perl/PostgreSQL/Test/AdjustDump.pm new file mode 100644 index 00000000000..7697b488b17 --- /dev/null +++ b/src/test/perl/PostgreSQL/Test/AdjustDump.pm @@ -0,0 +1,109 @@ + +# Copyright (c) 2024-2025, PostgreSQL Global Development Group + +=pod + +=head1 NAME + +PostgreSQL::Test::AdjustDump - helper module for dump and restore tests + +=head1 SYNOPSIS + + use PostgreSQL::Test::AdjustDump; + + # Adjust contents of dump output file so that dump output from original + # regression database and that from the restored regression database match + $dump = adjust_regress_dumpfile($dump, $original); + +=head1 DESCRIPTION + +C<PostgreSQL::Test::AdjustDump> encapsulates various hacks needed to +compare the results of dump and retore tests + +=cut + +package PostgreSQL::Test::AdjustDump; + +use strict; +use warnings FATAL => 'all'; + +use Exporter 'import'; + +our @EXPORT = qw( + adjust_regress_dumpfile +); + +=pod + +=head1 ROUTINES + +=over + +=item $dump = adjust_regress_dumpfile($dump, $original) + +If we take dump of the regression database left behind after running regression +tests, restore the dump, and take dump of the restored regression database, the +outputs of both the dumps differ. Some regression tests purposefully create +some child tables in such a way that their column orders differ from column +orders of their respective parents. In the restored database, however, their +column orders are same as that of their respective parents. Thus the column +orders of these child tables in the original database and those in the restored +database differ, causing difference in the dump outputs. See MergeAttributes() +and dumpTableSchema() for details. + +This routine rearranges the column declarations in these C<CREATE TABLE ... INHERITS> +statements in the dump file from original database to match that from the +restored database. + +Additionally it adjusts blank and new lines to avoid noise. + +Arguments: + +=over + +=item C<dump>: Contents of dump file + +=item C<original>: 1 indicates that the given dump file is from the original +database, else 0 + +=back + +Returns the adjusted dump text. + +=cut + +sub adjust_regress_dumpfile +{ + my ($dump, $original) = @_; + + # use Unix newlines + $dump =~ s/\r\n/\n/g; + # Suppress blank lines, as some places in pg_dump emit more or fewer. + $dump =~ s/\n\n+/\n/g; + + # Adjust the CREATE TABLE ... INHERITS statements. + if ($original) + { + $dump =~ s/(^CREATE\sTABLE\sgenerated_stored_tests\.gtestxx_4\s\() + (\n\s+b\sinteger), + (\n\s+a\sinteger)/$1$3,$2/mgx; + $dump =~ s/(^CREATE\sTABLE\spublic\.test_type_diff2_c1\s\() + (\n\s+int_four\sbigint), + (\n\s+int_eight\sbigint), + (\n\s+int_two\ssmallint)/$1$4,$2,$3/mgx; + $dump =~ s/(CREATE\sTABLE\spublic\.test_type_diff2_c2\s\() + (\n\s+int_eight\sbigint), + (\n\s+int_two\ssmallint), + (\n\s+int_four\sbigint)/$1$3,$4,$2/mgx; + } + + return $dump; +} + +=pod + +=back + +=cut + +1; -- 2.34.1
