Merge cockroachdb#78165

78165: backupccl: add deprecation notice for `BACKUP TO` and `RESTORE FROM` without subdir r=msbutler a=adityamaru

backupccl: add deprecation notice for BACKUP TO

Informs: cockroachdb#78153

Release note (sql change): The `BACKUP TO` syntax
to take backups is deprecated, and will be removed in a future
release. Users are recommended to create a backup collection
using the `BACKUP INTO` syntax in our docs.

backupccl: add deprecation notice to RESTORE FROM without subdir
Informs: cockroachdb#78153

Release note (sql change): The `RESTORE FROM` syntax without an
explicit subdirectory pointing to a backup in a collection is deprecated,
and will be removed in a future release.
Users are recommended to use `RESTORE FROM <backup> IN <collection>` to
restore a particular backup in a collection.

backupccl: add datadriven tests for deprecation notice
Informs: cockroachdb#78153

Release note: None

Co-authored-by: Aditya Maru <[email protected]>

pkg/ccl/backupccl/backup_planning.go

@@ -45,6 +45,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
+	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgnotice"
 	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
@@ -592,6 +593,16 @@ func backupPlanHook(
 		return nil, nil, nil, false, err
 	}
 
+	// Deprecation notice for `BACKUP TO` syntax. Remove this once the syntax is
+	// deleted in 22.2.
+	if !backupStmt.Nested {
+		p.BufferClientNotice(ctx,
+			pgnotice.Newf("The `BACKUP TO` syntax will be removed in a future release, please"+
+				" switch over to using `BACKUP INTO` to create a backup collection: %s. "+
+				"Backups created using the `BACKUP TO` syntax may not be restoreable in the next major version release.",
+				"https://www.cockroachlabs.com/docs/stable/backup.html#considerations"))
+	}
+
 	var err error
 	subdirFn := func() (string, error) { return "", nil }
 	if backupStmt.Subdir != nil {

pkg/ccl/backupccl/restore_planning.go

@@ -1075,6 +1075,13 @@ func restorePlanHook(
 		if err != nil {
 			return nil, nil, nil, false, err
 		}
+	} else {
+		// Deprecation notice for non-colelction `RESTORE FROM` syntax. Remove this
+		// once the syntax is deleted in 22.2.
+		p.BufferClientNotice(ctx,
+			pgnotice.Newf("The `RESTORE FROM <backup>` syntax will be removed in a future release, please"+
+				" switch over to using `RESTORE FROM <backup> IN <collection>` to restore a particular backup from a collection: %s",
+				"https://www.cockroachlabs.com/docs/stable/restore.html#view-the-backup-subdirectories"))
 	}
 
 	var incStorageFn func() ([]string, error)

pkg/ccl/backupccl/testdata/backup-restore/backup-permissions

@@ -11,14 +11,14 @@ INSERT INTO d.t VALUES (1), (2), (3);
 
 # BACKUP is not allowed in a batch-statement.
 exec-sql
-BACKUP TO 'nodelocal://0/test-root/';
+BACKUP INTO 'nodelocal://0/test-root/';
 SELECT 1;
 ----
 pq: BACKUP cannot be used inside a multi-statement transaction without DETACHED option
 
 # Cluster backup should succeed as a root user.
 exec-sql
-BACKUP TO 'nodelocal://0/test-root/'
+BACKUP INTO 'nodelocal://0/test-root/'
 ----
 
 # Backups should succeed as a non-root user with admin role.
@@ -28,15 +28,15 @@ GRANT ADMIN TO testuser;
 ----
 
 exec-sql user=testuser
-BACKUP TO 'nodelocal://0/test-nonroot-cluster';
+BACKUP INTO 'nodelocal://0/test-nonroot-cluster';
 ----
 
 exec-sql user=testuser
-BACKUP DATABASE d TO 'nodelocal://0/test-nonroot-db';
+BACKUP DATABASE d INTO 'nodelocal://0/test-nonroot-db';
 ----
 
 exec-sql user=testuser
-BACKUP TABLE d.t TO 'nodelocal://0/test-nonroot-table';
+BACKUP TABLE d.t INTO 'nodelocal://0/test-nonroot-table';
 ----
 
 # Start a new cluster with the same IO dir.
@@ -75,7 +75,7 @@ GRANT CONNECT ON DATABASE d2 TO testuser;
 
 # Table backup as a non-admin user should have SELECT privileges.
 exec-sql user=testuser
-BACKUP TABLE d2.t TO 'nodelocal://0/d2-table'
+BACKUP TABLE d2.t INTO 'nodelocal://0/d2-table'
 ----
 pq: user testuser does not have SELECT privilege on relation t
 
@@ -90,7 +90,7 @@ CREATE SCHEMA sc2;
 
 # Schema backup as a non-admin user should have USAGE privileges.
 exec-sql user=testuser
-BACKUP DATABASE d2 TO 'nodelocal://0/d2-schema';
+BACKUP DATABASE d2 INTO 'nodelocal://0/d2-schema';
 ----
 pq: user testuser does not have USAGE privilege on schema sc2
 
@@ -108,7 +108,7 @@ REVOKE USAGE ON TYPE d2.greeting FROM public;
 
 # Type backup as a non-admin user should have USAGE privileges.
 exec-sql user=testuser
-BACKUP DATABASE d2 TO 'nodelocal://0/d2-schema';
+BACKUP DATABASE d2 INTO 'nodelocal://0/d2-schema';
 ----
 pq: user testuser does not have USAGE privilege on type greeting
 
@@ -118,11 +118,11 @@ GRANT USAGE ON TYPE d2.greeting TO testuser;
 
 # testuser should now have all the required privileges.
 exec-sql server=s2 user=testuser
-BACKUP DATABASE d2 TO 'nodelocal://0/d2';
+BACKUP DATABASE d2 INTO 'nodelocal://0/d2';
 ----
 
 exec-sql server=s2 user=testuser
-BACKUP TABLE d2.t TO 'nodelocal://0/d2-table';
+BACKUP TABLE d2.t INTO 'nodelocal://0/d2-table';
 ----
 
 exec-sql server=s2 user=testuser
@@ -147,7 +147,7 @@ SHOW BACKUP 'http://COCKROACH_TEST_HTTP_SERVER/'
 pq: only users with the admin role are allowed to SHOW BACKUP from the specified http URI
 
 exec-sql user=testuser
-BACKUP DATABASE d TO 'nodelocal://0/test3'
+BACKUP DATABASE d INTO 'nodelocal://0/test3'
 ----
 pq: only users with the admin role are allowed to BACKUP to the specified nodelocal URI
 

pkg/ccl/backupccl/testdata/backup-restore/column-families

@@ -29,15 +29,15 @@ SET CLUSTER SETTING bulkio.backup.merge_file_buffer_size = '1MiB';
 ----
 
 exec-sql
-BACKUP cfs TO 'nodelocal://1/foo';
+BACKUP cfs INTO 'nodelocal://1/foo';
 ----
 
 exec-sql
 CREATE DATABASE r1;
 ----
 
 exec-sql
-RESTORE cfs FROM 'nodelocal://1/foo' WITH into_db='r1';
+RESTORE cfs FROM LATEST IN 'nodelocal://1/foo' WITH into_db='r1';
 ----
 
 query-sql

pkg/ccl/backupccl/testdata/backup-restore/descriptor-broadening

@@ -11,19 +11,19 @@ CREATE TABLE db1.t (a INT);
 ----
 
 exec-sql
-BACKUP DATABASE db1 TO 'nodelocal://1/backup';
+BACKUP DATABASE db1 INTO 'nodelocal://1/backup';
 ----
 
 exec-sql
-BACKUP DATABASE db1,db2 TO 'nodelocal://1/backup';
+BACKUP DATABASE db1,db2 INTO LATEST IN 'nodelocal://1/backup';
 ----
 pq: previous backup does not contain the complete database "db2"
 
 exec-sql
-BACKUP db1.t TO 'nodelocal://1/backup_2';
+BACKUP db1.t INTO 'nodelocal://1/backup_2';
 ----
 
 exec-sql
-BACKUP DATABASE db1 TO 'nodelocal://1/backup_2';
+BACKUP DATABASE db1 INTO LATEST IN 'nodelocal://1/backup_2';
 ----
 pq: previous backup does not contain the complete database "db1"

pkg/ccl/backupccl/testdata/backup-restore/feature-flags

@@ -1,3 +1,5 @@
+subtest backup-feature-flags
+
 new-server name=s1
 ----
 
@@ -13,7 +15,7 @@ SET CLUSTER SETTING feature.backup.enabled = FALSE;
 ----
 
 exec-sql
-BACKUP TO 'nodelocal://0/test-root/';
+BACKUP INTO 'nodelocal://0/test-root/';
 ----
 pq: feature BACKUP was disabled by the database administrator
 
@@ -23,7 +25,7 @@ SET CLUSTER SETTING feature.backup.enabled = TRUE;
 ----
 
 exec-sql
-BACKUP TO 'nodelocal://0/test-root/';
+BACKUP INTO 'nodelocal://0/test-root/';
 ----
 
 exec-sql
@@ -36,7 +38,7 @@ SET CLUSTER SETTING feature.restore.enabled = FALSE;
 ----
 
 exec-sql
-RESTORE TABLE d.t FROM 'nodelocal://0/test-root/';
+RESTORE TABLE d.t FROM LATEST IN 'nodelocal://0/test-root/';
 ----
 pq: feature RESTORE was disabled by the database administrator
 
@@ -46,5 +48,38 @@ SET CLUSTER SETTING feature.restore.enabled = TRUE;
 ----
 
 exec-sql
-RESTORE TABLE d.t FROM 'nodelocal://0/test-root/';
+RESTORE TABLE d.t FROM LATEST IN 'nodelocal://0/test-root/';
+----
+
+subtest end
+
+# TODO(adityamaru): Delete in 22.2 once the syntax is removed. We don't want
+# to start a testserver just for this test.
+subtest backup-restore-deprecation-notice
+
+exec-sql
+BACKUP TO 'nodelocal://1/deprecated';
 ----
+NOTICE: The `BACKUP TO` syntax will be removed in a future release, please switch over to using `BACKUP INTO` to create a backup collection: https://www.cockroachlabs.com/docs/stable/backup.html#considerations. Backups created using the `BACKUP TO` syntax may not be restoreable in the next major version release.
+
+exec-sql
+BACKUP TO 'nodelocal://1/deprecated/incfrom' INCREMENTAL FROM 'nodelocal://1/deprecated';
+----
+NOTICE: The `BACKUP TO` syntax will be removed in a future release, please switch over to using `BACKUP INTO` to create a backup collection: https://www.cockroachlabs.com/docs/stable/backup.html#considerations. Backups created using the `BACKUP TO` syntax may not be restoreable in the next major version release.
+
+exec-sql
+DROP TABLE d.t;
+----
+
+exec-sql
+RESTORE TABLE d.t FROM 'nodelocal://1/deprecated';
+----
+NOTICE: The `RESTORE FROM <backup>` syntax will be removed in a future release, please switch over to using `RESTORE FROM <backup> IN <collection>` to restore a particular backup from a collection: https://www.cockroachlabs.com/docs/stable/restore.html#view-the-backup-subdirectories
+
+
+exec-sql
+RESTORE SYSTEM USERS FROM 'nodelocal://1/deprecated';
+----
+NOTICE: The `RESTORE FROM <backup>` syntax will be removed in a future release, please switch over to using `RESTORE FROM <backup> IN <collection>` to restore a particular backup from a collection: https://www.cockroachlabs.com/docs/stable/restore.html#view-the-backup-subdirectories
+
+subtest end

pkg/ccl/backupccl/testdata/backup-restore/max-row-size

@@ -23,15 +23,15 @@ INSERT INTO maxrow VALUES (2, repeat('x', 20000))
 pq: row larger than max row size: table 109 family 0 primary key /Table/109/1/2/0 size 20013
 
 exec-sql
-BACKUP maxrow TO 'nodelocal://1/maxrow';
+BACKUP maxrow INTO 'nodelocal://1/maxrow';
 ----
 
 exec-sql
 CREATE DATABASE d2;
 ----
 
 exec-sql
-RESTORE maxrow FROM 'nodelocal://1/maxrow' WITH into_db='d2';
+RESTORE maxrow FROM LATEST IN 'nodelocal://1/maxrow' WITH into_db='d2';
 ----
 
 query-sql

pkg/ccl/backupccl/testdata/backup-restore/multiregion

@@ -15,27 +15,27 @@ us-east-1
 us-west-1
 
 exec-sql
-BACKUP DATABASE d TO 'nodelocal://1/database_backup/';
+BACKUP DATABASE d INTO 'nodelocal://1/database_backup/';
 ----
 
 exec-sql
-BACKUP TO 'nodelocal://1/full_cluster_backup/';
+BACKUP INTO 'nodelocal://1/full_cluster_backup/';
 ----
 
 # A new cluster with the same locality settings.
 new-server name=s2 share-io-dir=s1 allow-implicit-access localities=us-east-1,us-west-1,eu-central-1
 ----
 
 exec-sql
-RESTORE FROM 'nodelocal://0/full_cluster_backup/';
+RESTORE FROM LATEST IN 'nodelocal://0/full_cluster_backup/';
 ----
 
 exec-sql
 DROP DATABASE d;
 ----
 
 exec-sql
-RESTORE DATABASE d FROM 'nodelocal://0/database_backup/';
+RESTORE DATABASE d FROM LATEST IN 'nodelocal://0/database_backup/';
 ----
 
 query-sql
@@ -52,13 +52,13 @@ new-server name=s3 share-io-dir=s1 allow-implicit-access localities=eu-central-1
 ----
 
 exec-sql
-RESTORE DATABASE d FROM 'nodelocal://0/database_backup/';
+RESTORE DATABASE d FROM LATEST IN 'nodelocal://0/database_backup/';
 ----
 pq: detected a mismatch in regions between the restore cluster and the backup cluster, missing regions detected: us-east-1, us-west-1.
 HINT: there are two ways you can resolve this issue: 1) update the cluster to which you're restoring to ensure that the regions present on the nodes' --locality flags match those present in the backup image, or 2) restore with the "skip_localities_check" option
 
 exec-sql
