Skip to content

Commit

Permalink
upgrades_test: use old bootstrap schema instead of injecting descriptors
Browse files Browse the repository at this point in the history 
This patch removes uses of upgrades.InjectLegacyTable within tests for
v22.2 to v23.1 upgrades in favor of using the v22.2 bootstrap schema to
start up the test cluster, since that better resembles the initial state
of non-test clusters and leads to less brittle test code.

Release note: None
  • Loading branch information
@andyyang890
andyyang890 committed Mar 30, 2023
1 parent dbf2dfb commit 16ff79d
Show file tree
Hide file tree
Showing 12 changed files with 12 additions and 789 deletions.
1 change: 0 additions & 1 deletion pkg/upgrade/upgrades/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,6 @@ go_test(
"//pkg/sql/schemachanger/scop",
"//pkg/sql/schemachanger/scplan",
"//pkg/sql/sem/builtins/builtinconstants",
"//pkg/sql/sem/catconstants",
"//pkg/sql/sem/catid",
"//pkg/sql/sem/tree",
"//pkg/sql/tests",
Expand Down
106 changes: 1 addition & 105 deletions pkg/upgrade/upgrades/alter_jobs_add_job_type_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/security/username"
"github.com/cockroachdb/cockroach/pkg/server"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catenumpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/systemschema"
"github.com/cockroachdb/cockroach/pkg/sql/privilege"
"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
"github.com/cockroachdb/cockroach/pkg/testutils/testcluster"
"github.com/cockroachdb/cockroach/pkg/upgrade/upgrades"
Expand Down Expand Up @@ -105,6 +99,7 @@ func TestAlterSystemJobsTableAddJobTypeColumn(t *testing.T) {
Knobs: base.TestingKnobs{
Server: &server.TestingKnobs{
DisableAutomaticVersionUpgrade: make(chan struct{}),
BootstrapVersionKeyOverride: clusterversion.V22_2,
BinaryVersionOverride: clusterversion.ByKey(
clusterversion.V23_1AddTypeColumnToJobsTable - 1),
},
Expand All @@ -128,8 +123,6 @@ func TestAlterSystemJobsTableAddJobTypeColumn(t *testing.T) {
}
)

// Inject the old copy of the descriptor and validate that the schema matches the old version.
upgrades.InjectLegacyTable(ctx, t, s, systemschema.JobsTable, getJobsTableDescriptorPriorToV23_1AddTypeColumnToJobsTable)
upgrades.ValidateSchemaExists(
ctx,
t,
Expand Down Expand Up @@ -224,100 +217,3 @@ func TestAlterSystemJobsTableAddJobTypeColumn(t *testing.T) {
assert.True(t, seenTypes.Contains(int(typ)))
}, false)
}

func getJobsTableDescriptorPriorToV23_1AddTypeColumnToJobsTable() *descpb.TableDescriptor {
defaultID := "unique_rowid()"
defaultCreated := "now():::TIMESTAMP"
return &descpb.TableDescriptor{
Name: string(catconstants.JobsTableName),
ID: keys.JobsTableID,
ParentID: keys.SystemDatabaseID,
UnexposedParentSchemaID: keys.PublicSchemaID,
Version: 1,
Columns: []descpb.ColumnDescriptor{
{Name: "id", ID: 1, Type: types.Int, DefaultExpr: &defaultID},
{Name: "status", ID: 2, Type: types.String},
{Name: "created", ID: 3, Type: types.Timestamp, DefaultExpr: &defaultCreated},
{Name: "payload", ID: 4, Type: types.Bytes},
{Name: "progress", ID: 5, Type: types.Bytes},
{Name: "created_by_type", ID: 6, Type: types.String, Nullable: true},
{Name: "created_by_id", ID: 7, Type: types.Int, Nullable: true},
{Name: "claim_session_id", ID: 8, Type: types.Bytes, Nullable: true},
{Name: "claim_instance_id", ID: 9, Type: types.Int, Nullable: true},
{Name: "num_runs", ID: 10, Type: types.Int, Nullable: true},
{Name: "last_run", ID: 11, Type: types.Timestamp, Nullable: true},
},
NextColumnID: 12,
Families: []descpb.ColumnFamilyDescriptor{
{
Name: "fam_0_id_status_created_payload",
ID: 0,
ColumnNames: []string{"id", "status", "created", "payload", "created_by_type", "created_by_id"},
ColumnIDs: []descpb.ColumnID{1, 2, 3, 4, 6, 7},
},
{
Name: "progress",
ID: 1,
ColumnNames: []string{"progress"},
ColumnIDs: []descpb.ColumnID{5},
DefaultColumnID: 5,
},
{
Name: "claim",
ID: 2,
ColumnNames: []string{"claim_session_id", "claim_instance_id", "num_runs", "last_run"},
ColumnIDs: []descpb.ColumnID{8, 9, 10, 11},
},
},
NextFamilyID: 3,
PrimaryIndex: descpb.IndexDescriptor{
Name: "id",
ID: 1,
Unique: true,
KeyColumnNames: []string{"id"},
KeyColumnDirections: []catenumpb.IndexColumn_Direction{catenumpb.IndexColumn_ASC},
KeyColumnIDs: []descpb.ColumnID{1},
},
Indexes: []descpb.IndexDescriptor{
{
Name: "jobs_status_created_idx",
ID: 2,
Unique: false,
KeyColumnNames: []string{"status", "created"},
KeyColumnDirections: []catenumpb.IndexColumn_Direction{catenumpb.IndexColumn_ASC, catenumpb.IndexColumn_ASC},
KeyColumnIDs: []descpb.ColumnID{2, 3},
KeySuffixColumnIDs: []descpb.ColumnID{1},
Version: descpb.StrictIndexColumnIDGuaranteesVersion,
},
{
Name: "jobs_created_by_type_created_by_id_idx",
ID: 3,
Unique: false,
KeyColumnNames: []string{"created_by_type", "created_by_id"},
KeyColumnDirections: []catenumpb.IndexColumn_Direction{catenumpb.IndexColumn_ASC, catenumpb.IndexColumn_ASC},
KeyColumnIDs: []descpb.ColumnID{6, 7},
StoreColumnIDs: []descpb.ColumnID{2},
StoreColumnNames: []string{"status"},
KeySuffixColumnIDs: []descpb.ColumnID{1},
Version: descpb.StrictIndexColumnIDGuaranteesVersion,
},
{
Name: "jobs_run_stats_idx",
ID: 4,
Unique: false,
KeyColumnNames: []string{"claim_session_id", "status", "created"},
KeyColumnDirections: []catenumpb.IndexColumn_Direction{catenumpb.IndexColumn_ASC, catenumpb.IndexColumn_ASC, catenumpb.IndexColumn_ASC},
KeyColumnIDs: []descpb.ColumnID{8, 2, 3},
StoreColumnNames: []string{"last_run", "num_runs", "claim_instance_id"},
StoreColumnIDs: []descpb.ColumnID{11, 10, 9},
KeySuffixColumnIDs: []descpb.ColumnID{1},
Version: descpb.StrictIndexColumnIDGuaranteesVersion,
Predicate: systemschema.JobsRunStatsIdxPredicate,
},
},
NextIndexID: 5,
Privileges: catpb.NewCustomSuperuserPrivilegeDescriptor(privilege.ReadWriteData, username.NodeUserName()),
NextMutationID: 1,
FormatVersion: 3,
}
}
52 changes: 1 addition & 51 deletions pkg/upgrade/upgrades/alter_sql_instances_sql_addr_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,8 @@ import (
"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/security/username"
"github.com/cockroachdb/cockroach/pkg/server"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catenumpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/systemschema"
"github.com/cockroachdb/cockroach/pkg/sql/privilege"
"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/testutils/testcluster"
"github.com/cockroachdb/cockroach/pkg/upgrade/upgrades"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
Expand All @@ -41,6 +34,7 @@ func TestAlterSystemSqlInstancesTableAddSqlAddr(t *testing.T) {
Knobs: base.TestingKnobs{
Server: &server.TestingKnobs{
DisableAutomaticVersionUpgrade: make(chan struct{}),
BootstrapVersionKeyOverride: clusterversion.V22_2,
BinaryVersionOverride: clusterversion.ByKey(
clusterversion.V23_1AlterSystemSQLInstancesAddSQLAddr - 1),
},
Expand All @@ -63,9 +57,7 @@ func TestAlterSystemSqlInstancesTableAddSqlAddr(t *testing.T) {
}
)

// Inject the old copy of the descriptor.
sqlInstancesTable := systemschema.SQLInstancesTable()
upgrades.InjectLegacyTable(ctx, t, s, sqlInstancesTable, getDeprecatedSqlInstancesDescriptorWithoutSqlAddr)
// Validate that the table sql_instances has the old schema.
upgrades.ValidateSchemaExists(
ctx,
Expand Down Expand Up @@ -99,45 +91,3 @@ func TestAlterSystemSqlInstancesTableAddSqlAddr(t *testing.T) {
true, /* expectExists */
)
}

// getDeprecatedSqlInstancesDescriptor returns the system.sql_instances
// table descriptor that was being used before adding a new column in the
// current version.
func getDeprecatedSqlInstancesDescriptorWithoutSqlAddr() *descpb.TableDescriptor {
return &descpb.TableDescriptor{
Name: string(catconstants.SQLInstancesTableName),
ID: keys.SQLInstancesTableID,
ParentID: keys.SystemDatabaseID,
UnexposedParentSchemaID: keys.PublicSchemaID,
Version: 1,
Columns: []descpb.ColumnDescriptor{
{Name: "id", ID: 1, Type: types.Int, Nullable: false},
{Name: "addr", ID: 2, Type: types.String, Nullable: true},
{Name: "session_id", ID: 3, Type: types.Bytes, Nullable: true},
{Name: "locality", ID: 4, Type: types.Bytes, Nullable: true},
},
NextColumnID: 5,
Families: []descpb.ColumnFamilyDescriptor{
{
Name: "primary",
ID: 0,
ColumnNames: []string{"id", "addr", "session_id", "locality"},
ColumnIDs: []descpb.ColumnID{1, 2, 3, 4},
DefaultColumnID: 0,
},
},
NextFamilyID: 1,
PrimaryIndex: descpb.IndexDescriptor{
Name: "id",
ID: 1,
Unique: true,
KeyColumnNames: []string{"id"},
KeyColumnDirections: []catenumpb.IndexColumn_Direction{catenumpb.IndexColumn_ASC},
KeyColumnIDs: []descpb.ColumnID{1},
},
NextIndexID: 2,
Privileges: catpb.NewCustomSuperuserPrivilegeDescriptor(privilege.ReadWriteData, username.NodeUserName()),
NextMutationID: 1,
FormatVersion: 3,
}
}
71 changes: 1 addition & 70 deletions pkg/upgrade/upgrades/alter_table_statistics_partial_predicate_and_id_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,8 @@ import (
"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/security/username"
"github.com/cockroachdb/cockroach/pkg/server"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catenumpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/systemschema"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
"github.com/cockroachdb/cockroach/pkg/sql/privilege"
"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/testutils/testcluster"
"github.com/cockroachdb/cockroach/pkg/upgrade/upgrades"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
Expand All @@ -42,6 +34,7 @@ func TestAlterSystemTableStatisticsAddPartialPredicateAndID(t *testing.T) {
Knobs: base.TestingKnobs{
Server: &server.TestingKnobs{
DisableAutomaticVersionUpgrade: make(chan struct{}),
BootstrapVersionKeyOverride: clusterversion.V22_2,
BinaryVersionOverride: clusterversion.ByKey(
clusterversion.V23_1AddPartialStatisticsColumns - 1),
},
Expand All @@ -64,8 +57,6 @@ func TestAlterSystemTableStatisticsAddPartialPredicateAndID(t *testing.T) {
}
)

// Inject the old copy of the descriptor.
upgrades.InjectLegacyTable(ctx, t, s, systemschema.TableStatisticsTable, getDeprecatedTableStatisticsDescriptor)
// Validate that the table statistics table has the old schema.
upgrades.ValidateSchemaExists(
ctx,
Expand Down Expand Up @@ -100,63 +91,3 @@ func TestAlterSystemTableStatisticsAddPartialPredicateAndID(t *testing.T) {
true, /* expectExists */
)
}

func getDeprecatedTableStatisticsDescriptor() *descpb.TableDescriptor {
uniqueRowIDString := "unique_rowid()"
nowString := "now()::TIMESTAMP"
zeroIntString := "0:::INT8"

return &descpb.TableDescriptor{
Name: string(catconstants.TableStatisticsTableName),
ID: keys.TableStatisticsTableID,
ParentID: keys.SystemDatabaseID,
UnexposedParentSchemaID: keys.PublicSchemaID,
Version: 1,
Columns: []descpb.ColumnDescriptor{
{Name: "tableID", ID: 1, Type: types.Int},
{Name: "statisticID", ID: 2, Type: types.Int, DefaultExpr: &uniqueRowIDString},
{Name: "name", ID: 3, Type: types.String, Nullable: true},
{Name: "columnIDs", ID: 4, Type: types.IntArray},
{Name: "createdAt", ID: 5, Type: types.Timestamp, DefaultExpr: &nowString},
{Name: "rowCount", ID: 6, Type: types.Int},
{Name: "distinctCount", ID: 7, Type: types.Int},
{Name: "nullCount", ID: 8, Type: types.Int},
{Name: "histogram", ID: 9, Type: types.Bytes, Nullable: true},
{Name: "avgSize", ID: 10, Type: types.Int, DefaultExpr: &zeroIntString},
},
NextColumnID: 11,
Families: []descpb.ColumnFamilyDescriptor{
{
Name: "fam_0_tableID_statisticID_name_columnIDs_createdAt_rowCount_distinctCount_nullCount_histogram",
ID: 0,
ColumnNames: []string{
"tableID",
"statisticID",
"name",
"columnIDs",
"createdAt",
"rowCount",
"distinctCount",
"nullCount",
"histogram",
"avgSize",
},
ColumnIDs: []descpb.ColumnID{1, 2, 3, 4, 5, 6, 7, 8, 9, 10},
},
},
NextFamilyID: 1,
PrimaryIndex: descpb.IndexDescriptor{
Name: tabledesc.LegacyPrimaryKeyIndexName,
ID: 1,
Unique: true,
KeyColumnNames: []string{"tableID", "statisticID"},
KeyColumnDirections: []catenumpb.IndexColumn_Direction{catenumpb.IndexColumn_ASC, catenumpb.IndexColumn_ASC},
KeyColumnIDs: []descpb.ColumnID{1, 2},
},
NextIndexID: 2,
Privileges: catpb.NewCustomSuperuserPrivilegeDescriptor(privilege.ReadWriteData, username.NodeUserName()),
NextMutationID: 1,
FormatVersion: 3,
}

}
Loading

0 comments on commit 16ff79d

Please sign in to comment.