Skip to content

Commit

Permalink
ccl: syntax change restore option strip_localities to remove_regions
Browse files Browse the repository at this point in the history
This changes the syntax from newly added RESTORE option,
strip_localities, to remove_regions. The former was
inadequate because it was too similar to an existing
option, along with not being an accurately descriptive enough
name.

Epic: none
Informs: cockroachdb#111348
Part of: https://cockroachlabs.atlassian.net/browse/CRDB-29129

Release note (sql change): the syntax for RESTORE option,
strip_localities, is now remove_regions.
  • Loading branch information
annrpom committed Oct 5, 2023
1 parent 3b35d3d commit 0bf7dd5
Show file tree
Hide file tree
Showing 10 changed files with 34 additions and 34 deletions.
4 changes: 2 additions & 2 deletions docs/generated/sql/bnf/stmt_block.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -1304,6 +1304,7 @@ unreserved_keyword ::=
| 'RELATIVE'
| 'RELEASE'
| 'RELOCATE'
| 'REMOVE_REGIONS'
| 'RENAME'
| 'REPEATABLE'
| 'REPLACE'
Expand Down Expand Up @@ -1366,7 +1367,6 @@ unreserved_keyword ::=
| 'SKIP_MISSING_SEQUENCE_OWNERS'
| 'SKIP_MISSING_VIEWS'
| 'SKIP_MISSING_UDFS'
| 'STRIP_LOCALITIES'
| 'SNAPSHOT'
| 'SPLIT'
| 'SQL'
Expand Down Expand Up @@ -3853,6 +3853,7 @@ bare_label_keywords ::=
| 'RELATIVE'
| 'RELEASE'
| 'RELOCATE'
| 'REMOVE_REGIONS'
| 'RENAME'
| 'REPEATABLE'
| 'REPLACE'
Expand Down Expand Up @@ -3919,7 +3920,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 @@ -988,7 +988,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 @@ -1028,7 +1028,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
2 changes: 1 addition & 1 deletion pkg/ccl/backupccl/restore_planning.go
Original file line number Diff line number Diff line change
Expand Up @@ -1925,7 +1925,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
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
8 changes: 4 additions & 4 deletions pkg/sql/parser/sql.y
Original file line number Diff line number Diff line change
Expand Up @@ -969,15 +969,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 @@ -16666,6 +16666,7 @@ unreserved_keyword:
| RELATIVE
| RELEASE
| RELOCATE
| REMOVE_REGIONS
| RENAME
| REPEATABLE
| REPLACE
Expand Down Expand Up @@ -16728,7 +16729,6 @@ unreserved_keyword:
| SKIP_MISSING_SEQUENCE_OWNERS
| SKIP_MISSING_VIEWS
| SKIP_MISSING_UDFS
| STRIP_LOCALITIES
| SNAPSHOT
| SPLIT
| SQL
Expand Down Expand Up @@ -17216,6 +17216,7 @@ bare_label_keywords:
| RELATIVE
| RELEASE
| RELOCATE
| REMOVE_REGIONS
| RENAME
| REPEATABLE
| REPLACE
Expand Down Expand Up @@ -17282,7 +17283,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
4 changes: 2 additions & 2 deletions pkg/sql/sem/tree/backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -518,9 +518,9 @@ func (o *RestoreOptions) Format(ctx *FmtCtx) {
ctx.FormatNode(o.ExecutionLocality)
}

if o.StripLocalities {
if o.RemoveRegions {
maybeAddSep()
ctx.WriteString("strip_localities")
ctx.WriteString("remove_regions")
}
}

Expand Down

0 comments on commit 0bf7dd5

Please sign in to comment.