From d28b4d1321e709affd48109ccba8eaa8c0ed83bb Mon Sep 17 00:00:00 2001 From: Ben Bardin Date: Wed, 27 Jul 2022 12:12:07 -0400 Subject: [PATCH] backupccl: Use Expr for backup's Detached and Revision History options Release note (sql change): Can now specify explicit "true" and "false" values for "detached" and "revision_history" arguments in CREATE BACKUP and CREATE SCHEDULE FOR BACKUP. --- docs/generated/sql/bnf/stmt_block.bnf | 3 + pkg/ccl/backupccl/backup_planning.go | 26 ++++-- pkg/ccl/backupccl/backup_test.go | 2 +- pkg/ccl/backupccl/create_scheduled_backup.go | 2 +- .../backupccl/create_scheduled_backup_test.go | 28 +++--- pkg/ccl/backupccl/schedule_exec.go | 4 +- pkg/internal/sqlsmith/bulkio.go | 7 +- pkg/sql/parser/sql.y | 16 +++- pkg/sql/parser/testdata/backup_restore | 85 +++++++++++++------ pkg/sql/parser/testdata/create_schedule | 24 +++--- pkg/sql/planhook.go | 1 + pkg/sql/planner.go | 40 +++++++++ pkg/sql/sem/tree/backup.go | 19 +++-- .../pretty/backup.align-deindent.golden.short | 2 +- .../pretty/backup.align-only.golden.short | 2 +- .../testdata/pretty/backup.ref.golden.short | 2 +- pkg/sql/sem/tree/testdata/pretty/backup.sql | 2 +- 17 files changed, 187 insertions(+), 78 deletions(-) diff --git a/docs/generated/sql/bnf/stmt_block.bnf b/docs/generated/sql/bnf/stmt_block.bnf index 2a1c413dfd9a..9e9b4a3477d5 100644 --- a/docs/generated/sql/bnf/stmt_block.bnf +++ b/docs/generated/sql/bnf/stmt_block.bnf @@ -2160,7 +2160,10 @@ array_expr ::= backup_options ::= 'ENCRYPTION_PASSPHRASE' '=' string_or_placeholder | 'REVISION_HISTORY' + | 'REVISION_HISTORY' '=' a_expr | 'DETACHED' + | 'DETACHED' '=' 'TRUE' + | 'DETACHED' '=' 'FALSE' | 'KMS' '=' string_or_placeholder_opt_list | 'INCREMENTAL_LOCATION' '=' string_or_placeholder_opt_list diff --git a/pkg/ccl/backupccl/backup_planning.go b/pkg/ccl/backupccl/backup_planning.go index ee7e5dab9542..4f58ffd7afda 100644 --- a/pkg/ccl/backupccl/backup_planning.go +++ b/pkg/ccl/backupccl/backup_planning.go @@ -426,6 +426,18 @@ func backupPlanHook( return nil, nil, nil, false, err } + detached := false + if backupStmt.Options.Detached == tree.DBoolTrue { + detached = true + } + revisionHistoryFn := func() (bool, error) { return false, nil } // Defaults to false. + if backupStmt.Options.CaptureRevisionHistory != nil { + revisionHistoryFn, err = p.TypeAsBool(ctx, backupStmt.Options.CaptureRevisionHistory, "BACKUP") + if err != nil { + return nil, nil, nil, false, err + } + } + encryptionParams := jobspb.BackupEncryptionOptions{Mode: jobspb.EncryptionMode_None} var pwFn func() (string, error) @@ -464,7 +476,7 @@ func backupPlanHook( ctx, span := tracing.ChildSpan(ctx, stmt.StatementTag()) defer span.Finish() - if !(p.ExtendedEvalContext().TxnIsSingleStmt || backupStmt.Options.Detached) { + if !(p.ExtendedEvalContext().TxnIsSingleStmt || detached) { return errors.Errorf("BACKUP cannot be used inside a multi-statement transaction without DETACHED option") } @@ -529,12 +541,14 @@ func backupPlanHook( } } - var revisionHistory bool - if backupStmt.Options.CaptureRevisionHistory { + revisionHistory, err := revisionHistoryFn() + if err != nil { + return err + } + if revisionHistory { if err := requireEnterprise(p.ExecCfg(), "revision_history"); err != nil { return err } - revisionHistory = true } var targetDescs []catalog.Descriptor @@ -633,7 +647,7 @@ func backupPlanHook( } plannerTxn := p.Txn() - if backupStmt.Options.Detached { + if detached { // When running inside an explicit transaction, we simply create the job // record. We do not wait for the job to finish. _, err := p.ExecCfg().JobRegistry.CreateAdoptableJobWithTxn( @@ -674,7 +688,7 @@ func backupPlanHook( return sj.ReportExecutionResults(ctx, resultsCh) } - if backupStmt.Options.Detached { + if detached { return fn, jobs.DetachedJobExecutionResultHeader, nil, false, nil } return fn, jobs.BulkJobExecutionResultHeader, nil, false, nil diff --git a/pkg/ccl/backupccl/backup_test.go b/pkg/ccl/backupccl/backup_test.go index a11cd08f3bda..3d5f1169da09 100644 --- a/pkg/ccl/backupccl/backup_test.go +++ b/pkg/ccl/backupccl/backup_test.go @@ -5583,7 +5583,7 @@ func TestBackupRestoreShowJob(t *testing.T) { sqlDB.CheckQueryResults( t, "SELECT description FROM [SHOW JOBS] WHERE description != 'updating privileges' ORDER BY description", [][]string{ - {"BACKUP DATABASE data TO 'nodelocal://0/foo' WITH revision_history"}, + {"BACKUP DATABASE data TO 'nodelocal://0/foo' WITH revision_history = true"}, {"RESTORE TABLE data.bank FROM 'nodelocal://0/foo' WITH into_db = 'data 2', skip_missing_foreign_keys"}, }, ) diff --git a/pkg/ccl/backupccl/create_scheduled_backup.go b/pkg/ccl/backupccl/create_scheduled_backup.go index 7b57b2a71cbc..08d1ddfaf89c 100644 --- a/pkg/ccl/backupccl/create_scheduled_backup.go +++ b/pkg/ccl/backupccl/create_scheduled_backup.go @@ -296,7 +296,7 @@ func doCreateBackupSchedules( backupNode := &tree.Backup{ Options: tree.BackupOptions{ CaptureRevisionHistory: eval.BackupOptions.CaptureRevisionHistory, - Detached: true, + Detached: tree.DBoolTrue, }, Nested: true, AppendToLatest: false, diff --git a/pkg/ccl/backupccl/create_scheduled_backup_test.go b/pkg/ccl/backupccl/create_scheduled_backup_test.go index 2309cefdb413..df41931b2e12 100644 --- a/pkg/ccl/backupccl/create_scheduled_backup_test.go +++ b/pkg/ccl/backupccl/create_scheduled_backup_test.go @@ -422,7 +422,7 @@ func TestSerializesScheduledBackupExecutionArgs(t *testing.T) { expectedSchedules: []expectedSchedule{ { nameRe: "BACKUP .+", - backupStmt: "BACKUP INTO 'nodelocal://0/backup' WITH revision_history, detached", + backupStmt: "BACKUP INTO 'nodelocal://0/backup' WITH revision_history = true, detached", period: time.Hour, }, }, @@ -459,7 +459,7 @@ func TestSerializesScheduledBackupExecutionArgs(t *testing.T) { nameRe: "BACKUP .*", backupStmt: "BACKUP TABLE system.public.jobs, " + "system.public.scheduled_jobs INTO LATEST IN 'nodelocal://0/backup' WITH" + - " revision_history, detached", + " revision_history = true, detached", period: time.Hour, paused: true, chainProtectedTimestampRecord: true, @@ -467,7 +467,7 @@ func TestSerializesScheduledBackupExecutionArgs(t *testing.T) { { nameRe: "BACKUP .+", backupStmt: "BACKUP TABLE system.public.jobs, " + - "system.public.scheduled_jobs INTO 'nodelocal://0/backup' WITH revision_history, detached", + "system.public.scheduled_jobs INTO 'nodelocal://0/backup' WITH revision_history = true, detached", period: 24 * time.Hour, runsNow: true, chainProtectedTimestampRecord: true, @@ -483,14 +483,14 @@ func TestSerializesScheduledBackupExecutionArgs(t *testing.T) { expectedSchedules: []expectedSchedule{ { nameRe: "BACKUP .*", - backupStmt: "BACKUP DATABASE system INTO LATEST IN 'nodelocal://0/backup' WITH revision_history, detached", + backupStmt: "BACKUP DATABASE system INTO LATEST IN 'nodelocal://0/backup' WITH revision_history = true, detached", period: time.Hour, paused: true, chainProtectedTimestampRecord: true, }, { nameRe: "BACKUP .+", - backupStmt: "BACKUP DATABASE system INTO 'nodelocal://0/backup' WITH revision_history, detached", + backupStmt: "BACKUP DATABASE system INTO 'nodelocal://0/backup' WITH revision_history = true, detached", period: 24 * time.Hour, runsNow: true, chainProtectedTimestampRecord: true, @@ -506,14 +506,14 @@ func TestSerializesScheduledBackupExecutionArgs(t *testing.T) { expectedSchedules: []expectedSchedule{ { nameRe: "BACKUP .*", - backupStmt: "BACKUP TABLE system.public.* INTO LATEST IN 'nodelocal://0/backup' WITH revision_history, detached", + backupStmt: "BACKUP TABLE system.public.* INTO LATEST IN 'nodelocal://0/backup' WITH revision_history = true, detached", period: time.Hour, paused: true, chainProtectedTimestampRecord: true, }, { nameRe: "BACKUP .+", - backupStmt: "BACKUP TABLE system.public.* INTO 'nodelocal://0/backup' WITH revision_history, detached", + backupStmt: "BACKUP TABLE system.public.* INTO 'nodelocal://0/backup' WITH revision_history = true, detached", period: 24 * time.Hour, runsNow: true, chainProtectedTimestampRecord: true, @@ -546,14 +546,14 @@ func TestSerializesScheduledBackupExecutionArgs(t *testing.T) { expectedSchedules: []expectedSchedule{ { nameRe: "my_backup_name", - backupStmt: "BACKUP INTO LATEST IN 'nodelocal://0/backup' WITH revision_history, detached", + backupStmt: "BACKUP INTO LATEST IN 'nodelocal://0/backup' WITH revision_history = true, detached", period: time.Hour, paused: true, chainProtectedTimestampRecord: true, }, { nameRe: "my_backup_name", - backupStmt: "BACKUP INTO 'nodelocal://0/backup' WITH revision_history, detached", + backupStmt: "BACKUP INTO 'nodelocal://0/backup' WITH revision_history = true, detached", period: 24 * time.Hour, runsNow: true, chainProtectedTimestampRecord: true, @@ -571,10 +571,10 @@ func TestSerializesScheduledBackupExecutionArgs(t *testing.T) { nameRe: "BACKUP .*", backupStmt: "BACKUP TABLE system.public.jobs, " + "system.public.scheduled_jobs INTO 'nodelocal://0/backup' WITH" + - " revision_history, encryption_passphrase = 'secret', detached", + " revision_history = true, encryption_passphrase = 'secret', detached", shownStmt: "BACKUP TABLE system.public.jobs, " + "system.public.scheduled_jobs INTO 'nodelocal://0/backup' WITH" + - " revision_history, encryption_passphrase = '*****', detached", + " revision_history = true, encryption_passphrase = '*****', detached", period: 7 * 24 * time.Hour, }, }, @@ -596,7 +596,7 @@ func TestSerializesScheduledBackupExecutionArgs(t *testing.T) { nameRe: "BACKUP .+", backupStmt: "BACKUP DATABASE system INTO " + "('nodelocal://0/backup?COCKROACH_LOCALITY=x%3Dy', 'nodelocal://0/backup2?COCKROACH_LOCALITY=default') " + - "WITH revision_history, detached", + "WITH revision_history = true, detached", period: 24 * time.Hour, }, }, @@ -614,7 +614,7 @@ func TestSerializesScheduledBackupExecutionArgs(t *testing.T) { }, } - for _, tc := range testCases { + for i, tc := range testCases { t.Run(fmt.Sprintf("%s-%s", tc.name, tc.user), func(t *testing.T) { defer th.clearSchedules(t) @@ -652,7 +652,7 @@ func TestSerializesScheduledBackupExecutionArgs(t *testing.T) { for _, s := range schedules { stmt := getScheduledBackupStatement(t, s.ExecutionArgs()) expectedSchedule, ok := expectedByName[stmt] - require.True(t, ok, "could not find matching name for %q", stmt) + require.True(t, ok, "in test case %d, could not find matching name for %q", i, stmt) require.Regexp(t, regexp.MustCompile(expectedSchedule.nameRe), s.ScheduleLabel()) expectedShown := fmt.Sprintf("%q", expectedSchedule.backupStmt) diff --git a/pkg/ccl/backupccl/schedule_exec.go b/pkg/ccl/backupccl/schedule_exec.go index 8f975220a65a..62f749659102 100644 --- a/pkg/ccl/backupccl/schedule_exec.go +++ b/pkg/ccl/backupccl/schedule_exec.go @@ -70,8 +70,8 @@ func (e *scheduledBackupExecutor) executeBackup( } // Sanity check: backup should be detached. - if !backupStmt.Options.Detached { - backupStmt.Options.Detached = true + if backupStmt.Options.Detached != tree.DBoolTrue { + backupStmt.Options.Detached = tree.DBoolTrue log.Warningf(ctx, "force setting detached option for backup schedule %d", sj.ScheduleID()) } diff --git a/pkg/internal/sqlsmith/bulkio.go b/pkg/internal/sqlsmith/bulkio.go index 51940e6ab59e..4cbba8344478 100644 --- a/pkg/internal/sqlsmith/bulkio.go +++ b/pkg/internal/sqlsmith/bulkio.go @@ -106,11 +106,16 @@ func makeBackup(s *Smither) (tree.Statement, bool) { s.bulkBackups[name] = targets s.lock.Unlock() + coinD := tree.DBoolFalse + if s.coin() { + coinD = tree.DBoolTrue + } + return &tree.Backup{ Targets: &targets, To: tree.StringOrPlaceholderOptList{tree.NewStrVal(name)}, AsOf: makeAsOf(s), - Options: tree.BackupOptions{CaptureRevisionHistory: s.coin()}, + Options: tree.BackupOptions{CaptureRevisionHistory: coinD}, }, true } diff --git a/pkg/sql/parser/sql.y b/pkg/sql/parser/sql.y index f822ad983ac2..2867c500cae8 100644 --- a/pkg/sql/parser/sql.y +++ b/pkg/sql/parser/sql.y @@ -2972,11 +2972,23 @@ backup_options: } | REVISION_HISTORY { - $$.val = &tree.BackupOptions{CaptureRevisionHistory: true} + $$.val = &tree.BackupOptions{CaptureRevisionHistory: tree.MakeDBool(true)} + } +| REVISION_HISTORY '=' a_expr + { + $$.val = &tree.BackupOptions{CaptureRevisionHistory: $3.expr()} } | DETACHED { - $$.val = &tree.BackupOptions{Detached: true} + $$.val = &tree.BackupOptions{Detached: tree.MakeDBool(true)} + } +| DETACHED '=' TRUE + { + $$.val = &tree.BackupOptions{Detached: tree.MakeDBool(true)} + } +| DETACHED '=' FALSE + { + $$.val = &tree.BackupOptions{Detached: tree.MakeDBool(false)} } | KMS '=' string_or_placeholder_opt_list { diff --git a/pkg/sql/parser/testdata/backup_restore b/pkg/sql/parser/testdata/backup_restore index fb6fd33b9a20..a9409e1a45ef 100644 --- a/pkg/sql/parser/testdata/backup_restore +++ b/pkg/sql/parser/testdata/backup_restore @@ -315,36 +315,36 @@ BACKUP DATABASE _ TO ($1, $2) INCREMENTAL FROM 'baz' -- identifiers removed parse BACKUP foo TO 'bar' WITH ENCRYPTION_PASSPHRASE = 'secret', revision_history ---- -BACKUP TABLE foo TO 'bar' WITH revision_history, encryption_passphrase = '*****' -- normalized! -BACKUP TABLE (foo) TO ('bar') WITH revision_history, encryption_passphrase = '*****' -- fully parenthesized -BACKUP TABLE foo TO '_' WITH revision_history, encryption_passphrase = '*****' -- literals removed -BACKUP TABLE _ TO 'bar' WITH revision_history, encryption_passphrase = '*****' -- identifiers removed -BACKUP TABLE foo TO 'bar' WITH revision_history, encryption_passphrase = 'secret' -- passwords exposed +BACKUP TABLE foo TO 'bar' WITH revision_history = true, encryption_passphrase = '*****' -- normalized! +BACKUP TABLE (foo) TO ('bar') WITH revision_history = (true), encryption_passphrase = '*****' -- fully parenthesized +BACKUP TABLE foo TO '_' WITH revision_history = _, encryption_passphrase = '*****' -- literals removed +BACKUP TABLE _ TO 'bar' WITH revision_history = true, encryption_passphrase = '*****' -- identifiers removed +BACKUP TABLE foo TO 'bar' WITH revision_history = true, encryption_passphrase = 'secret' -- passwords exposed parse BACKUP foo TO 'bar' WITH KMS = ('foo', 'bar'), revision_history ---- -BACKUP TABLE foo TO 'bar' WITH revision_history, kms = ('foo', 'bar') -- normalized! -BACKUP TABLE (foo) TO ('bar') WITH revision_history, kms = (('foo'), ('bar')) -- fully parenthesized -BACKUP TABLE foo TO '_' WITH revision_history, kms = ('_', '_') -- literals removed -BACKUP TABLE _ TO 'bar' WITH revision_history, kms = ('foo', 'bar') -- identifiers removed +BACKUP TABLE foo TO 'bar' WITH revision_history = true, kms = ('foo', 'bar') -- normalized! +BACKUP TABLE (foo) TO ('bar') WITH revision_history = (true), kms = (('foo'), ('bar')) -- fully parenthesized +BACKUP TABLE foo TO '_' WITH revision_history = _, kms = ('_', '_') -- literals removed +BACKUP TABLE _ TO 'bar' WITH revision_history = true, kms = ('foo', 'bar') -- identifiers removed parse BACKUP foo TO 'bar' WITH OPTIONS (detached, ENCRYPTION_PASSPHRASE = 'secret', revision_history) ---- -BACKUP TABLE foo TO 'bar' WITH revision_history, encryption_passphrase = '*****', detached -- normalized! -BACKUP TABLE (foo) TO ('bar') WITH revision_history, encryption_passphrase = '*****', detached -- fully parenthesized -BACKUP TABLE foo TO '_' WITH revision_history, encryption_passphrase = '*****', detached -- literals removed -BACKUP TABLE _ TO 'bar' WITH revision_history, encryption_passphrase = '*****', detached -- identifiers removed -BACKUP TABLE foo TO 'bar' WITH revision_history, encryption_passphrase = 'secret', detached -- passwords exposed +BACKUP TABLE foo TO 'bar' WITH revision_history = true, encryption_passphrase = '*****', detached -- normalized! +BACKUP TABLE (foo) TO ('bar') WITH revision_history = (true), encryption_passphrase = '*****', detached -- fully parenthesized +BACKUP TABLE foo TO '_' WITH revision_history = _, encryption_passphrase = '*****', detached -- literals removed +BACKUP TABLE _ TO 'bar' WITH revision_history = true, encryption_passphrase = '*****', detached -- identifiers removed +BACKUP TABLE foo TO 'bar' WITH revision_history = true, encryption_passphrase = 'secret', detached -- passwords exposed parse BACKUP foo TO 'bar' WITH OPTIONS (detached, KMS = ('foo', 'bar'), revision_history) ---- -BACKUP TABLE foo TO 'bar' WITH revision_history, detached, kms = ('foo', 'bar') -- normalized! -BACKUP TABLE (foo) TO ('bar') WITH revision_history, detached, kms = (('foo'), ('bar')) -- fully parenthesized -BACKUP TABLE foo TO '_' WITH revision_history, detached, kms = ('_', '_') -- literals removed -BACKUP TABLE _ TO 'bar' WITH revision_history, detached, kms = ('foo', 'bar') -- identifiers removed +BACKUP TABLE foo TO 'bar' WITH revision_history = true, detached, kms = ('foo', 'bar') -- normalized! +BACKUP TABLE (foo) TO ('bar') WITH revision_history = (true), detached, kms = (('foo'), ('bar')) -- fully parenthesized +BACKUP TABLE foo TO '_' WITH revision_history = _, detached, kms = ('_', '_') -- literals removed +BACKUP TABLE _ TO 'bar' WITH revision_history = true, detached, kms = ('foo', 'bar') -- identifiers removed parse BACKUP TENANT 36 TO 'bar' @@ -740,10 +740,18 @@ RESTORE TENANT 123 FROM REPLICATION STREAM FROM $1 -- identifiers removed parse BACKUP TABLE foo TO 'bar' WITH revision_history, detached ---- -BACKUP TABLE foo TO 'bar' WITH revision_history, detached -BACKUP TABLE (foo) TO ('bar') WITH revision_history, detached -- fully parenthesized -BACKUP TABLE foo TO '_' WITH revision_history, detached -- literals removed -BACKUP TABLE _ TO 'bar' WITH revision_history, detached -- identifiers removed +BACKUP TABLE foo TO 'bar' WITH revision_history = true, detached -- normalized! +BACKUP TABLE (foo) TO ('bar') WITH revision_history = (true), detached -- fully parenthesized +BACKUP TABLE foo TO '_' WITH revision_history = _, detached -- literals removed +BACKUP TABLE _ TO 'bar' WITH revision_history = true, detached -- identifiers removed + +parse +BACKUP TABLE foo TO 'bar' WITH revision_history = $1, detached +---- +BACKUP TABLE foo TO 'bar' WITH revision_history = $1, detached +BACKUP TABLE (foo) TO ('bar') WITH revision_history = ($1), detached -- fully parenthesized +BACKUP TABLE foo TO '_' WITH revision_history = $1, detached -- literals removed +BACKUP TABLE _ TO 'bar' WITH revision_history = $1, detached -- identifiers removed parse RESTORE TABLE foo FROM 'bar' WITH skip_missing_foreign_keys, skip_missing_sequences, detached @@ -765,18 +773,43 @@ HINT: try \h BACKUP error BACKUP foo TO 'bar' WITH revision_history, revision_history ---- -at or near "revision_history": syntax error: revision_history option specified multiple times +at or near "EOF": syntax error: revision_history option specified multiple times DETAIL: source SQL: BACKUP foo TO 'bar' WITH revision_history, revision_history - ^ + ^ error BACKUP foo TO 'bar' WITH detached, revision_history, detached ---- -at or near "detached": syntax error: detached option specified multiple times +at or near "EOF": syntax error: detached option specified multiple times DETAIL: source SQL: BACKUP foo TO 'bar' WITH detached, revision_history, detached - ^ + ^ + +error +BACKUP foo TO 'bar' WITH revision_history=false, revision_history, detached +---- +at or near ",": syntax error: revision_history option specified multiple times +DETAIL: source SQL: +BACKUP foo TO 'bar' WITH revision_history=false, revision_history, detached + ^ + +error +BACKUP foo TO 'bar' WITH detached=true, revision_history, detached=true +---- +at or near "true": syntax error: detached option specified multiple times +DETAIL: source SQL: +BACKUP foo TO 'bar' WITH detached=true, revision_history, detached=true + ^ + +error +BACKUP foo TO 'bar' WITH detached=$1, revision_history +---- +at or near "1": syntax error +DETAIL: source SQL: +BACKUP foo TO 'bar' WITH detached=$1, revision_history + ^ +HINT: try \h BACKUP error RESTORE foo FROM 'bar' WITH key1, key2 = 'value' diff --git a/pkg/sql/parser/testdata/create_schedule b/pkg/sql/parser/testdata/create_schedule index bbf152f24848..5c712a9c24b9 100644 --- a/pkg/sql/parser/testdata/create_schedule +++ b/pkg/sql/parser/testdata/create_schedule @@ -41,23 +41,23 @@ CREATE SCHEDULE FOR BACKUP TABLE _, _, _ INTO 'bar' RECURRING '@daily' FULL BACK parse CREATE SCHEDULE FOR BACKUP TABLE foo, bar, buz INTO 'bar' WITH revision_history RECURRING '@daily' FULL BACKUP '@weekly' ---- -CREATE SCHEDULE FOR BACKUP TABLE foo, bar, buz INTO 'bar' WITH revision_history RECURRING '@daily' FULL BACKUP '@weekly' -CREATE SCHEDULE FOR BACKUP TABLE (foo), (bar), (buz) INTO ('bar') WITH revision_history RECURRING ('@daily') FULL BACKUP ('@weekly') -- fully parenthesized -CREATE SCHEDULE FOR BACKUP TABLE foo, bar, buz INTO '_' WITH revision_history RECURRING '_' FULL BACKUP '_' -- literals removed -CREATE SCHEDULE FOR BACKUP TABLE _, _, _ INTO 'bar' WITH revision_history RECURRING '@daily' FULL BACKUP '@weekly' -- identifiers removed +CREATE SCHEDULE FOR BACKUP TABLE foo, bar, buz INTO 'bar' WITH revision_history = true RECURRING '@daily' FULL BACKUP '@weekly' -- normalized! +CREATE SCHEDULE FOR BACKUP TABLE (foo), (bar), (buz) INTO ('bar') WITH revision_history = (true) RECURRING ('@daily') FULL BACKUP ('@weekly') -- fully parenthesized +CREATE SCHEDULE FOR BACKUP TABLE foo, bar, buz INTO '_' WITH revision_history = _ RECURRING '_' FULL BACKUP '_' -- literals removed +CREATE SCHEDULE FOR BACKUP TABLE _, _, _ INTO 'bar' WITH revision_history = true RECURRING '@daily' FULL BACKUP '@weekly' -- identifiers removed parse CREATE SCHEDULE FOR BACKUP INTO 'bar' WITH revision_history RECURRING '@daily' FULL BACKUP '@weekly' WITH SCHEDULE OPTIONS foo = 'bar' ---- -CREATE SCHEDULE FOR BACKUP INTO 'bar' WITH revision_history RECURRING '@daily' FULL BACKUP '@weekly' WITH SCHEDULE OPTIONS foo = 'bar' -CREATE SCHEDULE FOR BACKUP INTO ('bar') WITH revision_history RECURRING ('@daily') FULL BACKUP ('@weekly') WITH SCHEDULE OPTIONS foo = ('bar') -- fully parenthesized -CREATE SCHEDULE FOR BACKUP INTO '_' WITH revision_history RECURRING '_' FULL BACKUP '_' WITH SCHEDULE OPTIONS foo = '_' -- literals removed -CREATE SCHEDULE FOR BACKUP INTO 'bar' WITH revision_history RECURRING '@daily' FULL BACKUP '@weekly' WITH SCHEDULE OPTIONS _ = 'bar' -- identifiers removed +CREATE SCHEDULE FOR BACKUP INTO 'bar' WITH revision_history = true RECURRING '@daily' FULL BACKUP '@weekly' WITH SCHEDULE OPTIONS foo = 'bar' -- normalized! +CREATE SCHEDULE FOR BACKUP INTO ('bar') WITH revision_history = (true) RECURRING ('@daily') FULL BACKUP ('@weekly') WITH SCHEDULE OPTIONS foo = ('bar') -- fully parenthesized +CREATE SCHEDULE FOR BACKUP INTO '_' WITH revision_history = _ RECURRING '_' FULL BACKUP '_' WITH SCHEDULE OPTIONS foo = '_' -- literals removed +CREATE SCHEDULE FOR BACKUP INTO 'bar' WITH revision_history = true RECURRING '@daily' FULL BACKUP '@weekly' WITH SCHEDULE OPTIONS _ = 'bar' -- identifiers removed parse CREATE SCHEDULE IF NOT EXISTS 'baz' FOR BACKUP INTO 'bar' WITH revision_history RECURRING '@daily' FULL BACKUP '@weekly' WITH SCHEDULE OPTIONS first_run = 'now' ---- -CREATE SCHEDULE IF NOT EXISTS 'baz' FOR BACKUP INTO 'bar' WITH revision_history RECURRING '@daily' FULL BACKUP '@weekly' WITH SCHEDULE OPTIONS first_run = 'now' -CREATE SCHEDULE IF NOT EXISTS ('baz') FOR BACKUP INTO ('bar') WITH revision_history RECURRING ('@daily') FULL BACKUP ('@weekly') WITH SCHEDULE OPTIONS first_run = ('now') -- fully parenthesized -CREATE SCHEDULE IF NOT EXISTS '_' FOR BACKUP INTO '_' WITH revision_history RECURRING '_' FULL BACKUP '_' WITH SCHEDULE OPTIONS first_run = '_' -- literals removed -CREATE SCHEDULE IF NOT EXISTS 'baz' FOR BACKUP INTO 'bar' WITH revision_history RECURRING '@daily' FULL BACKUP '@weekly' WITH SCHEDULE OPTIONS _ = 'now' -- identifiers removed +CREATE SCHEDULE IF NOT EXISTS 'baz' FOR BACKUP INTO 'bar' WITH revision_history = true RECURRING '@daily' FULL BACKUP '@weekly' WITH SCHEDULE OPTIONS first_run = 'now' -- normalized! +CREATE SCHEDULE IF NOT EXISTS ('baz') FOR BACKUP INTO ('bar') WITH revision_history = (true) RECURRING ('@daily') FULL BACKUP ('@weekly') WITH SCHEDULE OPTIONS first_run = ('now') -- fully parenthesized +CREATE SCHEDULE IF NOT EXISTS '_' FOR BACKUP INTO '_' WITH revision_history = _ RECURRING '_' FULL BACKUP '_' WITH SCHEDULE OPTIONS first_run = '_' -- literals removed +CREATE SCHEDULE IF NOT EXISTS 'baz' FOR BACKUP INTO 'bar' WITH revision_history = true RECURRING '@daily' FULL BACKUP '@weekly' WITH SCHEDULE OPTIONS _ = 'now' -- identifiers removed diff --git a/pkg/sql/planhook.go b/pkg/sql/planhook.go index 24fb361598e6..510f8f283bba 100644 --- a/pkg/sql/planhook.go +++ b/pkg/sql/planhook.go @@ -88,6 +88,7 @@ type PlanHookState interface { DistSQLPlanner() *DistSQLPlanner LeaseMgr() *lease.Manager TypeAsString(ctx context.Context, e tree.Expr, op string) (func() (string, error), error) + TypeAsBool(ctx context.Context, e tree.Expr, op string) (func() (bool, error), error) TypeAsStringArray(ctx context.Context, e tree.Exprs, op string) (func() ([]string, error), error) TypeAsStringOpts( ctx context.Context, opts tree.KVOptions, optsValidate map[string]KVStringOptValidate, diff --git a/pkg/sql/planner.go b/pkg/sql/planner.go index d3e43b465309..c8569a921d03 100644 --- a/pkg/sql/planner.go +++ b/pkg/sql/planner.go @@ -653,6 +653,46 @@ func (p *planner) makeStringEvalFn(typedE tree.TypedExpr) func() (bool, string, } } +// TypeAsBool enforces (not hints) that the given expression typechecks as a +// string and returns a function that can be called to get the string value +// during (planNode).Start. +func (p *planner) TypeAsBool( + ctx context.Context, e tree.Expr, op string, +) (func() (bool, error), error) { + typedE, err := tree.TypeCheckAndRequire(ctx, e, &p.semaCtx, types.Bool, op) + if err != nil { + return nil, err + } + evalFn := p.makeBoolEvalFn(typedE) + return func() (bool, error) { + isNull, b, err := evalFn() + if err != nil { + return false, err + } + if isNull { + return false, errors.Errorf("expected string, got NULL") + } + return b, nil + }, nil +} + +func (p *planner) makeBoolEvalFn(typedE tree.TypedExpr) func() (bool, bool, error) { + return func() (bool, bool, error) { + d, err := eval.Expr(p.EvalContext(), typedE) + if err != nil { + return false, false, err + } + if d == tree.DNull { + return true, false, nil + } + b, ok := d.(*tree.DBool) + if !ok { + return false, false, errors.Errorf("failed to cast %T to bool", d) + } + return false, bool(*b), nil + } +} + // KVStringOptValidate indicates the requested validation of a TypeAsStringOpts // option. type KVStringOptValidate string diff --git a/pkg/sql/sem/tree/backup.go b/pkg/sql/sem/tree/backup.go index 3f58c5e52822..6a68a755148a 100644 --- a/pkg/sql/sem/tree/backup.go +++ b/pkg/sql/sem/tree/backup.go @@ -40,9 +40,9 @@ const ( // BackupOptions describes options for the BACKUP execution. type BackupOptions struct { - CaptureRevisionHistory bool + CaptureRevisionHistory Expr EncryptionPassphrase Expr - Detached bool + Detached *DBool EncryptionKMSURI StringOrPlaceholderOptList IncrementalStorage StringOrPlaceholderOptList } @@ -244,8 +244,9 @@ func (o *BackupOptions) Format(ctx *FmtCtx) { } addSep = true } - if o.CaptureRevisionHistory { - ctx.WriteString("revision_history") + if o.CaptureRevisionHistory != nil { + ctx.WriteString("revision_history = ") + ctx.FormatNode(o.CaptureRevisionHistory) addSep = true } @@ -259,7 +260,7 @@ func (o *BackupOptions) Format(ctx *FmtCtx) { } } - if o.Detached { + if o.Detached == DBoolTrue { maybeAddSep() ctx.WriteString("detached") } @@ -280,8 +281,8 @@ func (o *BackupOptions) Format(ctx *FmtCtx) { // CombineWith merges other backup options into this backup options struct. // An error is returned if the same option merged multiple times. func (o *BackupOptions) CombineWith(other *BackupOptions) error { - if o.CaptureRevisionHistory { - if other.CaptureRevisionHistory { + if o.CaptureRevisionHistory != nil { + if other.CaptureRevisionHistory != nil { return errors.New("revision_history option specified multiple times") } } else { @@ -294,8 +295,8 @@ func (o *BackupOptions) CombineWith(other *BackupOptions) error { return errors.New("encryption_passphrase specified multiple times") } - if o.Detached { - if other.Detached { + if o.Detached != nil { + if other.Detached != nil { return errors.New("detached option specified multiple times") } } else { diff --git a/pkg/sql/sem/tree/testdata/pretty/backup.align-deindent.golden.short b/pkg/sql/sem/tree/testdata/pretty/backup.align-deindent.golden.short index c13f618b381c..692781e68089 100644 --- a/pkg/sql/sem/tree/testdata/pretty/backup.align-deindent.golden.short +++ b/pkg/sql/sem/tree/testdata/pretty/backup.align-deindent.golden.short @@ -8,6 +8,6 @@ AS OF SYSTEM TIME '-10s' INCREMENTAL FROM 'gs://acme-co-backup/database-bank-2017-03-27-weekly', 'gs://acme-co-backup/database-bank-2017-03-28-nightly' - WITH revision_history + WITH revision_history = true diff --git a/pkg/sql/sem/tree/testdata/pretty/backup.align-only.golden.short b/pkg/sql/sem/tree/testdata/pretty/backup.align-only.golden.short index c13f618b381c..692781e68089 100644 --- a/pkg/sql/sem/tree/testdata/pretty/backup.align-only.golden.short +++ b/pkg/sql/sem/tree/testdata/pretty/backup.align-only.golden.short @@ -8,6 +8,6 @@ AS OF SYSTEM TIME '-10s' INCREMENTAL FROM 'gs://acme-co-backup/database-bank-2017-03-27-weekly', 'gs://acme-co-backup/database-bank-2017-03-28-nightly' - WITH revision_history + WITH revision_history = true diff --git a/pkg/sql/sem/tree/testdata/pretty/backup.ref.golden.short b/pkg/sql/sem/tree/testdata/pretty/backup.ref.golden.short index 7c4ec63d0c4d..e5d65d42f3ec 100644 --- a/pkg/sql/sem/tree/testdata/pretty/backup.ref.golden.short +++ b/pkg/sql/sem/tree/testdata/pretty/backup.ref.golden.short @@ -13,6 +13,6 @@ INCREMENTAL FROM 'gs://acme-co-backup/database-bank-2017-03-27-weekly', 'gs://acme-co-backup/database-bank-2017-03-28-nightly' WITH - revision_history + revision_history = true diff --git a/pkg/sql/sem/tree/testdata/pretty/backup.sql b/pkg/sql/sem/tree/testdata/pretty/backup.sql index ab103ec89142..0176117d0104 100644 --- a/pkg/sql/sem/tree/testdata/pretty/backup.sql +++ b/pkg/sql/sem/tree/testdata/pretty/backup.sql @@ -2,4 +2,4 @@ BACKUP DATABASE bank TO 'gs://acme-co-backup/database-bank-2017-03-29-nightly' AS OF SYSTEM TIME '-10s' INCREMENTAL FROM 'gs://acme-co-backup/database-bank-2017-03-27-weekly', 'gs://acme-co-backup/database-bank-2017-03-28-nightly' -WITH revision_history +WITH revision_history = true