Merge pull request #90415 from ajwerner/backport22.2-90254

cockroachdb · Oct 21, 2022 · ebf272f · ebf272f
2 parents 8579361 + 7351c0a
commit ebf272f
Show file tree

Hide file tree

Showing 42 changed files with 1,334 additions and 992 deletions.
diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel
@@ -352,6 +352,7 @@ ALL_TESTS = [
     "//pkg/sql/execinfrapb:execinfrapb_test",
     "//pkg/sql/execstats:execstats_disallowed_imports_test",
     "//pkg/sql/execstats:execstats_test",
+    "//pkg/sql/exprutil:exprutil_test",
     "//pkg/sql/flowinfra:flowinfra_disallowed_imports_test",
     "//pkg/sql/flowinfra:flowinfra_test",
     "//pkg/sql/gcjob/gcjobnotifier:gcjobnotifier_test",
@@ -1464,6 +1465,9 @@ GO_TARGETS = [
     "//pkg/sql/execinfrapb:execinfrapb_test",
     "//pkg/sql/execstats:execstats",
     "//pkg/sql/execstats:execstats_test",
+    "//pkg/sql/exprutil:evalexpr",
+    "//pkg/sql/exprutil:exprutil",
+    "//pkg/sql/exprutil:exprutil_test",
     "//pkg/sql/faketreeeval:faketreeeval",
     "//pkg/sql/flowinfra:flowinfra",
     "//pkg/sql/flowinfra:flowinfra_test",
@@ -2619,6 +2623,7 @@ GET_X_DATA_TARGETS = [
     "//pkg/sql/execinfra/execreleasable:get_x_data",
     "//pkg/sql/execinfrapb:get_x_data",
     "//pkg/sql/execstats:get_x_data",
+    "//pkg/sql/exprutil:get_x_data",
     "//pkg/sql/faketreeeval:get_x_data",
     "//pkg/sql/flowinfra:get_x_data",
     "//pkg/sql/gcjob:get_x_data",

diff --git a/pkg/ccl/backupccl/BUILD.bazel b/pkg/ccl/backupccl/BUILD.bazel
@@ -95,6 +95,7 @@ go_library(
         "//pkg/sql/doctor",
         "//pkg/sql/execinfra",
         "//pkg/sql/execinfrapb",
+        "//pkg/sql/exprutil",
         "//pkg/sql/parser",
         "//pkg/sql/pgwire/pgcode",
         "//pkg/sql/pgwire/pgerror",

diff --git a/pkg/ccl/backupccl/alter_backup_planning.go b/pkg/ccl/backupccl/alter_backup_planning.go
@@ -20,10 +20,30 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
+	"github.com/cockroachdb/cockroach/pkg/sql/exprutil"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/errors"
 )
 
+func alterBackupTypeCheck(
+	ctx context.Context, stmt tree.Statement, p sql.PlanHookState,
+) (ok bool, _ colinfo.ResultColumns, _ error) {
+	alterBackupStmt, ok := stmt.(*tree.AlterBackup)
+	if !ok {
+		return false, nil, nil
+	}
+	if err := exprutil.TypeCheck(
+		ctx, "ALTER BACKUP", p.SemaCtx(),
+		exprutil.Strings{
+			alterBackupStmt.Backup,
+			alterBackupStmt.Subdir,
+		},
+	); err != nil {
+		return false, nil, err
+	}
+	return true, nil, nil
+}
+
 func alterBackupPlanHook(
 	ctx context.Context, stmt tree.Statement, p sql.PlanHookState,
 ) (sql.PlanHookRowFn, colinfo.ResultColumns, []sql.PlanNode, bool, error) {
@@ -41,46 +61,38 @@ func alterBackupPlanHook(
 		return nil, nil, nil, false, err
 	}
 
-	fromFn, err := p.TypeAsString(ctx, alterBackupStmt.Backup, "ALTER BACKUP")
+	exprEval := p.ExprEvaluator("ALTER BACKUP")
+	backup, err := exprEval.String(ctx, alterBackupStmt.Backup)
 	if err != nil {
 		return nil, nil, nil, false, err
 	}
 
-	subdirFn := func() (string, error) { return "", nil }
+	var subdir string
 	if alterBackupStmt.Subdir != nil {
-		subdirFn, err = p.TypeAsString(ctx, alterBackupStmt.Subdir, "ALTER BACKUP")
+		subdir, err = exprEval.String(ctx, alterBackupStmt.Subdir)
 		if err != nil {
 			return nil, nil, nil, false, err
 		}
 	}
 
-	var newKmsFn func() ([]string, error)
-	var oldKmsFn func() ([]string, error)
+	var newKms []string
+	var oldKms []string
 
 	for _, cmd := range alterBackupStmt.Cmds {
 		switch v := cmd.(type) {
 		case *tree.AlterBackupKMS:
-			newKmsFn, err = p.TypeAsStringArray(ctx, tree.Exprs(v.KMSInfo.NewKMSURI), "ALTER BACKUP")
+			newKms, err = exprEval.StringArray(ctx, tree.Exprs(v.KMSInfo.NewKMSURI))
 			if err != nil {
 				return nil, nil, nil, false, err
 			}
-			oldKmsFn, err = p.TypeAsStringArray(ctx, tree.Exprs(v.KMSInfo.OldKMSURI), "ALTER BACKUP")
+			oldKms, err = exprEval.StringArray(ctx, tree.Exprs(v.KMSInfo.OldKMSURI))
 			if err != nil {
 				return nil, nil, nil, false, err
 			}
 		}
 	}
 
 	fn := func(ctx context.Context, _ []sql.PlanNode, resultsCh chan<- tree.Datums) error {
-		backup, err := fromFn()
-		if err != nil {
-			return err
-		}
-
-		subdir, err := subdirFn()
-		if err != nil {
-			return err
-		}
 
 		if subdir != "" {
 			if strings.EqualFold(subdir, "LATEST") {
@@ -107,18 +119,6 @@ func alterBackupPlanHook(
 			}
 		}
 
-		var newKms []string
-		newKms, err = newKmsFn()
-		if err != nil {
-			return err
-		}
-
-		var oldKms []string
-		oldKms, err = oldKmsFn()
-		if err != nil {
-			return err
-		}
-
 		return doAlterBackupPlan(ctx, alterBackupStmt, p, backup, newKms, oldKms)
 	}
 
@@ -213,5 +213,9 @@ func doAlterBackupPlan(
 }
 
 func init() {
-	sql.AddPlanHook("alter backup", alterBackupPlanHook)
+	sql.AddPlanHook(
+		"alter backup",
+		alterBackupPlanHook,
+		alterBackupTypeCheck,
+	)
 }