Merge branch 'master' into 1.29.4-sync

arenadata/Dockerfile

@@ -0,0 +1,3 @@
+FROM hub.adsw.io/library/gpdb6_regress:latest
+
+COPY . /home/gpadmin/go/src/github.com/greenplum-db/gpbackup

arenadata/README.md

@@ -0,0 +1,6 @@
+## How to run tests
+
+```bash
+docker build -t gpbackup:test -f arenadata/Dockerfile .
+docker run --rm -it --sysctl 'kernel.sem=500 1024000 200 4096' gpbackup:test bash -c "ssh-keygen -A && /usr/sbin/sshd && bash /home/gpadmin/go/src/github.com/greenplum-db/gpbackup/arenadata/run_gpbackup_tests.bash"
+```

arenadata/arenadata_helper.go

@@ -0,0 +1,42 @@
+package arenadata
+
+import (
+	"regexp"
+	"strconv"
+
+	"github.com/greenplum-db/gp-common-go-libs/gplog"
+	"github.com/pkg/errors"
+)
+
+var (
+	adPattern = regexp.MustCompile(`_arenadata(\d+)`)
+)
+
+func EnsureAdVersionCompatibility(backupVersion string, restoreVersion string) {
+	var (
+		adBackup, adRestore int
+		err                 error
+	)
+	if strVersion := adPattern.FindAllStringSubmatch(backupVersion, -1); len(strVersion) > 0 {
+		adBackup, err = strconv.Atoi(strVersion[0][1])
+		gplog.FatalOnError(err)
+	} else {
+		gplog.Fatal(errors.Errorf("Invalid arenadata version format for gpbackup: %s", backupVersion), "")
+	}
+	if strVersion := adPattern.FindAllStringSubmatch(restoreVersion, -1); len(strVersion) > 0 {
+		adRestore, err = strconv.Atoi(strVersion[0][1])
+		gplog.FatalOnError(err)
+	} else {
+		gplog.Fatal(errors.Errorf("Invalid arenadata version format for gprestore: %s", restoreVersion), "")
+	}
+	if adRestore < adBackup {
+		gplog.Fatal(errors.Errorf("gprestore arenadata%d cannot restore a backup taken with gpbackup arenadata%d; please use gprestore arenadata%d or later.",
+			adRestore, adBackup, adBackup), "")
+	}
+}
+
+// fullVersion: gpbackup version + '_' + arenadata release + ('+' + gpbackup build)
+// example: 1.20.4_arenadata2+dev.1.g768b7e0 -> 1.20.4+dev.1.g768b7e0
+func GetOriginalVersion(fullVersion string) string {
+	return adPattern.ReplaceAllString(fullVersion, "")
+}

arenadata/run_gpbackup_tests.bash

@@ -0,0 +1,19 @@
+#!/bin/bash -l
+
+set -eox pipefail
+
+source gpdb_src/concourse/scripts/common.bash
+install_and_configure_gpdb
+make -C gpdb_src/src/test/regress/
+make -C gpdb_src/contrib/dummy_seclabel/ install
+gpdb_src/concourse/scripts/setup_gpadmin_user.bash
+make_cluster
+
+wget https://golang.org/dl/go1.20.5.linux-amd64.tar.gz -O - | tar -C /opt -xz;
+
+su - gpadmin -c "
+source /usr/local/greenplum-db-devel/greenplum_path.sh;
+source ~/gpdb_src/gpAux/gpdemo/gpdemo-env.sh;
+gpconfig -c shared_preload_libraries -v dummy_seclabel;
+gpstop -ar;
+PATH=$PATH:/opt/go/bin:~/go/bin GOPATH=~/go make depend build install test end_to_end -C /home/gpadmin/go/src/github.com/greenplum-db/gpbackup"

backup/backup.go

