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.

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

ccl: syntax change restore option strip_localities to remove_regions #111356

Merged
merged 1 commit into from
Sep 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion docs/generated/sql/bnf/restore_options.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ restore_options ::=
| 'UNSAFE_RESTORE_INCOMPATIBLE_VERSION'
| 'EXECUTION' 'LOCALITY' '=' string_or_placeholder
| 'EXPERIMENTAL' 'DEFERRED' 'COPY'
| 'STRIP_LOCALITIES'
| 'REMOVE_REGIONS'
6 changes: 3 additions & 3 deletions docs/generated/sql/bnf/stmt_block.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -1332,6 +1332,7 @@ unreserved_keyword ::=
| 'RELATIVE'
| 'RELEASE'
| 'RELOCATE'
| 'REMOVE_REGIONS'
| 'RENAME'
| 'REPEATABLE'
| 'REPLACE'
Expand Down Expand Up @@ -1394,7 +1395,6 @@ unreserved_keyword ::=
| 'SKIP_MISSING_SEQUENCE_OWNERS'
| 'SKIP_MISSING_VIEWS'
| 'SKIP_MISSING_UDFS'
| 'STRIP_LOCALITIES'
| 'SNAPSHOT'
| 'SPLIT'
| 'SQL'
Expand Down Expand Up @@ -2675,7 +2675,7 @@ restore_options ::=
| 'UNSAFE_RESTORE_INCOMPATIBLE_VERSION'
| 'EXECUTION' 'LOCALITY' '=' string_or_placeholder
| 'EXPERIMENTAL' 'DEFERRED' 'COPY'
| 'STRIP_LOCALITIES'
| 'REMOVE_REGIONS'

scrub_option_list ::=
( scrub_option ) ( ( ',' scrub_option ) )*
Expand Down Expand Up @@ -3918,6 +3918,7 @@ bare_label_keywords ::=
| 'RELATIVE'
| 'RELEASE'
| 'RELOCATE'
| 'REMOVE_REGIONS'
| 'RENAME'
| 'REPEATABLE'
| 'REPLACE'
Expand Down Expand Up @@ -3984,7 +3985,6 @@ bare_label_keywords ::=
| 'SKIP_MISSING_SEQUENCE_OWNERS'
| 'SKIP_MISSING_UDFS'
| 'SKIP_MISSING_VIEWS'
| 'STRIP_LOCALITIES'
| 'SMALLINT'
| 'SNAPSHOT'
| 'SOME'
Expand Down
4 changes: 2 additions & 2 deletions pkg/ccl/backupccl/restore_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -1053,7 +1053,7 @@ func createImportingDescriptors(
typesByID[types[i].GetID()] = types[i]
}

