Hello!

On 02.06.2022 23:57, Andrew Dunstan wrote:


1. There is no mention of why there's a change w.r.t. Cygwin and
permissions checks. Maybe it's ok, but it seems off topic and is
certainly not referred to in the patch submission.

Thanks for the comments!
It was my error to change w.r.t. Cygwin. I've fixed it in the second version of the patch. But change in permissons check is correct. If we fix the error with initdb options, we've got the next one while testing upgrade from v10:
"files in PGDATA with permission != 640"
and the test.sh will end immediately.
The thing is that the default permissions have changed in v11+ due to this commit: https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=c37b3d08ca6873f9d4eaf24c72a90a550970cbb8.
Changes of permissions checks in test.sh fix this error.

> On 2022-06-01 We 21:37, Michael Paquier wrote:
>> A node's pg_version is assigned via _set_pg_version() when creating it
>> using PostgreSQL::Test::Cluster::new().  In order to make the
>> difference with the set of initdb options to use when setting up the
>> old node, it would be simpler to rely on that, no?  Version.pm is able
>> to handle integer as well as string comparisons for the version
>> strings.
>
2. As Michael says, we should not be using perl's version module, we
should be using the version object built into each
PostgreSQL::Test::Cluster instance.

Sure, very valuable note. Fixed it in the 2nd version of the patch attached.

Also find that i forgot to adjust initdb keys for new node in v15. So there was an error due to wal-segsize mismatch. Fixed it in the 2nd version too. And added patches for other versions.

> The client buildfarm does not make use of the in-core facility, as it > has its own module and logic to check after the case of cross-version > upgrades (see PGBuild/Modules/TestUpgradeXversion.pm)..

Michael, thanks a lot for your 2c.

With best regards,

--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
commit 2c8c78faba37f66c2ef88392f58ce8e241772300
Author: Anton A. Melnikov <a.melni...@postgrespro.ru>
Date:   Fri Jun 3 23:50:14 2022 +0300

    Fix test for pg_upgrade from 10x versions.

diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
index 80437e93b7..d9d97b1b3d 100644
--- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
+++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
@@ -58,7 +58,14 @@ my $oldnode =
 # To increase coverage of non-standard segment size and group access without
 # increasing test runtime, run these tests with a custom setting.
 # --allow-group-access and --wal-segsize have been added in v11.
-$oldnode->init(extra => [ '--wal-segsize', '1', '--allow-group-access' ]);
+my $ver_with_newopts = 11;
+my $oldver = $oldnode->{_pg_version};
+
+$oldnode->init(extra => [ '--wal-segsize', '1', '--allow-group-access' ])
+		if $oldver >= $ver_with_newopts;
+$oldnode->init()
+		if $oldver < $ver_with_newopts;
+
 $oldnode->start;
 
 # The default location of the source code is the root of this directory.
@@ -145,7 +152,10 @@ if (defined($ENV{oldinstall}))
 
 # Initialize a new node for the upgrade.
 my $newnode = PostgreSQL::Test::Cluster->new('new_node');
-$newnode->init(extra => [ '--wal-segsize', '1', '--allow-group-access' ]);
+$newnode->init(extra => [ '--wal-segsize', '1', '--allow-group-access' ])
+		if $oldver >= $ver_with_newopts;
+$newnode->init()
+		if $oldver < $ver_with_newopts;
 my $newbindir = $newnode->config_data('--bindir');
 my $oldbindir = $oldnode->config_data('--bindir');
 
commit dd62bd663167918365ce92577a19d208961a2f2a
Author: Anton A. Melnikov <a.melni...@postgrespro.ru>
Date:   Sat Jun 4 11:58:01 2022 +0300

    Fix test for pg_upgrade from 10x versions.

diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
index f353e565b5..ac9fc15646 100644
--- a/src/bin/pg_upgrade/test.sh
+++ b/src/bin/pg_upgrade/test.sh
@@ -24,7 +24,13 @@ standard_initdb() {
 	# without increasing test runtime, run these tests with a custom setting.
 	# Also, specify "-A trust" explicitly to suppress initdb's warning.
 	# --allow-group-access and --wal-segsize have been added in v11.
-	"$1" -N --wal-segsize 1 --allow-group-access -A trust
+	initdbopt="-N -A trust"
+	if [ $OLD_PG_VERSION_NUM -ge 110000 ]; then
+		initdbopt="$initdbopt --wal-segsize 1 --allow-group-access"
+	fi
+
+	"$1" $initdbopt
+
 	if [ -n "$TEMP_CONFIG" -a -r "$TEMP_CONFIG" ]
 	then
 		cat "$TEMP_CONFIG" >> "$PGDATA/postgresql.conf"
@@ -136,6 +142,7 @@ PGHOSTADDR="";        unset PGHOSTADDR
 
 # Select a non-conflicting port number, similarly to pg_regress.c
 PG_VERSION_NUM=`grep '#define PG_VERSION_NUM' "$newsrc"/src/include/pg_config.h | awk '{print $3}'`
+OLD_PG_VERSION_NUM=`grep '#define PG_VERSION_NUM' "$oldsrc"/src/include/pg_config.h | awk '{print $3}'`
 PGPORT=`expr $PG_VERSION_NUM % 16384 + 49152`
 export PGPORT
 
@@ -240,18 +247,26 @@ pg_upgrade $PG_UPGRADE_OPTS -d "${PGDATA}.old" -D "$PGDATA" -b "$oldbindir" -p "
 
 # make sure all directories and files have group permissions, on Unix hosts
 # Windows hosts don't support Unix-y permissions.
+if [ $OLD_PG_VERSION_NUM -lt 110000 ]; then
+	NEW_DIR_PERM=700
+	NEW_FILE_PERM=600
+else
+	NEW_DIR_PERM=750
+	NEW_FILE_PERM=640
+fi
+
 case $testhost in
 	MINGW*|CYGWIN*) ;;
-	*)	if [ `find "$PGDATA" -type f ! -perm 640 | wc -l` -ne 0 ]; then
-			echo "files in PGDATA with permission != 640";
+	*)	if [ `find "$PGDATA" -type f ! -perm $NEW_FILE_PERM | wc -l` -ne 0 ]; then
+			echo "files in PGDATA with permission != $NEW_FILE_PERM";
 			exit 1;
 		fi ;;
 esac
 
 case $testhost in
 	MINGW*|CYGWIN*) ;;
-	*)	if [ `find "$PGDATA" -type d ! -perm 750 | wc -l` -ne 0 ]; then
-			echo "directories in PGDATA with permission != 750";
+	*)	if [ `find "$PGDATA" -type d ! -perm $NEW_DIR_PERM | wc -l` -ne 0 ]; then
+			echo "directories in PGDATA with permission != $NEW_DIR_PERM";
 			exit 1;
 		fi ;;
 esac
commit 5857e955aabe1a4eb8a8e826ddcba02a25fd2b00
Author: Anton A. Melnikov <a.melni...@postgrespro.ru>
Date:   Sat Jun 4 12:24:16 2022 +0300

    Fix test for pg_upgrade from 10x versions.

diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
index 9f6fb3e018..4fc6e6657c 100644
--- a/src/bin/pg_upgrade/test.sh
+++ b/src/bin/pg_upgrade/test.sh
@@ -24,7 +24,13 @@ standard_initdb() {
 	# without increasing test runtime, run these tests with a custom setting.
 	# Also, specify "-A trust" explicitly to suppress initdb's warning.
 	# --allow-group-access and --wal-segsize have been added in v11.
-	"$1" -N --wal-segsize 1 --allow-group-access -A trust
+	initdbopt="-N -A trust"
+	if [ $OLD_PG_VERSION_NUM -ge 110000 ]; then
+		initdbopt="$initdbopt --wal-segsize 1 --allow-group-access"
+	fi
+
+	"$1" $initdbopt
+
 	if [ -n "$TEMP_CONFIG" -a -r "$TEMP_CONFIG" ]
 	then
 		cat "$TEMP_CONFIG" >> "$PGDATA/postgresql.conf"
@@ -131,6 +137,7 @@ PGHOSTADDR="";        unset PGHOSTADDR
 
 # Select a non-conflicting port number, similarly to pg_regress.c
 PG_VERSION_NUM=`grep '#define PG_VERSION_NUM' "$newsrc"/src/include/pg_config.h | awk '{print $3}'`
+OLD_PG_VERSION_NUM=`grep '#define PG_VERSION_NUM' "$oldsrc"/src/include/pg_config.h | awk '{print $3}'`
 PGPORT=`expr $PG_VERSION_NUM % 16384 + 49152`
 export PGPORT
 
@@ -235,18 +242,26 @@ pg_upgrade $PG_UPGRADE_OPTS -d "${PGDATA}.old" -D "$PGDATA" -b "$oldbindir" -p "
 
 # make sure all directories and files have group permissions, on Unix hosts
 # Windows hosts don't support Unix-y permissions.
+if [ $OLD_PG_VERSION_NUM -lt 110000 ]; then
+	NEW_DIR_PERM=700
+	NEW_FILE_PERM=600
+else
+	NEW_DIR_PERM=750
+	NEW_FILE_PERM=640
+fi
+
 case $testhost in
 	MINGW*) ;;