@@ -242,10 +242,10 @@ func backupPredata(metadataFile *utils.FileWithByteCount, tables []Table, tableO
 	objects := make([]Sortable, 0)
 	metadataMap := make(MetadataMap)
 
+	objects = append(objects, convertToSortableSlice(tables)...)
 	if !tableOnly {
 		functions, funcInfoMap = retrieveFunctions(&objects, metadataMap)
 	}
-	objects = append(objects, convertToSortableSlice(tables)...)
 	relationMetadata := GetMetadataForObjectType(connectionPool, TYPE_RELATION)
 	addToMetadataMap(relationMetadata, metadataMap)
 

backup/dependencies.go

@@ -2,6 +2,7 @@ package backup
 
 import (
 	"fmt"
+	"sort"
 	"strings"
 
 	"github.com/greenplum-db/gp-common-go-libs/dbconn"
@@ -87,6 +88,24 @@ type Sortable interface {
 	GetUniqueID() UniqueID
 }
 
+type Sortables []Sortable
+
+func (s Sortables) Len() int {
+	return len(s)
+}
+func (s Sortables) Swap(i, j int) {
+	s[i], s[j] = s[j], s[i]
+}
+func (s Sortables) Less(i, j int) bool {
+	return s[i].GetUniqueID().Oid < s[j].GetUniqueID().Oid
+}
+
+func SortByOid(sortables []Sortable) []Sortable {
+	s := Sortables(sortables)
+	sort.Sort(s)
+	return []Sortable(s)
+}
+
 func TopologicalSort(slice []Sortable, dependencies DependencyMap) []Sortable {
 	inDegrees := make(map[UniqueID]int)
 	dependencyIndexes := make(map[UniqueID]int)

backup/metadata_globals_test.go

@@ -50,9 +50,9 @@ var _ = Describe("backup/metadata_globals tests", func() {
 				`CREATE DATABASE testdb TEMPLATE template0;`,
 				`COMMENT ON DATABASE testdb IS 'This is a database comment.';`,
 				`ALTER DATABASE testdb OWNER TO testrole;`,
-				`REVOKE ALL ON DATABASE testdb FROM PUBLIC;
-REVOKE ALL ON DATABASE testdb FROM testrole;
-GRANT TEMPORARY,CONNECT ON DATABASE testdb TO testrole;`,
+				`REVOKE ALL ON DATABASE testdb FROM PUBLIC;`,
+				`REVOKE ALL ON DATABASE testdb FROM testrole;`,
+				`GRANT TEMPORARY,CONNECT ON DATABASE testdb TO testrole;`,
 				`SECURITY LABEL FOR dummy ON DATABASE testdb IS 'unclassified';`}
 			testutils.AssertBufferContents(tocfile.GlobalEntries, buffer, expectedStatements...)
 		})
@@ -428,9 +428,9 @@ ALTER ROLE "testRole2" WITH SUPERUSER INHERIT CREATEROLE CREATEDB LOGIN REPLICAT
 				`CREATE TABLESPACE test_tablespace FILESPACE test_filespace;`,
 				`COMMENT ON TABLESPACE test_tablespace IS 'This is a tablespace comment.';`,
 				`ALTER TABLESPACE test_tablespace OWNER TO testrole;`,
-				`REVOKE ALL ON TABLESPACE test_tablespace FROM PUBLIC;
-REVOKE ALL ON TABLESPACE test_tablespace FROM testrole;
-GRANT ALL ON TABLESPACE test_tablespace TO testrole;`,
+				`REVOKE ALL ON TABLESPACE test_tablespace FROM PUBLIC;`,
+				`REVOKE ALL ON TABLESPACE test_tablespace FROM testrole;`,
+				`GRANT ALL ON TABLESPACE test_tablespace TO testrole;`,
 				`SECURITY LABEL FOR dummy ON TABLESPACE test_tablespace IS 'unclassified';`}
 			testutils.AssertBufferContents(tocfile.GlobalEntries, buffer, expectedStatements...)
 

backup/predata_acl.go

@@ -108,8 +108,8 @@ func PrintObjectMetadata(metadataFile *utils.FileWithByteCount, toc *toc.TOC,
 			statements = append(statements, strings.TrimSpace(owner))
 		}
 	}
