From 4e52ddbaeb1ff1bf70689cce745b6b490c743f12 Mon Sep 17 00:00:00 2001
From: Michael Butler <butler@cockroachlabs.com>
Date: Mon, 11 Apr 2022 16:13:38 -0400
Subject: [PATCH] backupccl: ensure AOST incremental backup ends after previous
 backup

Informs #79674

Release note (sql change): Previously, a user could run an AS OF SYSTEM TIME
incremental backup with an end time earlier than the previous backup's end time
, which could lead to an out of order incremental backup chain. This PR causes
the incremental backup to fail if the AS OF SYSTEM TIME is less than the
previous backup's end time.
---
 pkg/ccl/backupccl/backup_planning.go |  6 ++++++
 pkg/ccl/backupccl/backup_test.go     | 10 ++++++++++
 2 files changed, 16 insertions(+)

diff --git a/pkg/ccl/backupccl/backup_planning.go b/pkg/ccl/backupccl/backup_planning.go
index ed6a7d73b2ee..f535080d3c3d 100644
--- a/pkg/ccl/backupccl/backup_planning.go
+++ b/pkg/ccl/backupccl/backup_planning.go
@@ -725,6 +725,12 @@ func backupPlanHook(
 			if err := requireEnterprise("incremental"); err != nil {
 				return err
 			}
+			lastEndTime := prevBackups[len(prevBackups)-1].EndTime
+			if endTime.Less(lastEndTime) {
+				return errors.Newf("`AS OF SYSTEM TIME` %s must be greater than "+
+					"the previous backup's end time of %s.",
+					endTime.GoTime(), lastEndTime.GoTime())
+			}
 			startTime = prevBackups[len(prevBackups)-1].EndTime
 		}
 
diff --git a/pkg/ccl/backupccl/backup_test.go b/pkg/ccl/backupccl/backup_test.go
index 2dbf7abe8872..b24999a5bb96 100644
--- a/pkg/ccl/backupccl/backup_test.go
+++ b/pkg/ccl/backupccl/backup_test.go
@@ -3659,8 +3659,18 @@ func TestBackupAsOfSystemTime(t *testing.T) {
 
 	beforeDir := LocalFoo + `/beforeTs`
 	sqlDB.Exec(t, fmt.Sprintf(`BACKUP DATABASE data TO '%s' AS OF SYSTEM TIME %s`, beforeDir, beforeTs))
+
 	equalDir := LocalFoo + `/equalTs`
 	sqlDB.Exec(t, fmt.Sprintf(`BACKUP DATABASE data TO '%s' AS OF SYSTEM TIME %s`, equalDir, equalTs))
+	{
+		// testing UX guardrails for AS OF SYSTEM TIME backups in collections
+		sqlDB.Exec(t, fmt.Sprintf(`BACKUP DATABASE data INTO '%s' AS OF SYSTEM TIME %s`, equalDir, equalTs))
+
+		sqlDB.ExpectErr(t, "`AS OF SYSTEM TIME` .* must be greater than the previous backup's end time of",
+			fmt.Sprintf(`BACKUP DATABASE data INTO LATEST IN '%s' AS OF SYSTEM TIME %s`,
+				equalDir,
+				beforeTs))
+	}
 
 	sqlDB.Exec(t, `DROP TABLE data.bank`)
 	sqlDB.Exec(t, `RESTORE data.* FROM $1`, beforeDir)