Skip to content

Commit

Permalink
backupccl: explicitly parse SHOW BACKUP options
Browse files Browse the repository at this point in the history
Fixes: #82912

Release note (sql change): Previously, SHOW BACKUP options would get parsed as
`kv_options`, which meant that a user could not pass multiple values to a show
backup option, causing feature gaps in SHOW BACKUP relative to BACKUP and
RESTORE. This patch rewrites the show backup option parser, closing the
following feature gaps: 1. A user can now pass and check multiple KMS URIs in
SHOW BACKUP 2. A user can pass locality aware incremental_locations, allowing a
user to also pass the check_files parameter to a locality aware backup chain
that also specifies the backup incremental location.

Note that while this patch introduces a couple new words to the CRDB SQL
syntax, the same SHOW BACKUP options should remain documented, specifically:
- [public option] -> value
- AS_JSON -> N/A
- CHECK_FILES -> N/A
- INCREMENTAL_LOCATION -> string, with potentially multiple uris
- DEBUG_IDS -> N/A
- KMS -> string, with potentially multiple uris
- PRIVILEGES -> N/A
- ENCRYPTION_PASSPHRASE -> string
  • Loading branch information
msbutler committed Jan 27, 2023
1 parent 6af080b commit e6f47a0
Show file tree
Hide file tree
Showing 9 changed files with 427 additions and 165 deletions.
28 changes: 7 additions & 21 deletions docs/generated/sql/bnf/show_backup.bnf
Original file line number Diff line number Diff line change
@@ -1,23 +1,9 @@
show_backup_stmt ::=
'SHOW' 'BACKUPS' 'IN' location_opt_list
| 'SHOW' 'BACKUP' show_backup_details 'FROM' string_or_placeholder 'IN' string_or_placeholder_opt_list 'WITH' kv_option_list
| 'SHOW' 'BACKUP' show_backup_details 'FROM' string_or_placeholder 'IN' string_or_placeholder_opt_list 'WITH' 'OPTIONS' '(' kv_option_list ')'
| 'SHOW' 'BACKUP' show_backup_details 'FROM' string_or_placeholder 'IN' string_or_placeholder_opt_list
| 'SHOW' 'BACKUP' subdirectory 'IN' location_opt_list 'WITH' kv_option_list
| 'SHOW' 'BACKUP' subdirectory 'IN' location_opt_list 'WITH' 'OPTIONS' '(' kv_option_list ')'
| 'SHOW' 'BACKUP' subdirectory 'IN' location_opt_list
| 'SHOW' 'BACKUP' string_or_placeholder 'WITH' kv_option_list
| 'SHOW' 'BACKUP' string_or_placeholder 'WITH' 'OPTIONS' '(' kv_option_list ')'
| 'SHOW' 'BACKUP' string_or_placeholder
| 'SHOW' 'BACKUP' 'SCHEMAS' location 'WITH' kv_option_list
| 'SHOW' 'BACKUP' 'SCHEMAS' location 'WITH' 'OPTIONS' '(' kv_option_list ')'
| 'SHOW' 'BACKUP' 'SCHEMAS' location
| 'SHOW' 'BACKUP' 'FILES' string_or_placeholder 'WITH' kv_option_list
| 'SHOW' 'BACKUP' 'FILES' string_or_placeholder 'WITH' 'OPTIONS' '(' kv_option_list ')'
| 'SHOW' 'BACKUP' 'FILES' string_or_placeholder
| 'SHOW' 'BACKUP' 'RANGES' string_or_placeholder 'WITH' kv_option_list
| 'SHOW' 'BACKUP' 'RANGES' string_or_placeholder 'WITH' 'OPTIONS' '(' kv_option_list ')'
| 'SHOW' 'BACKUP' 'RANGES' string_or_placeholder
| 'SHOW' 'BACKUP' 'VALIDATE' string_or_placeholder 'WITH' kv_option_list
| 'SHOW' 'BACKUP' 'VALIDATE' string_or_placeholder 'WITH' 'OPTIONS' '(' kv_option_list ')'
| 'SHOW' 'BACKUP' 'VALIDATE' string_or_placeholder
| 'SHOW' 'BACKUP' show_backup_details 'FROM' string_or_placeholder 'IN' string_or_placeholder_opt_list opt_with_show_backup_options
| 'SHOW' 'BACKUP' subdirectory 'IN' location_opt_list opt_with_show_backup_options
| 'SHOW' 'BACKUP' string_or_placeholder opt_with_show_backup_options
| 'SHOW' 'BACKUP' 'SCHEMAS' location opt_with_show_backup_options
| 'SHOW' 'BACKUP' 'FILES' string_or_placeholder opt_with_show_backup_options
| 'SHOW' 'BACKUP' 'RANGES' string_or_placeholder opt_with_show_backup_options
| 'SHOW' 'BACKUP' 'VALIDATE' string_or_placeholder opt_with_show_backup_options
45 changes: 37 additions & 8 deletions docs/generated/sql/bnf/stmt_block.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -771,13 +771,13 @@ use_stmt ::=