-	if privileges := metadata.GetPrivilegesStatements(obj.FQN(), entry.ObjectType); privileges != "" {
-		statements = append(statements, strings.TrimSpace(privileges))
+	if privileges := metadata.GetPrivilegesStatements(obj.FQN(), entry.ObjectType); len(privileges) > 0 {
+		statements = append(statements, privileges...);
 	}
 	if securityLabel := metadata.GetSecurityLabelStatement(obj.FQN(), entry.ObjectType); securityLabel != "" {
 		statements = append(statements, strings.TrimSpace(securityLabel))
@@ -138,8 +138,8 @@ func printExtensionFunctionACLs(metadataFile *utils.FileWithByteCount, toc *toc.
 	})
 	statements := make([]string, 0)
 	for _, obj := range objects {
-		if privileges := obj.GetPrivilegesStatements(obj.FQN(), "FUNCTION"); privileges != "" {
-			statements = append(statements, strings.TrimSpace(privileges))
+		if privileges := obj.GetPrivilegesStatements(obj.FQN(), "FUNCTION"); len(privileges) > 0 {
+			statements = append(statements, privileges...);
 			PrintStatements(metadataFile, toc, obj, statements)
 		}
 	}
@@ -290,7 +290,7 @@ func ParseACL(aclStr string) *ACL {
 	return nil
 }
 
-func (obj ObjectMetadata) GetPrivilegesStatements(objectName string, objectType string, columnName ...string) string {
+func (obj ObjectMetadata) GetPrivilegesStatements(objectName string, objectType string, columnName ...string) []string {
 	statements := make([]string, 0)
 	typeStr := fmt.Sprintf("%s ", objectType)
 	if objectType == "VIEW" || objectType == "FOREIGN TABLE" || objectType == "MATERIALIZED VIEW" {
@@ -325,10 +325,7 @@ func (obj ObjectMetadata) GetPrivilegesStatements(objectName string, objectType
 			}
 		}
 	}
-	if len(statements) > 0 {
-		return "\n\n" + strings.Join(statements, "\n")
-	}
-	return ""
+	return statements
 }
 
 func createPrivilegeStrings(acl ACL, objectType string) (string, string) {

backup/predata_acl_test.go

@@ -57,8 +57,14 @@ ALTER TABLE public.tablename OWNER TO testrole;`)
 			testhelper.ExpectRegexp(buffer, `
 
 REVOKE ALL ON TABLE public.tablename FROM PUBLIC;
+
+
 GRANT ALL ON TABLE public.tablename TO anothertestrole;
+
+
 GRANT SELECT,INSERT,UPDATE,DELETE,TRUNCATE,REFERENCES ON TABLE public.tablename TO testrole;
+
+
 GRANT TRIGGER ON TABLE public.tablename TO PUBLIC;`)
 		})
 		It("prints a block of REVOKE and GRANT statements WITH GRANT OPTION", func() {
@@ -67,8 +73,14 @@ GRANT TRIGGER ON TABLE public.tablename TO PUBLIC;`)
 			testhelper.ExpectRegexp(buffer, `
 
 REVOKE ALL ON TABLE public.tablename FROM PUBLIC;
+
+
 GRANT ALL ON TABLE public.tablename TO anothertestrole WITH GRANT OPTION;
+
+
 GRANT SELECT,INSERT,UPDATE,DELETE,TRUNCATE,REFERENCES ON TABLE public.tablename TO testrole WITH GRANT OPTION;
+
+
 GRANT TRIGGER ON TABLE public.tablename TO PUBLIC WITH GRANT OPTION;`)
 		})
 		It("prints a block of REVOKE and GRANT statements, some with WITH GRANT OPTION, some without", func() {
@@ -77,7 +89,11 @@ GRANT TRIGGER ON TABLE public.tablename TO PUBLIC WITH GRANT OPTION;`)
 			testhelper.ExpectRegexp(buffer, `
 
 REVOKE ALL ON TABLE public.tablename FROM PUBLIC;
+
+
 GRANT ALL ON TABLE public.tablename TO anothertestrole;
+
+
 GRANT SELECT,INSERT,UPDATE,DELETE,TRUNCATE,REFERENCES ON TABLE public.tablename TO testrole WITH GRANT OPTION;`)
 		})
 		It("prints both an ALTER TABLE ... OWNER TO statement and a table comment", func() {
@@ -99,9 +115,17 @@ ALTER TABLE public.tablename OWNER TO testrole;
 
 
 REVOKE ALL ON TABLE public.tablename FROM PUBLIC;
+
+
 REVOKE ALL ON TABLE public.tablename FROM testrole;
+
+
 GRANT ALL ON TABLE public.tablename TO anothertestrole;
+
+
 GRANT SELECT,INSERT,UPDATE,DELETE,TRUNCATE,REFERENCES ON TABLE public.tablename TO testrole;
+
+
 GRANT TRIGGER ON TABLE public.tablename TO PUBLIC;`)
 		})
 		It("prints both a block of REVOKE and GRANT statements and a table comment", func() {
@@ -113,8 +137,14 @@ COMMENT ON TABLE public.tablename IS 'This is a table comment.';
 
 
 REVOKE ALL ON TABLE public.tablename FROM PUBLIC;
+
+
 GRANT ALL ON TABLE public.tablename TO anothertestrole;
+
+
 GRANT SELECT,INSERT,UPDATE,DELETE,TRUNCATE,REFERENCES ON TABLE public.tablename TO testrole;
+
+
 GRANT TRIGGER ON TABLE public.tablename TO PUBLIC;`)
 		})
 		It("prints REVOKE and GRANT statements, an ALTER TABLE ... OWNER TO statement, and comments", func() {
@@ -129,9 +159,17 @@ ALTER TABLE public.tablename OWNER TO testrole;
 
 
 REVOKE ALL ON TABLE public.tablename FROM PUBLIC;
+
+
 REVOKE ALL ON TABLE public.tablename FROM testrole;
+
+
 GRANT ALL ON TABLE public.tablename TO anothertestrole;
+
+
 GRANT SELECT,INSERT,UPDATE,DELETE,TRUNCATE,REFERENCES ON TABLE public.tablename TO testrole;
+
+
 GRANT TRIGGER ON TABLE public.tablename TO PUBLIC;`)
 		})
 		It("prints SERVER for ALTER and FOREIGN SERVER for GRANT/REVOKE for a foreign server", func() {
@@ -145,7 +183,11 @@ ALTER SERVER foreignserver OWNER TO testrole;
 
 
 REVOKE ALL ON FOREIGN SERVER foreignserver FROM PUBLIC;
+
+
 REVOKE ALL ON FOREIGN SERVER foreignserver FROM testrole;
+
+
 GRANT ALL ON FOREIGN SERVER foreignserver TO testrole;`)
 		})
 		It("prints FUNCTION for REVOKE and AGGREGATE for ALTER for an aggregate function", func() {
@@ -159,6 +201,8 @@ ALTER AGGREGATE public.testagg(*) OWNER TO testrole;
 
 
 REVOKE ALL ON FUNCTION public.testagg(*) FROM PUBLIC;
+
+
 REVOKE ALL ON FUNCTION public.testagg(*) FROM testrole;`)
 		})
 		Context("Views and sequences have owners", func() {

backup/predata_externals_test.go

@@ -491,9 +491,9 @@ SEGMENT REJECT LIMIT 2 ROWS`)
 			expectedStatements := []string{
 				"CREATE PROTOCOL s3 (readfunc = public.read_fn_s3, writefunc = public.write_fn_s3);",
 				"ALTER PROTOCOL s3 OWNER TO testrole;",
-				`REVOKE ALL ON PROTOCOL s3 FROM PUBLIC;
-REVOKE ALL ON PROTOCOL s3 FROM testrole;
-GRANT ALL ON PROTOCOL s3 TO testrole;`}
+				`REVOKE ALL ON PROTOCOL s3 FROM PUBLIC;`,
+				`REVOKE ALL ON PROTOCOL s3 FROM testrole;`,
+				`GRANT ALL ON PROTOCOL s3 TO testrole;`}
 			testutils.AssertBufferContents(tocfile.PredataEntries, buffer, expectedStatements...)
 		})
 		It("prints a protocol ACL even when the protocol's CREATE statement is skipped", func() {
@@ -511,9 +511,9 @@ GRANT ALL ON PROTOCOL s3 TO testrole;`}
 			backup.PrintCreateExternalProtocolStatement(backupfile, tocfile, protocolUntrustedReadWrite, pgCatalogFuncInfoMap, protoMetadata)
 			expectedStatements := []string{
 				"ALTER PROTOCOL s3 OWNER TO testrole;",
-				`REVOKE ALL ON PROTOCOL s3 FROM PUBLIC;
-REVOKE ALL ON PROTOCOL s3 FROM testrole;
-GRANT ALL ON PROTOCOL s3 TO testrole;`}
+				`REVOKE ALL ON PROTOCOL s3 FROM PUBLIC;`,
+				`REVOKE ALL ON PROTOCOL s3 FROM testrole;`,
+				`GRANT ALL ON PROTOCOL s3 TO testrole;`}
 			testutils.AssertBufferContents(tocfile.PredataEntries, buffer, expectedStatements...)
 		})
 	})