if details.StripLocalities {
if details.RemoveRegions {
// Can't restore multi-region tables into non-multi-region database
for _, t := range tables {
t.TableDesc().LocalityConfig = nil
Expand Down Expand Up @@ -1093,7 +1093,7 @@ func createImportingDescriptors(
// we need to make sure that multi-region databases no longer get tagged as such - meaning
// that we want to change the TypeDescriptor_MULTIREGION_ENUM to a normal enum. We `continue`
// to skip the multi-region work below.
if details.StripLocalities {
if details.RemoveRegions {
t.TypeDesc().Kind = descpb.TypeDescriptor_ENUM
t.TypeDesc().RegionConfig = nil
continue
Expand Down
4 changes: 2 additions & 2 deletions pkg/ccl/backupccl/restore_multiregion_rbr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import (
// The goal of this test is to ensure that if a user ever performed a
// regionless restore where the backed-up target has a regional by row table,
// they would be able to get themselves out of a stuck state without needing
// an enterprise license (in addition to testing the ability to use strip_localities
// an enterprise license (in addition to testing the ability to use remove_regions
// without said license).
func TestMultiRegionRegionlessRestoreNoLicense(t *testing.T) {
defer leaktest.AfterTest(t)()
Expand Down Expand Up @@ -81,7 +81,7 @@ func TestMultiRegionRegionlessRestoreNoLicense(t *testing.T) {
defer sqlTC.Stopper().Stop(ctx)
sqlDB := sqlutils.MakeSQLRunner(sqlTC.Conns[0])

if err := backuptestutils.VerifyBackupRestoreStatementResult(t, sqlDB, `RESTORE DATABASE d FROM LATEST IN $1 WITH strip_localities`, localFoo); err != nil {
if err := backuptestutils.VerifyBackupRestoreStatementResult(t, sqlDB, `RESTORE DATABASE d FROM LATEST IN $1 WITH remove_regions`, localFoo); err != nil {
t.Fatal(err)
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/ccl/backupccl/restore_planning.go
Original file line number Diff line number Diff line change
Expand Up @@ -1081,7 +1081,7 @@ func resolveOptionsForRestoreJobDescription(
VerifyData: opts.VerifyData,
UnsafeRestoreIncompatibleVersion: opts.UnsafeRestoreIncompatibleVersion,
ExperimentalOnline: opts.ExperimentalOnline,
StripLocalities: opts.StripLocalities,
RemoveRegions: opts.RemoveRegions,
}

if opts.EncryptionPassphrase != nil {
Expand Down Expand Up @@ -2075,7 +2075,7 @@ func doRestorePlan(
// If we are stripping localities, wipe tables of their LocalityConfig before we allocate
// descriptor rewrites - as validation in remapTables compares these tables with the non-mr
// database and fails otherwise
if restoreStmt.Options.StripLocalities {
if restoreStmt.Options.RemoveRegions {
for _, t := range filteredTablesByID {
t.TableDesc().LocalityConfig = nil
}
Expand Down Expand Up @@ -2197,7 +2197,7 @@ func doRestorePlan(
SkipLocalitiesCheck: restoreStmt.Options.SkipLocalitiesCheck,
ExecutionLocality: execLocality,
ExperimentalOnline: restoreStmt.Options.ExperimentalOnline,
StripLocalities: restoreStmt.Options.StripLocalities,
RemoveRegions: restoreStmt.Options.RemoveRegions,
}

jr := jobs.Record{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ subtest end
new-cluster name=s2 share-io-dir=s1 allow-implicit-access localities=us-east-1
----

# test cluster restore with strip_localities
# test cluster restore with remove_regions
subtest restore_regionless_on_single_region_cluster

query-sql
Expand All @@ -56,7 +56,7 @@ postgres root <nil> <nil> {} <nil>
system node <nil> <nil> {} <nil>

exec-sql
RESTORE FROM LATEST IN 'nodelocal://1/cluster_backup/' WITH strip_localities;
RESTORE FROM LATEST IN 'nodelocal://1/cluster_backup/' WITH remove_regions;
----

# check cluster's regions
Expand Down Expand Up @@ -167,15 +167,15 @@ subtest end

subtest end

# test db restore with strip_localities
# test db restore with remove_regions
subtest restore_regionless_on_single_region_db

exec-sql
DROP DATABASE d;
----

exec-sql
RESTORE DATABASE d FROM LATEST IN 'nodelocal://1/database_backup/' WITH strip_localities;
RESTORE DATABASE d FROM LATEST IN 'nodelocal://1/database_backup/' WITH remove_regions;
----

# check to see if restored database, d, shows up
Expand Down Expand Up @@ -270,15 +270,15 @@ subtest end

subtest end

# test table restore with strip_localities
# test table restore with remove_regions
subtest restore_regionless_on_single_region_table

exec-sql
DROP TABLE d.t;
----

exec-sql
RESTORE TABLE d.t FROM LATEST IN 'nodelocal://1/table_backup/' WITH strip_localities;
RESTORE TABLE d.t FROM LATEST IN 'nodelocal://1/table_backup/' WITH remove_regions;
----

query-sql
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ subtest end
new-cluster name=s2 share-io-dir=s1 allow-implicit-access
----

# test cluster restore with strip_localities
# test cluster restore with remove_regions
subtest restore_regionless_on_regionless_cluster

query-sql
Expand All @@ -55,7 +55,7 @@ postgres root <nil> <nil> {} <nil>
system node <nil> <nil> {} <nil>

exec-sql
RESTORE FROM LATEST IN 'nodelocal://1/cluster_backup/' WITH strip_localities;
RESTORE FROM LATEST IN 'nodelocal://1/cluster_backup/' WITH remove_regions;
----

# check cluster's regions
Expand Down Expand Up @@ -100,15 +100,15 @@ SELECT * FROM d.t;

subtest end

# test db restore with strip_localities
# test db restore with remove_regions
subtest restore_regionless_on_regionless_db

exec-sql
DROP DATABASE d;
----

exec-sql
RESTORE DATABASE d FROM LATEST IN 'nodelocal://1/database_backup/' WITH strip_localities;
RESTORE DATABASE d FROM LATEST IN 'nodelocal://1/database_backup/' WITH remove_regions;
----

# check to see if restored database, d, shows up
Expand Down Expand Up @@ -148,15 +148,15 @@ SELECT * FROM d.t;

subtest end

# test table restore with strip_localities
# test table restore with remove_regions
subtest restore_regionless_on_regionless_table

exec-sql
DROP TABLE d.t;
----

exec-sql
RESTORE TABLE d.t FROM LATEST IN 'nodelocal://1/table_backup/' WITH strip_localities;
RESTORE TABLE d.t FROM LATEST IN 'nodelocal://1/table_backup/' WITH remove_regions;
----

query-sql
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ new-cluster name=s2 share-io-dir=s1 allow-implicit-access localities=us-east-1
subtest restore_regionless_cluster_rbr

exec-sql
RESTORE FROM LATEST IN 'nodelocal://1/rbr_cluster_backup/' WITH strip_localities;
RESTORE FROM LATEST IN 'nodelocal://1/rbr_cluster_backup/' WITH remove_regions;
----

# check cluster's regions
Expand Down Expand Up @@ -129,7 +129,7 @@ DROP DATABASE d;
----

exec-sql
RESTORE DATABASE d FROM LATEST IN 'nodelocal://1/rbr_database_backup/' WITH strip_localities;
RESTORE DATABASE d FROM LATEST IN 'nodelocal://1/rbr_database_backup/' WITH remove_regions;
----

# check to see if restored database, d, shows up
Expand Down Expand Up @@ -191,7 +191,7 @@ DROP TABLE d.t;
----

exec-sql
RESTORE TABLE d.t FROM LATEST IN 'nodelocal://1/rbr_table_backup/' WITH strip_localities;
RESTORE TABLE d.t FROM LATEST IN 'nodelocal://1/rbr_table_backup/' WITH remove_regions;
----
pq: "crdb_internal_region" is not compatible with type "crdb_internal_region" existing in cluster: "crdb_internal_region" of type "ENUM" is not compatible with type "MULTIREGION_ENUM"

Expand All @@ -201,7 +201,7 @@ DROP TYPE d.public.crdb_internal_region;
----

exec-sql
RESTORE TABLE d.t FROM LATEST IN 'nodelocal://1/rbr_table_backup/' WITH strip_localities;
RESTORE TABLE d.t FROM LATEST IN 'nodelocal://1/rbr_table_backup/' WITH remove_regions;
----

query-sql
Expand Down
4 changes: 2 additions & 2 deletions pkg/jobs/jobspb/jobs.proto
Original file line number Diff line number Diff line change
Expand Up @@ -489,8 +489,8 @@ message RestoreDetails {
// online restore.
repeated roachpb.Span download_spans = 32 [(gogoproto.nullable) = false];

// Strips localities
bool StripLocalities = 33;
// Removes regions.
bool RemoveRegions = 33;

// NEXT ID: 34.
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/sql/parser/sql.y
Original file line number Diff line number Diff line change
Expand Up @@ -987,15 +987,15 @@ func (u *sqlSymUnion) beginTransaction() *tree.BeginTransaction {

%token <str> RANGE RANGES READ REAL REASON REASSIGN RECURSIVE RECURRING REDACT REF REFERENCES REFRESH
%token <str> REGCLASS REGION REGIONAL REGIONS REGNAMESPACE REGPROC REGPROCEDURE REGROLE REGTYPE REINDEX
%token <str> RELATIVE RELOCATE REMOVE_PATH RENAME REPEATABLE REPLACE REPLICATION
%token <str> RELATIVE RELOCATE REMOVE_PATH REMOVE_REGIONS RENAME REPEATABLE REPLACE REPLICATION
%token <str> RELEASE RESET RESTART RESTORE RESTRICT RESTRICTED RESUME RETENTION RETURNING RETURN RETURNS RETRY REVISION_HISTORY
%token <str> REVOKE RIGHT ROLE ROLES ROLLBACK ROLLUP ROUTINES ROW ROWS RSHIFT RULE RUNNING

%token <str> SAVEPOINT SCANS SCATTER SCHEDULE SCHEDULES SCROLL SCHEMA SCHEMA_ONLY SCHEMAS SCRUB
%token <str> SEARCH SECOND SECONDARY SECURITY SELECT SEQUENCE SEQUENCES
%token <str> SERIALIZABLE SERVER SERVICE SESSION SESSIONS SESSION_USER SET SETOF SETS SETTING SETTINGS
%token <str> SHARE SHARED SHOW SIMILAR SIMPLE SIZE SKIP SKIP_LOCALITIES_CHECK SKIP_MISSING_FOREIGN_KEYS
%token <str> SKIP_MISSING_SEQUENCES SKIP_MISSING_SEQUENCE_OWNERS SKIP_MISSING_VIEWS SKIP_MISSING_UDFS STRIP_LOCALITIES SMALLINT SMALLSERIAL
%token <str> SKIP_MISSING_SEQUENCES SKIP_MISSING_SEQUENCE_OWNERS SKIP_MISSING_VIEWS SKIP_MISSING_UDFS SMALLINT SMALLSERIAL
%token <str> SNAPSHOT SOME SPLIT SQL SQLLOGIN
%token <str> STABLE START STATE STATISTICS STATUS STDIN STDOUT STOP STREAM STRICT STRING STORAGE STORE STORED STORING SUBSTRING SUPER
%token <str> SUPPORT SURVIVE SURVIVAL SYMMETRIC SYNTAX SYSTEM SQRT SUBSCRIPTION STATEMENTS
Expand Down Expand Up @@ -3845,9 +3845,9 @@ restore_options:
{
$$.val = &tree.RestoreOptions{ExperimentalOnline: true}
}
| STRIP_LOCALITIES
| REMOVE_REGIONS
{
$$.val = &tree.RestoreOptions{StripLocalities: true, SkipLocalitiesCheck: true}
$$.val = &tree.RestoreOptions{RemoveRegions: true, SkipLocalitiesCheck: true}
}

virtual_cluster_opt:
Expand Down Expand Up @@ -16891,6 +16891,7 @@ unreserved_keyword:
| RELATIVE
| RELEASE
| RELOCATE
| REMOVE_REGIONS
| RENAME
| REPEATABLE
| REPLACE
Expand Down Expand Up @@ -16953,7 +16954,6 @@ unreserved_keyword:
| SKIP_MISSING_SEQUENCE_OWNERS
| SKIP_MISSING_VIEWS
| SKIP_MISSING_UDFS
| STRIP_LOCALITIES
| SNAPSHOT
| SPLIT
| SQL
Expand Down Expand Up @@ -17446,6 +17446,7 @@ bare_label_keywords:
| RELATIVE
| RELEASE
| RELOCATE
| REMOVE_REGIONS
| RENAME
| REPEATABLE
| REPLACE
Expand Down Expand Up @@ -17512,7 +17513,6 @@ bare_label_keywords:
| SKIP_MISSING_SEQUENCE_OWNERS
| SKIP_MISSING_UDFS
| SKIP_MISSING_VIEWS
| STRIP_LOCALITIES
| SMALLINT
| SNAPSHOT
| SOME
Expand Down
10 changes: 5 additions & 5 deletions pkg/sql/parser/testdata/backup_restore
Original file line number Diff line number Diff line change
Expand Up @@ -824,12 +824,12 @@ RESTORE TABLE foo FROM '_' WITH OPTIONS (skip_missing_foreign_keys, skip_missing
RESTORE TABLE _ FROM 'bar' WITH OPTIONS (skip_missing_foreign_keys, skip_missing_sequences, detached) -- identifiers removed

parse
RESTORE TABLE foo FROM 'bar' WITH strip_localities
RESTORE TABLE foo FROM 'bar' WITH remove_regions
----
RESTORE TABLE foo FROM 'bar' WITH OPTIONS (skip_localities_check, strip_localities) -- normalized!
RESTORE TABLE (foo) FROM ('bar') WITH OPTIONS (skip_localities_check, strip_localities) -- fully parenthesized
RESTORE TABLE foo FROM '_' WITH OPTIONS (skip_localities_check, strip_localities) -- literals removed
RESTORE TABLE _ FROM 'bar' WITH OPTIONS (skip_localities_check, strip_localities) -- identifiers removed
RESTORE TABLE foo FROM 'bar' WITH OPTIONS (skip_localities_check, remove_regions) -- normalized!
RESTORE TABLE (foo) FROM ('bar') WITH OPTIONS (skip_localities_check, remove_regions) -- fully parenthesized
RESTORE TABLE foo FROM '_' WITH OPTIONS (skip_localities_check, remove_regions) -- literals removed
RESTORE TABLE _ FROM 'bar' WITH OPTIONS (skip_localities_check, remove_regions) -- identifiers removed

parse
BACKUP INTO 'bar' WITH include_all_virtual_clusters = $1, detached
Expand Down
Loading