show_backup_stmt ::=
'SHOW' 'BACKUPS' 'IN' string_or_placeholder_opt_list
| 'SHOW' 'BACKUP' show_backup_details 'FROM' string_or_placeholder 'IN' string_or_placeholder_opt_list opt_with_options
| 'SHOW' 'BACKUP' string_or_placeholder 'IN' string_or_placeholder_opt_list opt_with_options
| 'SHOW' 'BACKUP' string_or_placeholder opt_with_options
| 'SHOW' 'BACKUP' 'SCHEMAS' string_or_placeholder opt_with_options
| 'SHOW' 'BACKUP' 'FILES' string_or_placeholder opt_with_options
| 'SHOW' 'BACKUP' 'RANGES' string_or_placeholder opt_with_options
| 'SHOW' 'BACKUP' 'VALIDATE' string_or_placeholder opt_with_options
| 'SHOW' 'BACKUP' show_backup_details 'FROM' string_or_placeholder 'IN' string_or_placeholder_opt_list opt_with_show_backup_options
| 'SHOW' 'BACKUP' string_or_placeholder 'IN' string_or_placeholder_opt_list opt_with_show_backup_options
| 'SHOW' 'BACKUP' string_or_placeholder opt_with_show_backup_options
| 'SHOW' 'BACKUP' 'SCHEMAS' string_or_placeholder opt_with_show_backup_options
| 'SHOW' 'BACKUP' 'FILES' string_or_placeholder opt_with_show_backup_options
| 'SHOW' 'BACKUP' 'RANGES' string_or_placeholder opt_with_show_backup_options
| 'SHOW' 'BACKUP' 'VALIDATE' string_or_placeholder opt_with_show_backup_options

show_columns_stmt ::=
'SHOW' 'COLUMNS' 'FROM' table_name with_comment
Expand Down Expand Up @@ -1010,6 +1010,7 @@ unreserved_keyword ::=
| 'ALTER'
| 'ALWAYS'
| 'ASENSITIVE'
| 'AS_JSON'
| 'AT'
| 'ATOMIC'
| 'ATTRIBUTE'
Expand All @@ -1032,6 +1033,7 @@ unreserved_keyword ::=
| 'CAPABILITY'
| 'CASCADE'
| 'CHANGEFEED'
| 'CHECK_FILES'
| 'CLOSE'
| 'CLUSTER'
| 'COLUMNS'
Expand Down Expand Up @@ -1069,7 +1071,9 @@ unreserved_keyword ::=
| 'DATABASES'
| 'DAY'
| 'DEALLOCATE'
| 'DEBUG_IDS'
| 'DEBUG_PAUSE_ON'
| 'DEBUG_DUMP_METADATA_SST'
| 'DECLARE'
| 'DELETE'
| 'DEFAULTS'
Expand All @@ -1087,6 +1091,7 @@ unreserved_keyword ::=
| 'ENCODING'
| 'ENCRYPTED'
| 'ENCRYPTION_PASSPHRASE'
| 'ENCRYPTION_INFO_DIR'
| 'ENUM'
| 'ENUMS'
| 'ESCAPE'
Expand Down Expand Up @@ -1901,6 +1906,11 @@ show_backup_details ::=
| 'RANGES'
| 'VALIDATE'