backup/predata_functions_test.go

@@ -61,9 +61,9 @@ $$add_two_ints$$
 LANGUAGE internal%s;`, DEFAULT_PARALLEL),
 					"COMMENT ON FUNCTION public.func_name(integer, integer) IS 'This is a function comment.';",
 					"ALTER FUNCTION public.func_name(integer, integer) OWNER TO testrole;",
-					`REVOKE ALL ON FUNCTION public.func_name(integer, integer) FROM PUBLIC;
-REVOKE ALL ON FUNCTION public.func_name(integer, integer) FROM testrole;
-GRANT ALL ON FUNCTION public.func_name(integer, integer) TO testrole;`,
+					`REVOKE ALL ON FUNCTION public.func_name(integer, integer) FROM PUBLIC;`,
+					`REVOKE ALL ON FUNCTION public.func_name(integer, integer) FROM testrole;`,
+					`GRANT ALL ON FUNCTION public.func_name(integer, integer) TO testrole;`,
 					"SECURITY LABEL FOR dummy ON FUNCTION public.func_name(integer, integer) IS 'unclassified';"}
 				testutils.AssertBufferContents(tocfile.PredataEntries, buffer, expectedStatements...)
 
@@ -825,9 +825,9 @@ ALTER FUNCTION pg_catalog.plperl_validator(oid) OWNER TO testrole;`,
 				// Languages have implicit owners in 4.3, but do not support ALTER OWNER
 				expectedStatements = append(expectedStatements, "ALTER LANGUAGE plpythonu OWNER TO testrole;")
 			}
