ccl: syntax change restore option strip_localities to remove_regions

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.
annrpom · Sep 27, 2023 · 101b569 · 101b569
1 parent ed6b6d7
commit 101b569
Show file tree

Hide file tree

Showing 12 changed files with 50 additions and 50 deletions.
diff --git a/docs/generated/sql/bnf/restore_options.bnf b/docs/generated/sql/bnf/restore_options.bnf
@@ -21,4 +21,4 @@ restore_options ::=
 	| 'UNSAFE_RESTORE_INCOMPATIBLE_VERSION'
 	| 'EXECUTION' 'LOCALITY' '=' string_or_placeholder
 	| 'EXPERIMENTAL' 'DEFERRED' 'COPY'
-	| 'STRIP_LOCALITIES'
+	| 'REMOVE_REGIONS'
diff --git a/docs/generated/sql/bnf/stmt_block.bnf b/docs/generated/sql/bnf/stmt_block.bnf
@@ -1332,6 +1332,7 @@ unreserved_keyword ::=
 	| 'RELATIVE'
 	| 'RELEASE'
 	| 'RELOCATE'
+	| 'REMOVE_REGIONS'
 	| 'RENAME'
 	| 'REPEATABLE'
 	| 'REPLACE'
@@ -1394,7 +1395,6 @@ unreserved_keyword ::=
 	| 'SKIP_MISSING_SEQUENCE_OWNERS'
 	| 'SKIP_MISSING_VIEWS'
 	| 'SKIP_MISSING_UDFS'
-	| 'STRIP_LOCALITIES'
 	| 'SNAPSHOT'
 	| 'SPLIT'
 	| 'SQL'
@@ -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 ) )*
@@ -3918,6 +3918,7 @@ bare_label_keywords ::=
 	| 'RELATIVE'
 	| 'RELEASE'
 	| 'RELOCATE'
+	| 'REMOVE_REGIONS'
 	| 'RENAME'
 	| 'REPEATABLE'
 	| 'REPLACE'
@@ -3984,7 +3985,6 @@ bare_label_keywords ::=
 	| 'SKIP_MISSING_SEQUENCE_OWNERS'
 	| 'SKIP_MISSING_UDFS'
 	| 'SKIP_MISSING_VIEWS'
-	| 'STRIP_LOCALITIES'
 	| 'SMALLINT'
 	| 'SNAPSHOT'
 	| 'SOME'

diff --git a/pkg/ccl/backupccl/restore_job.go b/pkg/ccl/backupccl/restore_job.go
@@ -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
@@ -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

diff --git a/pkg/ccl/backupccl/restore_multiregion_rbr_test.go b/pkg/ccl/backupccl/restore_multiregion_rbr_test.go
@@ -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)()
@@ -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)
 	}
 

diff --git a/pkg/ccl/backupccl/restore_planning.go b/pkg/ccl/backupccl/restore_planning.go
@@ -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 {
@@ -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
 		}
@@ -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{

diff --git a/pkg/ccl/backupccl/testdata/backup-restore/restore-regionless-alter-region b/pkg/ccl/backupccl/testdata/backup-restore/restore-regionless-alter-region
@@ -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
@@ -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
@@ -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
@@ -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

diff --git a/pkg/ccl/backupccl/testdata/backup-restore/restore-regionless-on-regionless b/pkg/ccl/backupccl/testdata/backup-restore/restore-regionless-on-regionless
@@ -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
@@ -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
@@ -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
@@ -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

diff --git a/pkg/ccl/backupccl/testdata/backup-restore/restore-regionless-regional-by-row b/pkg/ccl/backupccl/testdata/backup-restore/restore-regionless-regional-by-row
@@ -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
@@ -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
@@ -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"
 
@@ -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

diff --git a/pkg/jobs/jobspb/jobs.proto b/pkg/jobs/jobspb/jobs.proto
@@ -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.
 }

diff --git a/pkg/sql/parser/sql.y b/pkg/sql/parser/sql.y
@@ -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
@@ -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:
@@ -16891,6 +16891,7 @@ unreserved_keyword:
 | RELATIVE
 | RELEASE
 | RELOCATE
+| REMOVE_REGIONS
 | RENAME
 | REPEATABLE
 | REPLACE
@@ -16953,7 +16954,6 @@ unreserved_keyword:
 | SKIP_MISSING_SEQUENCE_OWNERS
 | SKIP_MISSING_VIEWS
 | SKIP_MISSING_UDFS
-| STRIP_LOCALITIES
 | SNAPSHOT
 | SPLIT
 | SQL
@@ -17446,6 +17446,7 @@ bare_label_keywords:
 | RELATIVE
 | RELEASE
 | RELOCATE
+| REMOVE_REGIONS
 | RENAME
 | REPEATABLE
 | REPLACE
@@ -17512,7 +17513,6 @@ bare_label_keywords:
 | SKIP_MISSING_SEQUENCE_OWNERS
 | SKIP_MISSING_UDFS
 | SKIP_MISSING_VIEWS
-| STRIP_LOCALITIES
 | SMALLINT
 | SNAPSHOT
 | SOME

diff --git a/pkg/sql/parser/testdata/backup_restore b/pkg/sql/parser/testdata/backup_restore
@@ -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