Skip to content

Commit a5801ff

Browse files
committed
backupresolver: adjust test message expectations
This adjusts tests to check for the appropriate errors that are returned by the new ResolveTargets implementation. - The old code delegated to DescriptorsMatchingTargets, which was returning an incorrectly specific error message when a database did not exist. - To handle wildcard expansion, The previous implementation was calling ResolveObjectPrefix, which mutates the object name in place to add the `public` schema to the pattern if it was unspecified. This was not correct, as the db.* pattern causes all tables in the database to be backed up, not just the ones in the public schema. Release note: None
1 parent d74dbe8 commit a5801ff

File tree

4 files changed

+8
-15
lines changed

4 files changed

+8
-15
lines changed

pkg/backup/backupresolver/targets.go

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -782,10 +782,6 @@ func ResolveTargets(
782782
}
783783
schema, err := col.ByIDWithoutLeased(txn.KV()).Get().Schema(ctx, schemaID)
784784
if err != nil {
785-
// Schema might not exist or be dropped, skip it.
786-
if errors.Is(err, catalog.ErrDescriptorNotFound) {
787-
return nil
788-
}
789785
return err
790786
}
791787
if !shouldSkipSchema(schema) {
@@ -798,13 +794,10 @@ func ResolveTargets(
798794
for _, dbName := range targets.Databases {
799795
db, err := col.ByName(txn.KV()).Get().Database(ctx, string(dbName))
800796
if err != nil {
801-
if errors.Is(err, catalog.ErrDescriptorNotFound) {
802-
return errors.Errorf("database %q does not exist", dbName)
803-
}
804797
return err
805798
}
806799
if !db.Public() {
807-
return errors.Errorf("database %q does not exist", dbName)
800+
return sqlerrors.NewUndefinedDatabaseError(tree.ErrString(&dbName))
808801
}
809802
addDesc(db)
810803

pkg/backup/create_scheduled_backup_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ CREATE TABLE other_db.t1(a int);
333333
{
334334
name: "unqualified-all-tables-selectors",
335335
query: "CREATE SCHEDULE FOR BACKUP * INTO $1 RECURRING '@hourly'",
336-
expectedBackupStmt: "BACKUP TABLE mydb.public.* INTO %s'%s' WITH OPTIONS (detached)",
336+
expectedBackupStmt: "BACKUP TABLE mydb.* INTO %s'%s' WITH OPTIONS (detached)",
337337
},
338338
{
339339
name: "all-tables-selectors-with-user-defined-schema",
@@ -343,12 +343,12 @@ CREATE TABLE other_db.t1(a int);
343343
{
344344
name: "partially-qualified-all-tables-selectors-with-different-db",
345345
query: "CREATE SCHEDULE FOR BACKUP other_db.* INTO $1 RECURRING '@hourly'",
346-
expectedBackupStmt: "BACKUP TABLE other_db.public.* INTO %s'%s' WITH OPTIONS (detached)",
346+
expectedBackupStmt: "BACKUP TABLE other_db.* INTO %s'%s' WITH OPTIONS (detached)",
347347
},
348348
{
349349
name: "fully-qualified-all-tables-selectors-with-multiple-dbs",
350350
query: "CREATE SCHEDULE FOR BACKUP *, other_db.* INTO $1 RECURRING '@hourly'",
351-
expectedBackupStmt: "BACKUP TABLE mydb.public.*, other_db.public.* INTO %s'%s' WITH OPTIONS (detached)",
351+
expectedBackupStmt: "BACKUP TABLE mydb.*, other_db.* INTO %s'%s' WITH OPTIONS (detached)",
352352
},
353353
}
354354

@@ -592,14 +592,14 @@ func TestSerializesScheduledBackupExecutionArgs(t *testing.T) {
592592
expectedSchedules: []expectedSchedule{
593593
{
594594
nameRe: "BACKUP .*",
595-
backupStmt: "BACKUP TABLE system.public.* INTO LATEST IN 'nodelocal://1/backup' WITH OPTIONS (revision_history = true, detached)",
595+
backupStmt: "BACKUP TABLE system.* INTO LATEST IN 'nodelocal://1/backup' WITH OPTIONS (revision_history = true, detached)",
596596
period: time.Hour,
597597
paused: true,
598598
chainProtectedTimestampRecord: true,
599599
},
600600
{
601601
nameRe: "BACKUP .+",
602-
backupStmt: "BACKUP TABLE system.public.* INTO 'nodelocal://1/backup' WITH OPTIONS (revision_history = true, detached)",
602+
backupStmt: "BACKUP TABLE system.* INTO 'nodelocal://1/backup' WITH OPTIONS (revision_history = true, detached)",
603603
period: 24 * time.Hour,
604604
runsNow: true,
605605
chainProtectedTimestampRecord: true,

pkg/backup/testdata/backup-restore/backup-dropped-descriptors

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ SELECT orig->'database'->'name', orig->'database'->'state' FROM tbls WHERE id =
4141
exec-sql
4242
BACKUP DATABASE d INTO 'nodelocal://1/dropped-database';
4343
----
44-
pq: failed to resolve targets specified in the BACKUP stmt: database "d" does not exist, or invalid RESTORE timestamp: supplied backups do not cover requested time
44+
pq: failed to resolve targets specified in the BACKUP stmt: database "d" does not exist
4545

4646
# A cluster backup should succeed but should ignore the dropped database
4747
# and table descriptors.

pkg/backup/testdata/backup-restore/backup-dropped-descriptors-declarative

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ SELECT orig->'database'->'name', orig->'database'->'state' FROM tbls WHERE id =
4242
exec-sql
4343
BACKUP DATABASE dd INTO 'nodelocal://1/dropped-database';
4444
----
45-
pq: failed to resolve targets specified in the BACKUP stmt: database "dd" does not exist, or invalid RESTORE timestamp: supplied backups do not cover requested time
45+
pq: failed to resolve targets specified in the BACKUP stmt: database "dd" does not exist
4646

4747
# A cluster backup should succeed.
4848
exec-sql

0 commit comments

Comments
 (0)