-			expectedStatements = append(expectedStatements, `REVOKE ALL ON LANGUAGE plpythonu FROM PUBLIC;
-REVOKE ALL ON LANGUAGE plpythonu FROM testrole;
-GRANT ALL ON LANGUAGE plpythonu TO testrole;`,
+			expectedStatements = append(expectedStatements, `REVOKE ALL ON LANGUAGE plpythonu FROM PUBLIC;`,
+				`REVOKE ALL ON LANGUAGE plpythonu FROM testrole;`,
+				`GRANT ALL ON LANGUAGE plpythonu TO testrole;`,
 				"SECURITY LABEL FOR dummy ON LANGUAGE plpythonu IS 'unclassified';")
 
 			testutils.AssertBufferContents(tocfile.PredataEntries, buffer, expectedStatements...)
@@ -853,9 +853,9 @@ GRANT ALL ON LANGUAGE plpythonu TO testrole;`,
 				// Languages have implicit owners in 4.3, but do not support ALTER OWNER
 				expectedStatements = append(expectedStatements, `ALTER LANGUAGE plperl OWNER TO testrole;`)
 			}
-			expectedStatements = append(expectedStatements, `REVOKE ALL ON LANGUAGE plperl FROM PUBLIC;
-REVOKE ALL ON LANGUAGE plperl FROM testrole;
-GRANT ALL ON LANGUAGE plperl TO testrole;`,
+			expectedStatements = append(expectedStatements, `REVOKE ALL ON LANGUAGE plperl FROM PUBLIC;`,
+				`REVOKE ALL ON LANGUAGE plperl FROM testrole;`,
+				`GRANT ALL ON LANGUAGE plperl TO testrole;`,
 				"SECURITY LABEL FOR dummy ON LANGUAGE plperl IS 'unclassified';")
 
 			testutils.AssertBufferContents(tocfile.PredataEntries, buffer, expectedStatements...)

backup/predata_relations.go

@@ -259,7 +259,7 @@ func PrintPostCreateTableStatements(metadataFile *utils.FileWithByteCount, toc *
 		if att.Privileges.Valid {
 			columnMetadata := ObjectMetadata{Privileges: getColumnACL(att.Privileges, att.Kind), Owner: tableMetadata.Owner}
 			columnPrivileges := columnMetadata.GetPrivilegesStatements(table.FQN(), "COLUMN", att.Name)
-			statements = append(statements, strings.TrimSpace(columnPrivileges))
+			statements = append(statements, columnPrivileges...)
 		}
 		if att.SecurityLabel != "" {
 			escapedLabel := utils.EscapeSingleQuotes(att.SecurityLabel)