opt_with_show_backup_options ::=
'WITH' show_backup_options_list
| 'WITH' 'OPTIONS' '(' show_backup_options_list ')'
|

with_comment ::=
'WITH' 'COMMENT'
|
Expand Down Expand Up @@ -2642,6 +2652,9 @@ extra_var_value ::=
'ON'
| cockroachdb_extra_reserved_keyword

show_backup_options_list ::=
( show_backup_options ) ( ( ',' show_backup_options ) )*

targets_roles ::=
'ROLE' role_spec_list
| 'SCHEMA' schema_name_list
Expand Down Expand Up @@ -3181,6 +3194,17 @@ for_locking_item ::=
var_list ::=
( var_value ) ( ( ',' var_value ) )*

show_backup_options ::=
'AS_JSON'
| 'CHECK_FILES'
| 'DEBUG_IDS'
| 'INCREMENTAL_LOCATION' '=' string_or_placeholder_opt_list
| 'KMS' '=' string_or_placeholder_opt_list
| 'ENCRYPTION_PASSPHRASE' '=' string_or_placeholder
| 'PRIVILEGES'
| 'ENCRYPTION_INFO_DIR' '=' string_or_placeholder
| 'DEBUG_DUMP_METADATA_SST'

schema_wildcard ::=
wildcard_pattern

Expand Down Expand Up @@ -3382,11 +3406,16 @@ family_name ::=
name

bare_label_keywords ::=
'ATOMIC'
'AS_JSON'
| 'ATOMIC'
| 'CALLED'
| 'COST'
| 'CHECK_FILES'
| 'DEBUG_IDS'
| 'DEBUG_DUMP_METADATA_SST'
| 'DEFINER'
| 'DEPENDS'
| 'ENCRYPTION_INFO_DIR'
| 'EXTERNAL'
| 'IMMUTABLE'
| 'INPUT'
Expand Down
8 changes: 0 additions & 8 deletions pkg/ccl/backupccl/backup_planning.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,6 @@ import (
)

