Merge cockroachdb#31674

31674: sqlutils: fetch database/table id from namespace r=ridwanmsharif a=ridwanmsharif Resolves cockroachdb#28136 Instead of hard coding table and database IDs, fetch from the `system.namespace` for tables and databases because ID leaks during transaction retries causes test flakiness. Release note: None Co-authored-by: Ridwan Sharif <[email protected]>
benesch · Oct 21, 2018 · 2998190 · 2998190
2 parents 1bfd6e1 + 3306e6c
commit 2998190
Show file tree

Hide file tree

Showing 6 changed files with 34 additions and 52 deletions.
diff --git a/pkg/ccl/backupccl/backup_test.go b/pkg/ccl/backupccl/backup_test.go
@@ -444,21 +444,11 @@ func TestBackupRestoreSystemJobs(t *testing.T) {
 	sanitizedFullDir := localFoo + "/full"
 	fullDir := sanitizedFullDir + "?moarSecretsHere"
 
-	backupDatabaseID, err := sqlutils.QueryDatabaseID(sqlDB.DB, "data")
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	backupTableID, err := sqlutils.QueryTableID(sqlDB.DB, "data", "bank")
-	if err != nil {
-		t.Fatal(err)
-	}
+	backupDatabaseID := sqlutils.QueryDatabaseID(t, sqlDB.DB, "data")
+	backupTableID := sqlutils.QueryTableID(t, sqlDB.DB, "data", "bank")
 
 	sqlDB.Exec(t, `CREATE DATABASE restoredb`)
-	restoreDatabaseID, err := sqlutils.QueryDatabaseID(sqlDB.DB, "restoredb")
-	if err != nil {
-		t.Fatal(err)
-	}
+	restoreDatabaseID := sqlutils.QueryDatabaseID(t, sqlDB.DB, "restoredb")
 
 	// We create a full backup so that, below, we can test that incremental
 	// backups sanitize credentials in "INCREMENTAL FROM" URLs.
@@ -553,10 +543,7 @@ func checkInProgressBackupRestore(
 
 	sqlDB.Exec(t, `CREATE DATABASE restoredb`)
 
-	backupTableID, err := sqlutils.QueryTableID(sqlDB.DB, "data", "bank")
-	if err != nil {
-		t.Fatal(err)
-	}
+	backupTableID := sqlutils.QueryTableID(t, sqlDB.DB, "data", "bank")
 
 	do := func(query string, check inProgressChecker) {
 		jobDone := make(chan error)
@@ -791,10 +778,7 @@ func TestBackupRestoreResume(t *testing.T) {
 		restoreDir := "nodelocal:///restore"
 		sqlDB.Exec(t, `BACKUP DATABASE DATA TO $1`, restoreDir)
 		sqlDB.Exec(t, `CREATE DATABASE restoredb`)
-		restoreDatabaseID, err := sqlutils.QueryDatabaseID(sqlDB.DB, "restoredb")
-		if err != nil {
-			t.Fatal(err)
-		}
+		restoreDatabaseID := sqlutils.QueryDatabaseID(t, sqlDB.DB, "restoredb")
 		restoreTableID, err := sql.GenerateUniqueDescID(ctx, tc.Servers[0].DB())
 		if err != nil {
 			t.Fatal(err)

diff --git a/pkg/ccl/changefeedccl/helpers_test.go b/pkg/ccl/changefeedccl/helpers_test.go
@@ -707,10 +707,7 @@ func forceTableGC(
 	database, table string,
 ) {
 	t.Helper()
-	tblID, err := sqlutils.QueryTableID(sqlDB.DB, database, table)
-	if err != nil {
-		t.Fatal(err)
-	}
+	tblID := sqlutils.QueryTableID(t, sqlDB.DB, database, table)
 
 	tblKey := roachpb.Key(keys.MakeTablePrefix(tblID))
 	gcr := roachpb.GCRequest{

diff --git a/pkg/ccl/partitionccl/zone_test.go b/pkg/ccl/partitionccl/zone_test.go
@@ -50,7 +50,8 @@ func TestValidIndexPartitionSetShowZones(t *testing.T) {
 	partialZoneOverride := *config.NewZoneConfig()
 	partialZoneOverride.GC = &config.GCPolicy{TTLSeconds: 42}
 
-	dbDescID := uint32(keys.MinNonPredefinedUserDescID)
+	dbID := sqlutils.QueryDatabaseID(t, sqlDB.DB, "d")
+	tableID := sqlutils.QueryTableID(t, sqlDB.DB, "d", "t")
 
 	defaultRow := sqlutils.ZoneRow{
 		ID:           keys.RootNamespaceID,
@@ -63,54 +64,54 @@ func TestValidIndexPartitionSetShowZones(t *testing.T) {
 		Config:       zoneOverride,
 	}
 	dbRow := sqlutils.ZoneRow{
-		ID:           dbDescID,
+		ID:           dbID,
 		CLISpecifier: "d",
 		Config:       zoneOverride,
 	}
 	tableRow := sqlutils.ZoneRow{
-		ID:           dbDescID + 1,
+		ID:           tableID,
 		CLISpecifier: "d.t",
 		Config:       zoneOverride,
 	}
 	primaryRow := sqlutils.ZoneRow{
-		ID:           dbDescID + 1,
+		ID:           tableID,
 		CLISpecifier: "d.t@primary",
 		Config:       zoneOverride,
 	}
 	p0Row := sqlutils.ZoneRow{
-		ID:           dbDescID + 1,
+		ID:           tableID,
 		CLISpecifier: "d.t.p0",
 		Config:       zoneOverride,
 	}
 	p1Row := sqlutils.ZoneRow{
-		ID:           dbDescID + 1,
+		ID:           tableID,
 		CLISpecifier: "d.t.p1",
 		Config:       zoneOverride,
 	}
 
 	// Partially filled config rows
 	partialDbRow := sqlutils.ZoneRow{
-		ID:           dbDescID,
+		ID:           dbID,
 		CLISpecifier: "d",
 		Config:       partialZoneOverride,
 	}
 	partialTableRow := sqlutils.ZoneRow{
-		ID:           dbDescID + 1,
+		ID:           tableID,
 		CLISpecifier: "d.t",
 		Config:       partialZoneOverride,
 	}
 	partialPrimaryRow := sqlutils.ZoneRow{
-		ID:           dbDescID + 1,
+		ID:           tableID,
 		CLISpecifier: "d.t@primary",
 		Config:       partialZoneOverride,
 	}
 	partialP0Row := sqlutils.ZoneRow{
-		ID:           dbDescID + 1,
+		ID:           tableID,
 		CLISpecifier: "d.t.p0",
 		Config:       partialZoneOverride,
 	}
 	partialP1Row := sqlutils.ZoneRow{
-		ID:           dbDescID + 1,
+		ID:           tableID,
 		CLISpecifier: "d.t.p1",
 		Config:       partialZoneOverride,
 	}

diff --git a/pkg/sql/ambiguous_commit_test.go b/pkg/sql/ambiguous_commit_test.go
@@ -159,10 +159,7 @@ func TestAmbiguousCommit(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		tableID, err := sqlutils.QueryTableID(sqlDB, "test", "t")
-		if err != nil {
-			t.Fatal(err)
-		}
+		tableID := sqlutils.QueryTableID(t, sqlDB, "test", "t")
 		tableStartKey.Store(keys.MakeTablePrefix(tableID))
 
 		// Wait for new table to split & replication.

diff --git a/pkg/sql/zone_test.go b/pkg/sql/zone_test.go
@@ -71,14 +71,17 @@ func TestValidSetShowZones(t *testing.T) {
 		CLISpecifier: "system.jobs",
 		Config:       zoneOverride,
 	}
-	dbDescID := uint32(keys.MinNonPredefinedUserDescID)
+
+	dbID := sqlutils.QueryDatabaseID(t, sqlDB.DB, "d")
+	tableID := sqlutils.QueryTableID(t, sqlDB.DB, "d", "t")
+
 	dbRow := sqlutils.ZoneRow{
-		ID:           dbDescID,
+		ID:           dbID,
 		CLISpecifier: "d",
 		Config:       zoneOverride,
 	}
 	tableRow := sqlutils.ZoneRow{
-		ID:           dbDescID + 1,
+		ID:           tableID,
 		CLISpecifier: "d.t",
 		Config:       zoneOverride,
 	}
@@ -99,14 +102,13 @@ func TestValidSetShowZones(t *testing.T) {
 		CLISpecifier: "system.jobs",
 		Config:       partialZoneOverride,
 	}
-	dbDescID = uint32(keys.MinNonPredefinedUserDescID)
 	partialDbRow := sqlutils.ZoneRow{
-		ID:           dbDescID,
+		ID:           dbID,
 		CLISpecifier: "d",
 		Config:       partialZoneOverride,
 	}
 	partialTableRow := sqlutils.ZoneRow{
-		ID:           dbDescID + 1,
+		ID:           tableID,
 		CLISpecifier: "d.t",
 		Config:       partialZoneOverride,
 	}

diff --git a/pkg/testutils/sqlutils/table_id.go b/pkg/testutils/sqlutils/table_id.go
@@ -16,23 +16,24 @@ package sqlutils
 
 import (
 	gosql "database/sql"
+	"testing"
 )
 
 // QueryDatabaseID returns the database ID of the specified database using the
 // system.namespace table.
-func QueryDatabaseID(sqlDB *gosql.DB, dbName string) (uint32, error) {
+func QueryDatabaseID(t testing.TB, sqlDB *gosql.DB, dbName string) uint32 {
 	dbIDQuery := `SELECT id FROM system.namespace WHERE name = $1 AND "parentID" = 0`
 	var dbID uint32
 	result := sqlDB.QueryRow(dbIDQuery, dbName)
 	if err := result.Scan(&dbID); err != nil {
-		return 0, err
+		t.Fatal(err)
 	}
-	return dbID, nil
+	return dbID
 }
 
 // QueryTableID returns the table ID of the specified database.table
 // using the system.namespace table.
-func QueryTableID(sqlDB *gosql.DB, dbName, tableName string) (uint32, error) {
+func QueryTableID(t testing.TB, sqlDB *gosql.DB, dbName, tableName string) uint32 {
 	tableIDQuery := `
  SELECT tables.id FROM system.namespace tables
    JOIN system.namespace dbs ON dbs.id = tables."parentID"
@@ -41,7 +42,7 @@ func QueryTableID(sqlDB *gosql.DB, dbName, tableName string) (uint32, error) {
 	var tableID uint32
 	result := sqlDB.QueryRow(tableIDQuery, dbName, tableName)
 	if err := result.Scan(&tableID); err != nil {
-		return 0, err
+		t.Fatal(err)
 	}
-	return tableID, nil
+	return tableID
 }