-RESTORE FROM 'nodelocal://0/full_cluster_backup/';
+RESTORE FROM LATEST IN 'nodelocal://0/full_cluster_backup/';
 ----
 pq: detected a mismatch in regions between the restore cluster and the backup cluster, missing regions detected: us-east-1, us-west-1.
 HINT: there are two ways you can resolve this issue: 1) update the cluster to which you're restoring to ensure that the regions present on the nodes' --locality flags match those present in the backup image, or 2) restore with the "skip_localities_check" option
@@ -77,11 +77,11 @@ INSERT INTO no_region_db_2.t VALUES (1), (2), (3);
 ----
 
 exec-sql
-BACKUP DATABASE no_region_db TO 'nodelocal://1/no_region_database_backup/';
+BACKUP DATABASE no_region_db INTO 'nodelocal://1/no_region_database_backup/';
 ----
 
 exec-sql
-BACKUP TO 'nodelocal://1/no_region_cluster_backup/';
+BACKUP INTO 'nodelocal://1/no_region_cluster_backup/';
 ----
 
 exec-sql
@@ -97,7 +97,7 @@ SET CLUSTER SETTING sql.defaults.primary_region = 'non-existent-region';
 ----
 
 exec-sql
-RESTORE DATABASE no_region_db FROM 'nodelocal://1/no_region_database_backup/';
+RESTORE DATABASE no_region_db FROM LATEST IN 'nodelocal://1/no_region_database_backup/';
 ----
 pq: region "non-existent-region" does not exist
 HINT: valid regions: eu-central-1, eu-north-1