const (
backupOptRevisionHistory = "revision_history"
backupOptWithPrivileges = "privileges"
backupOptAsJSON = "as_json"
backupOptWithDebugIDs = "debug_ids"
backupOptIncStorage = "incremental_location"
backupOptDebugMetadataSST = "debug_dump_metadata_sst"
backupOptEncDir = "encryption_info_dir"
backupOptCheckFiles = "check_files"
// backupPartitionDescriptorPrefix is the file name prefix for serialized
// BackupPartitionDescriptor protos.
backupPartitionDescriptorPrefix = "BACKUP_PART"
Expand Down
28 changes: 13 additions & 15 deletions pkg/ccl/backupccl/backup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4253,9 +4253,8 @@ func TestEncryptedBackup(t *testing.T) {

sqlDB.Exec(t, fmt.Sprintf(`SHOW BACKUP $1 WITH %s`, encryptionOption), backupLoc1)

sqlDB.Exec(t, fmt.Sprintf(`SHOW BACKUP $1 WITH %s,encryption_info_dir='%s'`,
encryptionOption,
backupLoc1),
sqlDB.Exec(t, fmt.Sprintf(`SHOW BACKUP $1 WITH %s, encryption_info_dir = '%s'`,
encryptionOption, backupLoc1),
backupLoc1inc)

var expectedShowError string
Expand Down Expand Up @@ -4327,7 +4326,7 @@ func TestRegionalKMSEncryptedBackup(t *testing.T) {
backupLoc1 := localFoo + "/x?COCKROACH_LOCALITY=default"
backupLoc2 := localFoo + "/x2?COCKROACH_LOCALITY=" + url.QueryEscape("dc=dc1")

sqlDB.Exec(t, fmt.Sprintf(`BACKUP TO ($1, $2) WITH %s`,
sqlDB.Exec(t, fmt.Sprintf(`BACKUP INTO ($1, $2) WITH %s`,
concatMultiRegionKMSURIs(multiRegionKMSURIs)), backupLoc1,
backupLoc2)

Expand All @@ -4339,14 +4338,18 @@ func TestRegionalKMSEncryptedBackup(t *testing.T) {

checkBackupFilesEncrypted(t, rawDir)

sqlDB.Exec(t, fmt.Sprintf(`SHOW BACKUP $1 WITH KMS='%s'`, multiRegionKMSURIs[0]),
// check that SHOW BACKUP is valid when a single kms uri is provided and when multiple are
sqlDB.Exec(t, fmt.Sprintf(`SHOW BACKUP FROM LATEST IN $1 WITH KMS='%s'`, multiRegionKMSURIs[0]),
backupLoc1)

sqlDB.Exec(t, fmt.Sprintf(`SHOW BACKUP FROM LATEST IN ($1, $2) WITH %s`,
concatMultiRegionKMSURIs(multiRegionKMSURIs)), backupLoc1, backupLoc2)

// Attempt to RESTORE using each of the regional KMSs independently.
for _, uri := range multiRegionKMSURIs {
sqlDB.Exec(t, `DROP DATABASE neverappears CASCADE`)

sqlDB.Exec(t, fmt.Sprintf(`RESTORE DATABASE neverappears FROM ($1, $2) WITH %s`,
sqlDB.Exec(t, fmt.Sprintf(`RESTORE DATABASE neverappears FROM LATEST IN ($1, $2) WITH %s`,
concatMultiRegionKMSURIs([]string{uri})), backupLoc1, backupLoc2)

sqlDB.CheckQueryResults(t, `SHOW EXPERIMENTAL_FINGERPRINTS FROM TABLE neverappears.neverappears`, before)
Expand Down Expand Up @@ -9896,18 +9899,13 @@ func TestBackupRestoreSeparateExplicitIsDefault(t *testing.T) {
sqlDB.Exec(t, fmt.Sprintf("SHOW BACKUPS IN %s", dest))
sqlDB.Exec(t, fmt.Sprintf("SHOW BACKUP FROM LATEST IN %s", dest))

// Locality aware show backups will eventually fail if not all localities are provided,
// but for now, they're ok.
if len(br.dest) > 1 {
// Locality aware show backups will eventually fail if not all localities are provided,
// but for now, they're ok.
sqlDB.Exec(t, fmt.Sprintf("SHOW BACKUP FROM LATEST IN %s", br.dest[1]))

errorMsg := "SHOW BACKUP on locality aware backups using incremental_location is not" +
" supported yet"
sqlDB.ExpectErr(t, errorMsg, fmt.Sprintf("SHOW BACKUP FROM LATEST IN %s WITH incremental_location= %s", dest, br.inc[0]))
} else {
// non locality aware show backup with incremental_location should work!
sqlDB.Exec(t, fmt.Sprintf("SHOW BACKUP FROM LATEST IN %s WITH incremental_location= %s", dest, inc))
}

sqlDB.Exec(t, fmt.Sprintf("SHOW BACKUP FROM LATEST IN %s WITH incremental_location= %s", dest, inc))
}
sir := fmt.Sprintf("RESTORE DATABASE fkdb FROM LATEST IN %s WITH new_db_name = 'inc_fkdb', incremental_location = %s", dest, inc)
sqlDB.Exec(t, sir)
Expand Down
Loading

0 comments on commit e6f47a0

Please sign in to comment.