-	*)	if [ `find "$PGDATA" -type f ! -perm 640 | wc -l` -ne 0 ]; then
-			echo "files in PGDATA with permission != 640";
+	*)	if [ `find "$PGDATA" -type f ! -perm $NEW_FILE_PERM | wc -l` -ne 0 ]; then
+			echo "files in PGDATA with permission != $NEW_FILE_PERM";
 			exit 1;
 		fi ;;
 esac
 
 case $testhost in
 	MINGW*) ;;
-	*)	if [ `find "$PGDATA" -type d ! -perm 750 | wc -l` -ne 0 ]; then
-			echo "directories in PGDATA with permission != 750";
+	*)	if [ `find "$PGDATA" -type d ! -perm $NEW_DIR_PERM | wc -l` -ne 0 ]; then
+			echo "directories in PGDATA with permission != $NEW_DIR_PERM";
 			exit 1;
 		fi ;;
 esac
commit c3f47a7d857ee053b8b2c7c5de40b8ab9cb6bae7
Author: Anton A. Melnikov <a.melni...@postgrespro.ru>
Date:   Sat Jun 4 12:49:55 2022 +0300

    Fix test for pg_upgrade from 10x versions.

diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
index d4c4320a04..cc710633fb 100644
--- a/src/bin/pg_upgrade/test.sh
+++ b/src/bin/pg_upgrade/test.sh
@@ -23,7 +23,14 @@ standard_initdb() {
 	# To increase coverage of non-standard segment size and group access
 	# without increasing test runtime, run these tests with a custom setting.
 	# Also, specify "-A trust" explicitly to suppress initdb's warning.
-	"$1" -N --wal-segsize 1 -g -A trust
+	# --allow-group-access and --wal-segsize have been added in v11.
+	initdbopt="-N -A trust"
+	if [ $OLD_PG_VERSION_NUM -ge 110000 ]; then
+		initdbopt="$initdbopt --wal-segsize 1 --allow-group-access"
+	fi
+
+	"$1" $initdbopt
+
 	if [ -n "$TEMP_CONFIG" -a -r "$TEMP_CONFIG" ]
 	then
 		cat "$TEMP_CONFIG" >> "$PGDATA/postgresql.conf"
@@ -145,6 +152,7 @@ PGHOSTADDR="";        unset PGHOSTADDR
 
 # Select a non-conflicting port number, similarly to pg_regress.c
 PG_VERSION_NUM=`grep '#define PG_VERSION_NUM' "$newsrc"/src/include/pg_config.h | awk '{print $3}'`
+OLD_PG_VERSION_NUM=`grep '#define PG_VERSION_NUM' "$oldsrc"/src/include/pg_config.h | awk '{print $3}'`
 PGPORT=`expr $PG_VERSION_NUM % 16384 + 49152`
 export PGPORT
 
@@ -237,18 +245,26 @@ pg_upgrade $PG_UPGRADE_OPTS -d "${PGDATA}.old" -D "$PGDATA" -b "$oldbindir" -B "
 
 # make sure all directories and files have group permissions, on Unix hosts
 # Windows hosts don't support Unix-y permissions.
+if [ $OLD_PG_VERSION_NUM -lt 110000 ]; then
+	NEW_DIR_PERM=700
+	NEW_FILE_PERM=600
+else
+	NEW_DIR_PERM=750
+	NEW_FILE_PERM=640
+fi
+
 case $testhost in
 	MINGW*) ;;
-	*)	if [ `find "$PGDATA" -type f ! -perm 640 | wc -l` -ne 0 ]; then
-			echo "files in PGDATA with permission != 640";
+	*)	if [ `find "$PGDATA" -type f ! -perm $NEW_FILE_PERM | wc -l` -ne 0 ]; then
+			echo "files in PGDATA with permission != $NEW_FILE_PERM";
 			exit 1;
 		fi ;;
 esac
 
 case $testhost in
 	MINGW*) ;;
-	*)	if [ `find "$PGDATA" -type d ! -perm 750 | wc -l` -ne 0 ]; then
-			echo "directories in PGDATA with permission != 750";
+	*)	if [ `find "$PGDATA" -type d ! -perm $NEW_DIR_PERM | wc -l` -ne 0 ]; then
+			echo "directories in PGDATA with permission != $NEW_DIR_PERM";
 			exit 1;
 		fi ;;
 esac

Reply via email to