ADBDEV-8282: Fix inefficient sql when restoring statistics

arenadata/arenadata_helper.go

@@ -3,12 +3,8 @@ package arenadata
 import (
 	"regexp"
 	"strconv"
-	"strings"
 
-	"github.com/GreengageDB/gp-common-go-libs/dbconn"
 	"github.com/GreengageDB/gp-common-go-libs/gplog"
-	"github.com/greenplum-db/gpbackup/history"
-	"github.com/greenplum-db/gpbackup/toc"
 	"github.com/pkg/errors"
 )
 
@@ -17,9 +13,22 @@ var (
 )
 
 func EnsureAdVersionCompatibility(backupVersion string, restoreVersion string) {
-	adBackup := getArenadataVersion(backupVersion)
-	adRestore := getArenadataVersion(restoreVersion)
-
+	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), "")
@@ -31,36 +40,3 @@ func EnsureAdVersionCompatibility(backupVersion string, restoreVersion string) {
 func GetOriginalVersion(fullVersion string) string {
 	return adPattern.ReplaceAllString(fullVersion, "")
 }
-
-func PatchStatisticsStatements(backupConfig *history.BackupConfig, connectionPool *dbconn.DBConn, statements []toc.StatementWithType) []toc.StatementWithType {
-	// Backups created in versions 1.30.5_arenadata16 to 1.30.5_arenadata19 have ineffective sql for
-	// deleting statistics, which can affect restore performance. as a workaround for these
-	// versions, we enable nested loop.
-	if connectionPool.Version.Is("6") &&
-		strings.Contains(backupConfig.BackupVersion, "1.30.5_arenadata") &&
-		len(statements) > 0 {
-		arenadataVersion := getArenadataVersion(backupConfig.BackupVersion)
-
-		if arenadataVersion >= 16 && arenadataVersion <= 19 {
-			statements = append(statements, toc.StatementWithType{})
-			copy(statements[1:], statements[:])
-			statements[0] = toc.StatementWithType{
-				Statement: "SET enable_nestloop = ON;",
-			}
-			statements = append(statements, toc.StatementWithType{
-				Statement: "RESET enable_nestloop;",
-			})
-		}
-	}
-	return statements
-}
-
-func getArenadataVersion(fullVersion string) uint {
-	match := adPattern.FindStringSubmatch(fullVersion)
-	if len(match) != 2 {
-		gplog.Fatal(errors.Errorf("Invalid arenadata version format for gpbackup: %s", fullVersion), "")
-	}
-	result, err := strconv.ParseUint(match[1], 10, 32)
-	gplog.FatalOnError(err)
-	return uint(result)
-}

restore/restore.go

@@ -3,7 +3,6 @@ package restore
 import (
 	"bufio"
 	"fmt"
-	"github.com/greenplum-db/gpbackup/arenadata"
 	"os"
 	"regexp"
 	"runtime/debug"
@@ -553,9 +552,6 @@ func restoreStatistics() {
 
 	statements := GetRestoreMetadataStatementsFiltered("statistics", statisticsFilename, []string{}, []string{}, filters)
 	editStatementsRedirectSchema(statements, opts.RedirectSchema)
-
-	statements = arenadata.PatchStatisticsStatements(backupConfig, connectionPool, statements)
-
 	numErrors := ExecuteRestoreMetadataStatements("statistics", statements, "Table statistics", nil, utils.PB_VERBOSE, false)
 
 	if numErrors > 0 {

restore/restore_internal_test.go

@@ -1,17 +1,8 @@
 package restore
 
 import (
-	"github.com/DATA-DOG/go-sqlmock"
-	"github.com/GreengageDB/gp-common-go-libs/cluster"
 	"github.com/GreengageDB/gp-common-go-libs/dbconn"
-	"github.com/GreengageDB/gp-common-go-libs/gplog"
-	"github.com/GreengageDB/gp-common-go-libs/testhelper"
-	"github.com/greenplum-db/gpbackup/filepath"
-	"github.com/greenplum-db/gpbackup/history"
-	"github.com/greenplum-db/gpbackup/options"
 	"github.com/greenplum-db/gpbackup/toc"
-	"os"
-	path "path/filepath"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -236,175 +227,4 @@ var _ = Describe("restore internal tests", func() {
 			}
 		})
 	})
-	Describe("Restore statistic", Ordered, func() {
-		var mock sqlmock.Sqlmock
-		BeforeAll(func() {
-			err := os.MkdirAll(path.Join(os.TempDir(), "backup/gpseg-1/backups/20250815/20250815120713/"), 0777)
-			Expect(err).ToNot(HaveOccurred())
-
-			err = os.WriteFile(path.Join(os.TempDir(), "backup/gpseg-1/backups/20250815/20250815120713/gpbackup_20250815120713_statistics.sql"), []byte(
-				`UPDATE pg_class
-SET
-        relpages = 194::int,
-        reltuples = 10000.000000::real
-WHERE oid = 'public.t1'::regclass::oid;
-
-DELETE FROM pg_statistic WHERE (starelid, staattnum) =
-        (SELECT attrelid, attnum FROM pg_attribute WHERE attrelid = 'public.t1'::regclass::oid AND attname = 'a');
-INSERT INTO pg_statistic SELECT
-        attrelid,
-        attnum,
-        false::boolean,
-        0.000000::real,
-        8::integer,
-        -1.000000::real,
-        2::smallint,
-        3::smallint,
-        0::smallint,
-        0::smallint,
-        0::smallint,
-        412::oid,
-        412::oid,
-        0::oid,
-        0::oid,
-        0::oid,
-        NULL::real[],
-        '{"0.313703984"}'::real[],
-        NULL::real[],
-        NULL::real[],
-        NULL::real[],
-        array_in('{"1","100",}', 'int8'::regtype::oid, -1),
-        NULL,
-        NULL,
-        NULL,
-        NULL
-FROM pg_attribute WHERE attrelid = 'public.t1'::regclass::oid AND attname = 'a';
-`), 0777)
-			Expect(err).ToNot(HaveOccurred())
-
-			err = os.WriteFile(path.Join(os.TempDir(), "backup/gpseg-1/backups/20250815/20250815120713/gpbackup_20250815120713_toc.yaml"), []byte(`
-statisticsentries:
-- schema: public
-  name: t1
-  objecttype: STATISTICS
-  referenceobject: ""
-  startbyte: 0
-  endbyte: 129
-  tier:
-  - 0
-  - 0
-- schema: public
-  name: t1
-  objecttype: STATISTICS
-  referenceobject: ""
-  startbyte: 129
-  endbyte: 299
-  tier:
-  - 0
-  - 0
-- schema: public
-  name: t1
-  objecttype: STATISTICS
-  referenceobject: ""
-  startbyte: 299
-  endbyte: 966
-  tier:
-  - 0
-  - 0
-`), 0777)
-
-			gplog.InitializeLogging("gprestore", path.Join(os.TempDir(), "gprestore_test"))
-			gplog.SetLogFileVerbosity(gplog.LOGERROR)
-
-			opts = &options.Options{}
-
-			globalTOC = toc.NewTOC(path.Join(os.TempDir(), "backup/gpseg-1/backups/20250815/20250815120713/gpbackup_20250815120713_toc.yaml"))
-			globalTOC.InitializeMetadataEntryMap()
-
-			DeferCleanup(func() {
-				err := os.RemoveAll(path.Join(os.TempDir(), "backup"))
-				Expect(err).NotTo(HaveOccurred())
-			})
-		})
-
-		BeforeEach(func() {
-			configCoordinator := cluster.SegConfig{ContentID: -1, Hostname: "localhost", DataDir: "gpseg-1"}
-			testCluster := cluster.NewCluster([]cluster.SegConfig{configCoordinator})
-			testFPInfo := filepath.NewFilePathInfo(testCluster, "", "20250815120713",
-				"gpseg", false)
-			testFPInfo.SegDirMap[-1] = path.Join(os.TempDir(), "backup/gpseg-1")
-			SetFPInfo(testFPInfo)
-
-			connectionPool, mock, _, _, _ = testhelper.SetupTestEnvironment()
-			connectionPool.Version = dbconn.NewVersion("6.0.0")
-		})
-
-		DescribeTable("Restore statistic with different backup versions", func(backupVersion string, needNestedLoop bool) {
-			testConfig := history.BackupConfig{
-				BackupVersion: backupVersion,
-			}
-			SetBackupConfig(&testConfig)
-
-			if needNestedLoop {
-				mock.ExpectExec("SET enable_nestloop = ON;").WillReturnResult(sqlmock.NewResult(0, 0))
-			}
-			mock.ExpectExec("UPDATE pg_class SET relpages = 194::int, reltuples = 10000\\.000000::real WHERE oid = 'public\\.t1'::regclass::oid;").
-				WillReturnResult(sqlmock.NewResult(0, 1))
-			mock.ExpectExec("DELETE FROM pg_statistic WHERE \\(starelid, staattnum\\) =" +
-				" \\(SELECT attrelid, attnum FROM pg_attribute WHERE attrelid = 'public\\.t1'::regclass::oid AND attname = 'a'\\);").
-				WillReturnResult(sqlmock.NewResult(0, 1))
-			mock.ExpectExec("INSERT INTO pg_statistic SELECT" +
-				" attrelid, attnum," +
-				" false::boolean," +
-				" 0\\.000000::real," +
-				" 8::integer," +
-				" -1.000000::real," +
-				" 2::smallint, 3::smallint, 0::smallint, 0::smallint, 0::smallint," +
-				" 412::oid, 412::oid, 0::oid, 0::oid, 0::oid," +
-				" NULL::real\\[\\], '{\"0.313703984\"}'::real\\[\\], NULL::real\\[\\], NULL::real\\[\\], NULL::real\\[\\]," +
-				" array_in\\('{\"1\",\"100\",}', 'int8'::regtype::oid, -1\\)," +
-				" NULL, NULL, NULL, NULL FROM pg_attribute WHERE attrelid = 'public.t1'::regclass::oid AND attname = 'a';").
-				WillReturnResult(sqlmock.NewResult(0, 1))
-			if needNestedLoop {
-				mock.ExpectExec("RESET enable_nestloop;").WillReturnResult(sqlmock.NewResult(0, 0))
-			}
-			restoreStatistics()
-			err := mock.ExpectationsWereMet()
-			Expect(err).NotTo(HaveOccurred())
-		},
-			Entry("before problematic version", "1.30.5_arenadata15", false),
-			Entry("problematic version", "1.30.5_arenadata18", true),
-			Entry("after problematic version", "1.30.5_arenadata20", false),
-		)
-
-		// This test must be before the "statistic is empty" test since the statement list must not be empty.
-		It("Wrong version", func() {
-			defer testhelper.ShouldPanicWithMessage("Invalid arenadata version format for gpbackup: 1.30.5_arenadataABC")
-			testConfig := history.BackupConfig{
-				BackupVersion: "1.30.5_arenadataABC",
-			}
-			SetBackupConfig(&testConfig)
-			restoreStatistics()
-		})
-
-		It("statistic is empty", func() {
-			testConfig := history.BackupConfig{
-				BackupVersion: "1.30.5_arenadata18",
-			}
-			SetBackupConfig(&testConfig)
-
-			err := os.WriteFile(path.Join(os.TempDir(), "backup/gpseg-1/backups/20250815/20250815120713/gpbackup_20250815120713_statistics.sql"),
-				[]byte("SET client_encoding = 'UTF8';"), 0777)
-			Expect(err).NotTo(HaveOccurred())
-			err = os.WriteFile(path.Join(os.TempDir(), "backup/gpseg-1/backups/20250815/20250815120713/gpbackup_20250815120713_toc.yaml"), []byte(""), 0777)
-			Expect(err).NotTo(HaveOccurred())
-
-			globalTOC = toc.NewTOC(path.Join(os.TempDir(), "backup/gpseg-1/backups/20250815/20250815120713/gpbackup_20250815120713_toc.yaml"))
-			globalTOC.InitializeMetadataEntryMap()
-
-			restoreStatistics()
-			err = mock.ExpectationsWereMet()
-			Expect(err).NotTo(HaveOccurred())
-		})
-	})
 })

restore/wrappers.go

@@ -89,6 +89,7 @@ SET client_min_messages = error;
 SET standard_conforming_strings = on;
 SET default_with_oids = off;
 SET optimizer = off;
+SET enable_nestloop = on;
 `
 
 	setupQuery += "SET gp_ignore_error_table = on;\n"