Обсуждение: Test 002_pg_upgrade fails with olddump on Windows
Hello, When trying to use a custom dump with the test pg_upgrade/002_pg_upgrade, I observe the following test failure on Windows: >meson test --suite setup >echo create database regression>...\dump.sql >set olddump=...\dump.sql& set oldinstall=.../tmp_install/usr/local/pgsql& meson test pg_upgrade/002_pg_upgrade 1/1 postgresql:pg_upgrade / pg_upgrade/002_pg_upgrade ERROR 11.38s exit status 1 regress_log_002_pg_upgrade.txt contains: ... [09:07:06.704](3.793s) ok 11 - run of pg_upgrade for new instance ... [09:07:07.301](0.001s) not ok 15 - old and new dumps match after pg_upgrade [09:07:07.301](0.000s) # Failed test 'old and new dumps match after pg_upgrade' # at .../src/bin/pg_upgrade/t/002_pg_upgrade.pl line 452. [09:07:07.301](0.000s) # got: '1' # expected: '0' === diff of ...\build\testrun\pg_upgrade\002_pg_upgrade\data\tmp_test_ifk8/dump1.sql and ...\build\testrun\pg_upgrade\002_pg_upgrade\data\tmp_test_ifk8/dump2.sql === stdout === === stderr === === EOF === >dir "testrun\pg_upgrade\002_pg_upgrade\data\tmp_test_ifk8/" 11/25/2023 09:06 AM 2,729 dump1.sql 11/25/2023 09:07 AM 2,590 dump2.sql >diff -s "testrun\pg_upgrade\002_pg_upgrade\data\tmp_test_ifk8\dump1.sql" "testrun\pg_upgrade\002_pg_upgrade\data\tmp_test_ifk8\dump2.sql" Files testrun\pg_upgrade\002_pg_upgrade\data\tmp_test_ifk8\dump1.sql and testrun\pg_upgrade\002_pg_upgrade\data\tmp_test_ifk8\dump2.sql are identical As I can see, dump1.sql contains line endings 0d 0a, while dump2.sql — 0a. The attached patch fixes the issue for me. Best regards, Alexander
Вложения
On Sat, Nov 25, 2023 at 11:00:01PM +0300, Alexander Lakhin wrote: > diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl > index c6d83d3c21..d34b45e346 100644 > --- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl > +++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl > @@ -293,6 +293,7 @@ if (defined($ENV{oldinstall})) > } > > open my $fh, ">", $dump1_file or die "could not open dump file"; > + binmode $fh; > print $fh $dump_data; > close $fh; There is something I don't get here. The old and new dump files should be processed in filter_dump(), where AdjustUpgrade::adjust_old_dumpfile does the following so binmode should not be needed: # use Unix newlines $dump =~ s/\r\n/\n/g; Or you have used the test suite with an old installation that has the same major version as the new installation, meaning that the filtering was not happening, still you have detected some diffs? It sounds to me that we should just apply the filters to the dumps all the time if you have used matching versions. The filtering would remove only the comments, some extra newlines and replace the CRLFs in this case. -- Michael
Вложения
Hi Michael, 05.12.2023 10:56, Michael Paquier wrote: > Or you have used the test suite with an old installation that has the > same major version as the new installation, meaning that the filtering > was not happening, still you have detected some diffs? It sounds to > me that we should just apply the filters to the dumps all the time if > you have used matching versions. The filtering would remove only the > comments, some extra newlines and replace the CRLFs in this case. Yes, my case is with the same version, literally: build>echo create database regression>c:\temp\dump.sql build>set olddump=c:\temp\dump.sql& set oldinstall=%CD%/tmp_install/usr/local/pgsql& meson test pg_upgrade/002_pg_upgrade So removing the condition "if ($oldnode->pg_version != $newnode->pg_version)" works here as well, but maybe switching the file mode (to preserve EOLs produced by pg_dump) in the block "After dumping, update references ..." is more efficient than filtering dumps (on all OSes?). Best regards, Alexander
On Tue, Dec 05, 2023 at 12:00:00PM +0300, Alexander Lakhin wrote: > So removing the condition "if ($oldnode->pg_version != $newnode->pg_version)" > works here as well, but maybe switching the file mode (to preserve EOLs > produced by pg_dump) in the block "After dumping, update references ..." > is more efficient than filtering dumps (on all OSes?). Well, there's the argument that we replace the library references in a SQL file that we are handling as a text file, so switching it to use the binary mode is not right. A second argument is to apply the same filtering logic across both the old and new dumps, even if we know that the second dump file taken by pg_dump with not append CRLFs. At the end, just applying the filtering all the time makes the most sense to me, so I've applied a patch doing just that. -- Michael
Вложения
06.12.2023 04:17, Michael Paquier wrote: > At the end, just applying the filtering all the time makes the most > sense to me, so I've applied a patch doing just that. Thank you for the fix! Now that test with the minimal dump passes fine, but when I tried to run it with a complete dump borrowed from a normal test run: set olddump=& set oldinstall=& set PG_TEST_NOCLEAN=1& meson test pg_upgrade/002_pg_upgrade REM this test succeeded copy testrun\pg_upgrade\002_pg_upgrade\data\tmp_test_*\dump1.sql set olddump=c:\temp\dump1.sql& set oldinstall=%CD%/tmp_install/usr/local/pgsql& meson test pg_upgrade/002_pg_upgrade I encountered another failure: ... Creating dump of database schemas ok Checking for presence of required libraries fatal Your installation references loadable libraries that are missing from the new installation. You can add these libraries to the new installation, or remove the functions using them from the old installation. A list of problem libraries is in the file: .../build/testrun/pg_upgrade/002_pg_upgrade/data/t_002_pg_upgrade_new_node_data/pgdata/pg_upgrade_output.d/20231205T223247.304/loadable_libraries.txt Failure, exiting [22:32:51.086](3.796s) not ok 11 - run of pg_upgrade for new instance ... loadable_libraries.txt contains: could not load library ".../src/test/regress/refint.dll": ERROR: could not access file ".../src/test/regress/refint.dll": No such file or directory In database: regression could not load library ".../src/test/regress/autoinc.dll": ERROR: could not access file ".../src/test/regress/autoinc.dll": No such file or directory In database: regression could not load library ".../src/test/regress/regress.dll": ERROR: could not access file ".../src/test/regress/regress.dll": No such file or directory In database: regression Really, I can see refint.dll in ...\build\src\test\regress and in ...\build\tmp_install\usr\local\pgsql\lib, but not in .../src/test/regress/regress.dll c:\temp\dump1.sql contains: ... CREATE FUNCTION public.check_primary_key() RETURNS trigger LANGUAGE c AS '.../build/src/test/regress/refint.dll', 'check_primary_key'; while ...\build\testrun\pg_upgrade\002_pg_upgrade\data\tmp_test_T6jE\dump1.sql (for the failed test): ... CREATE FUNCTION public.check_primary_key() RETURNS trigger LANGUAGE c AS '.../src/test/regress/refint.dll', 'check_primary_key'; The same is on Linux: PG_TEST_NOCLEAN=1 meson test pg_upgrade/002_pg_upgrade cp testrun/pg_upgrade/002_pg_upgrade/data/tmp_test_*/dump1.sql /tmp/ olddump=/tmp/dump1.sql oldinstall=`pwd`/tmp_install/usr/local/pgsql meson test pg_upgrade/002_pg_upgrade So it looks like my $newregresssrc = "$srcdir/src/test/regress"; is incorrect for meson. Maybe it should be?: my $newregresssrc = dirname($ENV{REGRESS_SHLIB}); (With this line the test passes for me on Windows and Linux). Best regards, Alexander
On Wed, Dec 06, 2023 at 11:00:01AM +0300, Alexander Lakhin wrote: > So it looks like > my $newregresssrc = "$srcdir/src/test/regress"; > is incorrect for meson. > Maybe it should be?: > my $newregresssrc = dirname($ENV{REGRESS_SHLIB}); > (With this line the test passes for me on Windows and Linux). Hmm. Yes, it looks like you're right here. That should allow all the scenarios we expect to work to update the paths for the functions. -- Michael
Вложения
On Thu, Dec 07, 2023 at 05:44:53PM +0900, Michael Paquier wrote: > Hmm. Yes, it looks like you're right here. That should allow all the > scenarios we expect to work to update the paths for the functions. And done this one as well down to v15, where not only meson, but also vpath could have been confused with an update to an incorrect path. -- Michael
Вложения
On 2023-Dec-08, Michael Paquier wrote: > On Thu, Dec 07, 2023 at 05:44:53PM +0900, Michael Paquier wrote: > > Hmm. Yes, it looks like you're right here. That should allow all the > > scenarios we expect to work to update the paths for the functions. > > And done this one as well down to v15, where not only meson, but also > vpath could have been confused with an update to an incorrect path. Argh, yeah, this has caused me pain a couple of times. Thanks for fixing. -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/ "If you have nothing to say, maybe you need just the right tool to help you not say it." (New York Times, about Microsoft PowerPoint)