From b99c29e42671eb24031d56a91e44a1a273c6f37a Mon Sep 17 00:00:00 2001 From: Michael Butler Date: Wed, 6 Apr 2022 13:51:37 -0400 Subject: [PATCH] backupccl: ensure user passes locality aware uris with incremental_location Release note (sql change): This patch ensures the user passes the same number of locality aware URIs for the full backup destination as the incremental_location parameter. I.e. Good: `BACKUP INTO LATEST IN ($1, $2, $3) WITH incremental_location = ($4, $5, $6)` Bad: `BACKUP INTO LATEST IN $4 WITH incremental_location = ($1, $2, $3)` Note that the non locality uris for the full backup don't really affect incremental backup planning -- the patch just adds guardrails to the UX. Further work will ensure users cannot create incremental backup chains with mixed localities (#79135) --- pkg/ccl/backupccl/backup_planning.go | 4 ++++ pkg/ccl/backupccl/backup_test.go | 10 +++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/pkg/ccl/backupccl/backup_planning.go b/pkg/ccl/backupccl/backup_planning.go index 197e7da8abf6..8bf087d473e7 100644 --- a/pkg/ccl/backupccl/backup_planning.go +++ b/pkg/ccl/backupccl/backup_planning.go @@ -604,6 +604,10 @@ func backupPlanHook( if !backupStmt.Nested && len(incrementalStorage) > 0 { return errors.New("incremental_location option not supported with `BACKUP TO` syntax") } + if len(incrementalStorage) > 0 && (len(incrementalStorage) != len(to)) { + return errors.New("the incremental_location option must contain the same number of locality" + + " aware URIs as the full backup destination") + } endTime := p.ExecCfg().Clock.Now() if backupStmt.AsOf.Expr != nil { diff --git a/pkg/ccl/backupccl/backup_test.go b/pkg/ccl/backupccl/backup_test.go index 3c11994646b4..46b0afc7d2eb 100644 --- a/pkg/ccl/backupccl/backup_test.go +++ b/pkg/ccl/backupccl/backup_test.go @@ -651,7 +651,11 @@ func TestBackupAndRestoreJobDescription(t *testing.T) { sqlDB.Exec(t, "BACKUP INTO ($1, $2, $3)", collections...) sqlDB.Exec(t, "BACKUP INTO LATEST IN ($1, $2, $3)", collections...) sqlDB.Exec(t, "BACKUP INTO $4 IN ($1, $2, $3)", append(collections, "subdir")...) - sqlDB.Exec(t, "BACKUP INTO LATEST IN $4 WITH incremental_location = ($1, $2, $3)", + sqlDB.Exec(t, "BACKUP INTO LATEST IN ($1, $2, $3) WITH incremental_location = ($4, $5, $6)", + append(collections, incrementals...)...) + + sqlDB.ExpectErr(t, "the incremental_location option must contain the same number of locality", + "BACKUP INTO LATEST IN $4 WITH incremental_location = ($1, $2, $3)", append(incrementals, collections[0])...) // Find the subdirectory created by the full BACKUP INTO statement. @@ -673,8 +677,8 @@ func TestBackupAndRestoreJobDescription(t *testing.T) { collections[0], collections[1], collections[2])}, {fmt.Sprintf("BACKUP INTO '%s' IN ('%s', '%s', '%s')", "/subdir", collections[0], collections[1], collections[2])}, - {fmt.Sprintf("BACKUP INTO '%s' IN '%s' WITH incremental_location = ('%s', '%s', '%s')", - "/subdir", collections[0], incrementals[0], + {fmt.Sprintf("BACKUP INTO '%s' IN ('%s', '%s', '%s') WITH incremental_location = ('%s', '%s', '%s')", + "/subdir", collections[0], collections[1], collections[2], incrementals[0], incrementals[1], incrementals[2])}, }, )