@@ -109,7 +109,7 @@ SET CLUSTER SETTING sql.defaults.primary_region = 'eu-central-1';
 ----
 
 exec-sql
-RESTORE DATABASE no_region_db FROM 'nodelocal://1/no_region_database_backup/';
+RESTORE DATABASE no_region_db FROM LATEST IN 'nodelocal://1/no_region_database_backup/';
 ----
 NOTICE: setting the PRIMARY REGION as eu-central-1 on database no_region_db
 HINT: to change the default primary region, use SET CLUSTER SETTING sql.defaults.primary_region = 'region' or use RESET CLUSTER SETTING sql.defaults.primary_region to disable this behavior
@@ -136,7 +136,7 @@ INSERT INTO eu_central_db.t VALUES (1), (2), (3);
 NOTICE: setting eu-central-1 as the PRIMARY REGION as no PRIMARY REGION was specified
 
 exec-sql
-BACKUP DATABASE eu_central_db TO 'nodelocal://1/eu_central_database_backup/';
+BACKUP DATABASE eu_central_db INTO 'nodelocal://1/eu_central_database_backup/';
 ----
 
 # New cluster for a cluster backup.
@@ -148,7 +148,7 @@ SET CLUSTER SETTING sql.defaults.primary_region = 'eu-north-1';
 ----
 
 exec-sql
-RESTORE FROM 'nodelocal://1/no_region_cluster_backup/';
+RESTORE FROM LATEST IN 'nodelocal://1/no_region_cluster_backup/';
 ----
 NOTICE: setting the PRIMARY REGION as eu-north-1 on database defaultdb
 HINT: to change the default primary region, use SET CLUSTER SETTING sql.defaults.primary_region = 'region' or use RESET CLUSTER SETTING sql.defaults.primary_region to disable this behavior
@@ -176,7 +176,7 @@ public t table root <nil> REGIONAL BY TABLE IN PRIMARY REGION
 
 # Check we can restore without triggering the default primary region.
 exec-sql
-RESTORE DATABASE eu_central_db FROM 'nodelocal://1/eu_central_database_backup/';
+RESTORE DATABASE eu_central_db FROM LATEST IN 'nodelocal://1/eu_central_database_backup/';
 ----
 
 query-sql