Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[backupccl] Use Expr for backup's Detached and Revision History options #85146

Merged
merged 1 commit into from
Jul 29, 2022
Merged

[backupccl] Use Expr for backup's Detached and Revision History options #85146

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/generated/sql/bnf/stmt_block.bnf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
26 changes: 20 additions & 6 deletions pkg/ccl/backupccl/backup_planning.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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")
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/backupccl/backup_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
},
)
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/backupccl/create_scheduled_backup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
28 changes: 14 additions & 14 deletions pkg/ccl/backupccl/create_scheduled_backup_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
Expand Down Expand Up @@ -459,15 +459,15 @@ 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,
},
{
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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
},
},
Expand All @@ -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,
},
},
Expand All @@ -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)

Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions pkg/ccl/backupccl/schedule_exec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down
7 changes: 6 additions & 1 deletion pkg/internal/sqlsmith/bulkio.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
16 changes: 14 additions & 2 deletions pkg/sql/parser/sql.y
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
85 changes: 59 additions & 26 deletions pkg/sql/parser/testdata/backup_restore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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
Expand All @@ -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
dt marked this conversation as resolved.
Show resolved Hide resolved
----
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'
Expand Down
Loading