From 16ff79d7eede84679b0fef0025f46313a2b00475 Mon Sep 17 00:00:00 2001 From: Andy Yang Date: Thu, 30 Mar 2023 13:55:47 -0400 Subject: [PATCH] upgrades_test: use old bootstrap schema instead of injecting descriptors 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 --- pkg/upgrade/upgrades/BUILD.bazel | 1 - .../upgrades/alter_jobs_add_job_type_test.go | 106 +-------------- .../alter_sql_instances_sql_addr_test.go | 52 +------- ...tatistics_partial_predicate_and_id_test.go | 71 +--------- ...e_index_usage_statement_statistics_test.go | 123 +----------------- ...e_settings_table_user_id_migration_test.go | 59 +-------- ...onnections_table_user_id_migration_test.go | 66 +--------- .../role_members_ids_migration_test.go | 2 +- .../system_privileges_index_migration_test.go | 86 +----------- ...ystem_privileges_user_id_migration_test.go | 63 +-------- .../upgrades/tenant_table_migration_test.go | 53 +------- ...b_sessions_table_user_id_migration_test.go | 119 +---------------- 12 files changed, 12 insertions(+), 789 deletions(-) diff --git a/pkg/upgrade/upgrades/BUILD.bazel b/pkg/upgrade/upgrades/BUILD.bazel index 13c77a19aa7b..61d215119020 100644 --- a/pkg/upgrade/upgrades/BUILD.bazel +++ b/pkg/upgrade/upgrades/BUILD.bazel @@ -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", diff --git a/pkg/upgrade/upgrades/alter_jobs_add_job_type_test.go b/pkg/upgrade/upgrades/alter_jobs_add_job_type_test.go index 06b8f19ebc6d..9877603a3f4c 100644 --- a/pkg/upgrade/upgrades/alter_jobs_add_job_type_test.go +++ b/pkg/upgrade/upgrades/alter_jobs_add_job_type_test.go @@ -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" @@ -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), }, @@ -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, @@ -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, - } -} diff --git a/pkg/upgrade/upgrades/alter_sql_instances_sql_addr_test.go b/pkg/upgrade/upgrades/alter_sql_instances_sql_addr_test.go index 27e2ac20fffc..7f3981b5c81e 100644 --- a/pkg/upgrade/upgrades/alter_sql_instances_sql_addr_test.go +++ b/pkg/upgrade/upgrades/alter_sql_instances_sql_addr_test.go @@ -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" @@ -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), }, @@ -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, @@ -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, - } -} diff --git a/pkg/upgrade/upgrades/alter_table_statistics_partial_predicate_and_id_test.go b/pkg/upgrade/upgrades/alter_table_statistics_partial_predicate_and_id_test.go index 08548c846aad..64c80b3e3139 100644 --- a/pkg/upgrade/upgrades/alter_table_statistics_partial_predicate_and_id_test.go +++ b/pkg/upgrade/upgrades/alter_table_statistics_partial_predicate_and_id_test.go @@ -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" @@ -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), }, @@ -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, @@ -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, - } - -} diff --git a/pkg/upgrade/upgrades/create_index_usage_statement_statistics_test.go b/pkg/upgrade/upgrades/create_index_usage_statement_statistics_test.go index 03bd6beed297..6313310870e4 100644 --- a/pkg/upgrade/upgrades/create_index_usage_statement_statistics_test.go +++ b/pkg/upgrade/upgrades/create_index_usage_statement_statistics_test.go @@ -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" @@ -42,6 +34,7 @@ func TestCreateIndexOnIndexUsageOnSystemStatementStatistics(t *testing.T) { Knobs: base.TestingKnobs{ Server: &server.TestingKnobs{ DisableAutomaticVersionUpgrade: make(chan struct{}), + BootstrapVersionKeyOverride: clusterversion.V22_2, BinaryVersionOverride: clusterversion.ByKey( clusterversion.V23_1_AlterSystemStatementStatisticsAddIndexesUsage - 1), }, @@ -65,8 +58,6 @@ func TestCreateIndexOnIndexUsageOnSystemStatementStatistics(t *testing.T) { } ) - // Inject the old copy of the descriptor. - upgrades.InjectLegacyTable(ctx, t, s, systemschema.StatementStatisticsTable, getStatementStatisticsNoIndexDescriptor) // Validate that the table statement_statistics has the old schema. upgrades.ValidateSchemaExists( ctx, @@ -100,115 +91,3 @@ func TestCreateIndexOnIndexUsageOnSystemStatementStatistics(t *testing.T) { true, /* expectExists */ ) } - -// getStatementStatisticsNoIndexDescriptor returns the system.statement_statistics -// table descriptor that was being used before adding a new column in the -// current version. -func getStatementStatisticsNoIndexDescriptor() *descpb.TableDescriptor { - sqlStmtHashComputeExpr := `mod(fnv32(crdb_internal.datums_to_bytes(aggregated_ts, app_name, fingerprint_id, node_id, plan_hash, transaction_fingerprint_id)), 8:::INT8)` - defaultIndexRec := "ARRAY[]:::STRING[]" - - return &descpb.TableDescriptor{ - Name: string(catconstants.StatementStatisticsTableName), - ID: keys.StatementStatisticsTableID, - ParentID: keys.SystemDatabaseID, - UnexposedParentSchemaID: keys.PublicSchemaID, - Version: 1, - Columns: []descpb.ColumnDescriptor{ - {Name: "aggregated_ts", ID: 1, Type: types.TimestampTZ, Nullable: false}, - {Name: "fingerprint_id", ID: 2, Type: types.Bytes, Nullable: false}, - {Name: "transaction_fingerprint_id", ID: 3, Type: types.Bytes, Nullable: false}, - {Name: "plan_hash", ID: 4, Type: types.Bytes, Nullable: false}, - {Name: "app_name", ID: 5, Type: types.String, Nullable: false}, - {Name: "node_id", ID: 6, Type: types.Int, Nullable: false}, - {Name: "agg_interval", ID: 7, Type: types.Interval, Nullable: false}, - {Name: "metadata", ID: 8, Type: types.Jsonb, Nullable: false}, - {Name: "statistics", ID: 9, Type: types.Jsonb, Nullable: false}, - {Name: "plan", ID: 10, Type: types.Jsonb, Nullable: false}, - { - Name: "crdb_internal_aggregated_ts_app_name_fingerprint_id_node_id_plan_hash_transaction_fingerprint_id_shard_8", - ID: 11, - Type: types.Int4, - Nullable: false, - ComputeExpr: &sqlStmtHashComputeExpr, - Hidden: true, - }, - {Name: "index_recommendations", ID: 12, Type: types.StringArray, Nullable: false, DefaultExpr: &defaultIndexRec}, - }, - NextColumnID: 13, - Families: []descpb.ColumnFamilyDescriptor{ - { - Name: "primary", - ID: 0, - ColumnNames: []string{ - "crdb_internal_aggregated_ts_app_name_fingerprint_id_node_id_plan_hash_transaction_fingerprint_id_shard_8", - "aggregated_ts", "fingerprint_id", "transaction_fingerprint_id", "plan_hash", "app_name", "node_id", - "agg_interval", "metadata", "statistics", "plan", "index_recommendations", - }, - ColumnIDs: []descpb.ColumnID{11, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 12}, - DefaultColumnID: 0, - }, - }, - NextFamilyID: 1, - PrimaryIndex: descpb.IndexDescriptor{ - Name: tabledesc.LegacyPrimaryKeyIndexName, - ID: 1, - Unique: true, - KeyColumnNames: []string{ - "crdb_internal_aggregated_ts_app_name_fingerprint_id_node_id_plan_hash_transaction_fingerprint_id_shard_8", - "aggregated_ts", - "fingerprint_id", - "transaction_fingerprint_id", - "plan_hash", - "app_name", - "node_id", - }, - KeyColumnDirections: []catenumpb.IndexColumn_Direction{ - catenumpb.IndexColumn_ASC, - catenumpb.IndexColumn_ASC, - catenumpb.IndexColumn_ASC, - catenumpb.IndexColumn_ASC, - catenumpb.IndexColumn_ASC, - catenumpb.IndexColumn_ASC, - catenumpb.IndexColumn_ASC, - }, - KeyColumnIDs: []descpb.ColumnID{11, 1, 2, 3, 4, 5, 6}, - Version: descpb.StrictIndexColumnIDGuaranteesVersion, - Sharded: catpb.ShardedDescriptor{ - IsSharded: true, - Name: "crdb_internal_aggregated_ts_app_name_fingerprint_id_node_id_plan_hash_transaction_fingerprint_id_shard_8", - ShardBuckets: 8, - ColumnNames: []string{ - "aggregated_ts", - "app_name", - "fingerprint_id", - "node_id", - "plan_hash", - "transaction_fingerprint_id", - }, - }, - }, - Indexes: []descpb.IndexDescriptor{ - { - Name: "fingerprint_stats_idx", - ID: 2, - Unique: false, - KeyColumnNames: []string{ - "fingerprint_id", - "transaction_fingerprint_id", - }, - KeyColumnDirections: []catenumpb.IndexColumn_Direction{ - catenumpb.IndexColumn_ASC, - catenumpb.IndexColumn_ASC, - }, - KeyColumnIDs: []descpb.ColumnID{2, 3}, - KeySuffixColumnIDs: []descpb.ColumnID{11, 1, 4, 5, 6}, - Version: descpb.StrictIndexColumnIDGuaranteesVersion, - }, - }, - NextIndexID: 4, - Privileges: catpb.NewCustomSuperuserPrivilegeDescriptor(privilege.ReadWriteData, username.NodeUserName()), - NextMutationID: 1, - FormatVersion: 3, - } -} diff --git a/pkg/upgrade/upgrades/database_role_settings_table_user_id_migration_test.go b/pkg/upgrade/upgrades/database_role_settings_table_user_id_migration_test.go index 85557b230b8e..5762939612c2 100644 --- a/pkg/upgrade/upgrades/database_role_settings_table_user_id_migration_test.go +++ b/pkg/upgrade/upgrades/database_role_settings_table_user_id_migration_test.go @@ -18,15 +18,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/settings/cluster" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/catenumpb" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/systemschema" - "github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants" - "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/testutils/skip" "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" "github.com/cockroachdb/cockroach/pkg/testutils/testcluster" @@ -53,17 +46,12 @@ func runTestDatabaseRoleSettingsUserIDMigration(t *testing.T, numUsers int) { defer leaktest.AfterTest(t)() ctx := context.Background() - settings := cluster.MakeTestingClusterSettingsWithVersions( - clusterversion.TestingBinaryVersion, - clusterversion.ByKey(clusterversion.V23_1DatabaseRoleSettingsHasRoleIDColumn-1), - false, /* initializeVersion */ - ) tc := testcluster.StartTestCluster(t, 1 /* nodes */, base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ - Settings: settings, Knobs: base.TestingKnobs{ Server: &server.TestingKnobs{ DisableAutomaticVersionUpgrade: make(chan struct{}), + BootstrapVersionKeyOverride: clusterversion.V22_2, BinaryVersionOverride: clusterversion.ByKey(clusterversion.V23_1DatabaseRoleSettingsHasRoleIDColumn - 1), }, }, @@ -74,11 +62,6 @@ func runTestDatabaseRoleSettingsUserIDMigration(t *testing.T, numUsers int) { db := tc.ServerConn(0) defer db.Close() tdb := sqlutils.MakeSQLRunner(db) - s := tc.Server(0) - - // Inject the descriptor for the system.database_role_settings table from - // before the role_id column was added. - upgrades.InjectLegacyTable(ctx, t, s, systemschema.DatabaseRoleSettingsTable, getTableDescForDatabaseRoleSettingsTableBeforeRoleIDCol) // Create test users and add rows for each user to system.database_role_settings. upgrades.ExecForCountInTxns(ctx, t, db, numUsers, 100 /* txCount */, func(txRunner *sqlutils.SQLRunner, i int) { @@ -122,43 +105,3 @@ func runTestDatabaseRoleSettingsUserIDMigration(t *testing.T, numUsers int) { username.EmptyRole, username.EmptyRoleID), [][]string{{"0"}}) } - -func getTableDescForDatabaseRoleSettingsTableBeforeRoleIDCol() *descpb.TableDescriptor { - return &descpb.TableDescriptor{ - Name: string(catconstants.DatabaseRoleSettingsTableName), - ID: keys.DatabaseRoleSettingsTableID, - ParentID: keys.SystemDatabaseID, - UnexposedParentSchemaID: keys.PublicSchemaID, - Version: 1, - Columns: []descpb.ColumnDescriptor{ - {Name: "database_id", ID: 1, Type: types.Oid, Nullable: false}, - {Name: "role_name", ID: 2, Type: types.String, Nullable: false}, - {Name: "settings", ID: 3, Type: types.StringArray, Nullable: false}, - }, - NextColumnID: 4, - Families: []descpb.ColumnFamilyDescriptor{ - { - Name: "primary", - ID: 0, - ColumnNames: []string{"database_id", "role_name", "settings"}, - ColumnIDs: []descpb.ColumnID{1, 2, 3}, - DefaultColumnID: 3, - }, - }, - NextFamilyID: 1, - PrimaryIndex: descpb.IndexDescriptor{ - Name: "primary", - ID: 1, - Unique: true, - KeyColumnNames: []string{"database_id", "role_name"}, - KeyColumnDirections: []catenumpb.IndexColumn_Direction{ - catenumpb.IndexColumn_ASC, catenumpb.IndexColumn_ASC, - }, - KeyColumnIDs: []descpb.ColumnID{1, 2}, - }, - NextIndexID: 2, - FormatVersion: descpb.InterleavedFormatVersion, - NextMutationID: 1, - NextConstraintID: 1, - } -} diff --git a/pkg/upgrade/upgrades/external_connections_table_user_id_migration_test.go b/pkg/upgrade/upgrades/external_connections_table_user_id_migration_test.go index 0a080425d2b8..c5d7537e30a7 100644 --- a/pkg/upgrade/upgrades/external_connections_table_user_id_migration_test.go +++ b/pkg/upgrade/upgrades/external_connections_table_user_id_migration_test.go @@ -19,17 +19,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/base" _ "github.com/cockroachdb/cockroach/pkg/cloud/userfile" "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/settings/cluster" - "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/skip" "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" "github.com/cockroachdb/cockroach/pkg/testutils/testcluster" @@ -52,19 +42,13 @@ func runTestExternalConnectionsUserIDMigration(t *testing.T, numUsers int) { defer leaktest.AfterTest(t)() ctx := context.Background() - settings := cluster.MakeTestingClusterSettingsWithVersions( - clusterversion.TestingBinaryVersion, - clusterversion.ByKey(clusterversion.V23_1ExternalConnectionsTableHasOwnerIDColumn-1), - false, /* initializeVersion */ - ) - tc := testcluster.StartTestCluster(t, 1 /* nodes */, base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ - Settings: settings, Knobs: base.TestingKnobs{ Server: &server.TestingKnobs{ DisableAutomaticVersionUpgrade: make(chan struct{}), BinaryVersionOverride: clusterversion.ByKey(clusterversion.V23_1ExternalConnectionsTableHasOwnerIDColumn - 1), + BootstrapVersionKeyOverride: clusterversion.V22_2, }, }, }, @@ -74,12 +58,6 @@ func runTestExternalConnectionsUserIDMigration(t *testing.T, numUsers int) { db := tc.ServerConn(0) defer db.Close() tdb := sqlutils.MakeSQLRunner(db) - s := tc.Server(0) - - // Inject the descriptor for system.external_connections table from before - // the owner_id column was added. - upgrades.InjectLegacyTable(ctx, t, s, systemschema.SystemExternalConnectionsTable, - getTableDescForExternalConnectionsTableBeforeOwnerIDCol) // Create test users. upgrades.ExecForCountInTxns(ctx, t, db, numUsers, 100 /* txCount */, func(txRunner *sqlutils.SQLRunner, i int) { @@ -123,45 +101,3 @@ func runTestExternalConnectionsUserIDMigration(t *testing.T, numUsers int) { tdb.CheckQueryResults(t, "SELECT count(*) FROM system.external_connections", [][]string{{strconv.Itoa(numUsers)}}) tdb.CheckQueryResults(t, "SELECT count(*) FROM system.external_connections JOIN system.users ON owner = username AND owner_id <> user_id", [][]string{{"0"}}) } - -func getTableDescForExternalConnectionsTableBeforeOwnerIDCol() *descpb.TableDescriptor { - nowString := "now():::TIMESTAMP" - return &descpb.TableDescriptor{ - Name: string(catconstants.SystemExternalConnectionsTableName), - ID: descpb.InvalidID, - ParentID: keys.SystemDatabaseID, - UnexposedParentSchemaID: keys.PublicSchemaID, - Version: 1, - Columns: []descpb.ColumnDescriptor{ - {Name: "connection_name", ID: 1, Type: types.String}, - {Name: "created", ID: 2, Type: types.Timestamp, DefaultExpr: &nowString}, - {Name: "updated", ID: 3, Type: types.Timestamp, DefaultExpr: &nowString}, - {Name: "connection_type", ID: 4, Type: types.String}, - {Name: "connection_details", ID: 5, Type: types.Bytes}, - {Name: "owner", ID: 6, Type: types.String}, - }, - NextColumnID: 7, - Families: []descpb.ColumnFamilyDescriptor{ - { - Name: "primary", - ID: 0, - ColumnNames: []string{"connection_name", "created", "updated", "connection_type", "connection_details", "owner"}, - ColumnIDs: []descpb.ColumnID{1, 2, 3, 4, 5, 6}, - }, - }, - NextFamilyID: 1, - PrimaryIndex: descpb.IndexDescriptor{ - Name: "primary", - ID: 1, - Unique: true, - KeyColumnNames: []string{"connection_name"}, - KeyColumnDirections: []catenumpb.IndexColumn_Direction{catenumpb.IndexColumn_ASC}, - KeyColumnIDs: []descpb.ColumnID{1}, - }, - NextIndexID: 2, - Privileges: catpb.NewCustomSuperuserPrivilegeDescriptor(privilege.ReadWriteData, username.NodeUserName()), - FormatVersion: descpb.InterleavedFormatVersion, - NextMutationID: 1, - NextConstraintID: 1, - } -} diff --git a/pkg/upgrade/upgrades/role_members_ids_migration_test.go b/pkg/upgrade/upgrades/role_members_ids_migration_test.go index 390551e84830..e41fd8bac7fb 100644 --- a/pkg/upgrade/upgrades/role_members_ids_migration_test.go +++ b/pkg/upgrade/upgrades/role_members_ids_migration_test.go @@ -51,7 +51,7 @@ func runTestRoleMembersIDMigration(t *testing.T, numUsers int) { Server: &server.TestingKnobs{ BootstrapVersionKeyOverride: clusterversion.V22_2, DisableAutomaticVersionUpgrade: make(chan struct{}), - BinaryVersionOverride: clusterversion.ByKey(clusterversion.V22_2), + BinaryVersionOverride: clusterversion.ByKey(clusterversion.V23_1RoleMembersTableHasIDColumns - 1), }, }, }, diff --git a/pkg/upgrade/upgrades/system_privileges_index_migration_test.go b/pkg/upgrade/upgrades/system_privileges_index_migration_test.go index e26bc245a39f..d169e068e690 100644 --- a/pkg/upgrade/upgrades/system_privileges_index_migration_test.go +++ b/pkg/upgrade/upgrades/system_privileges_index_migration_test.go @@ -16,20 +16,9 @@ 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/settings/cluster" - "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" "github.com/cockroachdb/cockroach/pkg/util/leaktest" "github.com/stretchr/testify/require" ) @@ -38,18 +27,12 @@ func TestSystemPrivilegesIndexMigration(t *testing.T) { defer leaktest.AfterTest(t)() ctx := context.Background() - settings := cluster.MakeTestingClusterSettingsWithVersions( - clusterversion.TestingBinaryVersion, - clusterversion.ByKey(clusterversion.V23_1AlterSystemPrivilegesAddIndexOnPathAndUsername-1), - false, - ) - tc := testcluster.StartTestCluster(t, 1, base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ - Settings: settings, Knobs: base.TestingKnobs{ Server: &server.TestingKnobs{ DisableAutomaticVersionUpgrade: make(chan struct{}), + BootstrapVersionKeyOverride: clusterversion.V22_2, BinaryVersionOverride: clusterversion.ByKey(clusterversion.V23_1AlterSystemPrivilegesAddIndexOnPathAndUsername - 1), }, }, @@ -60,17 +43,6 @@ func TestSystemPrivilegesIndexMigration(t *testing.T) { db := tc.ServerConn(0) defer db.Close() tdb := sqlutils.MakeSQLRunner(db) - s := tc.Server(0) - - // Inject the descriptor for the system.privileges table from before - // the index on (path,user) column was added. - upgrades.InjectLegacyTable( - ctx, - t, - s, - systemschema.SystemPrivilegeTable, - getTableDescForSystemPrivilegesTableBeforeIndexonPathUsername, - ) // Run migration. _, err := tc.Conns[0].ExecContext( @@ -85,7 +57,7 @@ func TestSystemPrivilegesIndexMigration(t *testing.T) { path STRING NOT NULL, privileges STRING[] NOT NULL, grant_options STRING[] NOT NULL, - user_id OID NULL, + user_id OID NOT NULL, CONSTRAINT "primary" PRIMARY KEY (username ASC, path ASC), UNIQUE INDEX privileges_path_user_id_key (path ASC, user_id ASC) STORING (privileges, grant_options), UNIQUE INDEX privileges_path_username_key (path ASC, username ASC) STORING (privileges, grant_options) @@ -95,57 +67,3 @@ func TestSystemPrivilegesIndexMigration(t *testing.T) { r.Scan(&actualSchema) require.Equal(t, expectedSchema, actualSchema) } - -func getTableDescForSystemPrivilegesTableBeforeIndexonPathUsername() *descpb.TableDescriptor { - return &descpb.TableDescriptor{ - Name: string(catconstants.SystemPrivilegeTableName), - ID: descpb.InvalidID, - ParentID: keys.SystemDatabaseID, - UnexposedParentSchemaID: keys.PublicSchemaID, - Version: 1, - Columns: []descpb.ColumnDescriptor{ - {Name: "username", ID: 1, Type: types.String}, - {Name: "path", ID: 2, Type: types.String}, - {Name: "privileges", ID: 3, Type: types.StringArray}, - {Name: "grant_options", ID: 4, Type: types.StringArray}, - {Name: "user_id", ID: 5, Type: types.Oid, Nullable: true}, - }, - NextColumnID: 6, - Families: []descpb.ColumnFamilyDescriptor{ - { - Name: "primary", - ID: 0, - ColumnNames: []string{"username", "path", "privileges", "grant_options", "user_id"}, - ColumnIDs: []descpb.ColumnID{1, 2, 3, 4, 5}, - }, - }, - NextFamilyID: 1, - PrimaryIndex: descpb.IndexDescriptor{ - Name: "primary", - ID: 1, - Unique: true, - KeyColumnNames: []string{"username", "path"}, - KeyColumnDirections: []catenumpb.IndexColumn_Direction{catenumpb.IndexColumn_ASC, catenumpb.IndexColumn_ASC}, - KeyColumnIDs: []descpb.ColumnID{1, 2}, - }, - Indexes: []descpb.IndexDescriptor{ - { - Name: "privileges_path_user_id_key", - ID: 2, - Unique: true, - KeyColumnNames: []string{"path", "user_id"}, - KeyColumnDirections: []catenumpb.IndexColumn_Direction{catenumpb.IndexColumn_ASC, catenumpb.IndexColumn_ASC}, - KeyColumnIDs: []descpb.ColumnID{2, 5}, - KeySuffixColumnIDs: []descpb.ColumnID{1}, - StoreColumnNames: []string{"privileges", "grant_options"}, - StoreColumnIDs: []descpb.ColumnID{3, 4}, - Version: descpb.StrictIndexColumnIDGuaranteesVersion, - }, - }, - NextIndexID: 3, - Privileges: catpb.NewCustomSuperuserPrivilegeDescriptor(privilege.ReadWriteData, username.NodeUserName()), - FormatVersion: descpb.InterleavedFormatVersion, - NextMutationID: 1, - NextConstraintID: 1, - } -} diff --git a/pkg/upgrade/upgrades/system_privileges_user_id_migration_test.go b/pkg/upgrade/upgrades/system_privileges_user_id_migration_test.go index c7e9bfc723a9..6ae9c1f9c3d0 100644 --- a/pkg/upgrade/upgrades/system_privileges_user_id_migration_test.go +++ b/pkg/upgrade/upgrades/system_privileges_user_id_migration_test.go @@ -18,17 +18,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/settings/cluster" - "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/skip" "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" "github.com/cockroachdb/cockroach/pkg/testutils/testcluster" @@ -55,20 +46,12 @@ func runTestSystemPrivilegesUserIDMigration(t *testing.T, numUsers int) { defer leaktest.AfterTest(t)() ctx := context.Background() - skip.WithIssue(t, 99974, "flaky test") - - settings := cluster.MakeTestingClusterSettingsWithVersions( - clusterversion.TestingBinaryVersion, - clusterversion.ByKey(clusterversion.V23_1SystemPrivilegesTableHasUserIDColumn-1), - false, /* initializeVersion */ - ) - tc := testcluster.StartTestCluster(t, 1 /* nodes */, base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ - Settings: settings, Knobs: base.TestingKnobs{ Server: &server.TestingKnobs{ DisableAutomaticVersionUpgrade: make(chan struct{}), + BootstrapVersionKeyOverride: clusterversion.V22_2, BinaryVersionOverride: clusterversion.ByKey(clusterversion.V23_1SystemPrivilegesTableHasUserIDColumn - 1), }, }, @@ -79,11 +62,6 @@ func runTestSystemPrivilegesUserIDMigration(t *testing.T, numUsers int) { db := tc.ServerConn(0) defer db.Close() tdb := sqlutils.MakeSQLRunner(db) - s := tc.Server(0) - - // Inject the descriptor for the system.privileges table from before the - // user_id column was added. - upgrades.InjectLegacyTable(ctx, t, s, systemschema.SystemPrivilegeTable, getTableDescForSystemPrivilegesTableBeforeUserIDCol) // Create test users and add rows for each user to system.privileges. upgrades.ExecForCountInTxns(ctx, t, db, numUsers, 100 /* txCount */, func(txRunner *sqlutils.SQLRunner, i int) { @@ -128,42 +106,3 @@ func runTestSystemPrivilegesUserIDMigration(t *testing.T, numUsers int) { username.PublicRole, username.PublicRoleID), [][]string{{"0"}}) } - -func getTableDescForSystemPrivilegesTableBeforeUserIDCol() *descpb.TableDescriptor { - return &descpb.TableDescriptor{ - Name: string(catconstants.SystemPrivilegeTableName), - ID: descpb.InvalidID, - ParentID: keys.SystemDatabaseID, - UnexposedParentSchemaID: keys.PublicSchemaID, - Version: 1, - Columns: []descpb.ColumnDescriptor{ - {Name: "username", ID: 1, Type: types.String}, - {Name: "path", ID: 2, Type: types.String}, - {Name: "privileges", ID: 3, Type: types.StringArray}, - {Name: "grant_options", ID: 4, Type: types.StringArray}, - }, - NextColumnID: 5, - Families: []descpb.ColumnFamilyDescriptor{ - { - Name: "primary", - ID: 0, - ColumnNames: []string{"username", "path", "privileges", "grant_options"}, - ColumnIDs: []descpb.ColumnID{1, 2, 3, 4}, - }, - }, - NextFamilyID: 1, - PrimaryIndex: descpb.IndexDescriptor{ - Name: "primary", - ID: 1, - Unique: true, - KeyColumnNames: []string{"username", "path"}, - KeyColumnDirections: []catenumpb.IndexColumn_Direction{catenumpb.IndexColumn_ASC, catenumpb.IndexColumn_ASC}, - KeyColumnIDs: []descpb.ColumnID{1, 2}, - }, - NextIndexID: 2, - Privileges: catpb.NewCustomSuperuserPrivilegeDescriptor(privilege.ReadWriteData, username.NodeUserName()), - FormatVersion: descpb.InterleavedFormatVersion, - NextMutationID: 1, - NextConstraintID: 1, - } -} diff --git a/pkg/upgrade/upgrades/tenant_table_migration_test.go b/pkg/upgrade/upgrades/tenant_table_migration_test.go index 36a7ebe06fc2..f46a59444c16 100644 --- a/pkg/upgrade/upgrades/tenant_table_migration_test.go +++ b/pkg/upgrade/upgrades/tenant_table_migration_test.go @@ -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" @@ -42,6 +35,7 @@ func TestUpdateTenantsTable(t *testing.T) { Knobs: base.TestingKnobs{ Server: &server.TestingKnobs{ DisableAutomaticVersionUpgrade: make(chan struct{}), + BootstrapVersionKeyOverride: clusterversion.V22_2, BinaryVersionOverride: clusterversion.ByKey( clusterversion.V23_1TenantNamesStateAndServiceMode - 1), }, @@ -82,8 +76,6 @@ func TestUpdateTenantsTable(t *testing.T) { ) require.NoError(t, err) - // Inject the old copy of the descriptor. - upgrades.InjectLegacyTable(ctx, t, s, systemschema.TenantsTable, getDeprecatedTenantsDescriptor) // Validate that the table sql_instances has the old schema. upgrades.ValidateSchemaExists( ctx, @@ -123,46 +115,3 @@ func TestUpdateTenantsTable(t *testing.T) { _, err = sqlDB.Exec("ALTER TENANT foo START SERVICE SHARED") require.NoError(t, err) } - -// getDeprecatedTenantsDescriptor returns the system.tenants -// table descriptor that was being used before adding a new column in the -// current version. -func getDeprecatedTenantsDescriptor() *descpb.TableDescriptor { - trueBoolString := "true" - return &descpb.TableDescriptor{ - Name: string(catconstants.TenantsTableName), - ID: keys.TenantsTableID, - ParentID: keys.SystemDatabaseID, - UnexposedParentSchemaID: keys.PublicSchemaID, - Version: 1, - Columns: []descpb.ColumnDescriptor{ - {Name: "id", ID: 1, Type: types.Int}, - {Name: "active", ID: 2, Type: types.Bool, DefaultExpr: &trueBoolString}, - {Name: "info", ID: 3, Type: types.Bytes, Nullable: true}, - }, - NextColumnID: 4, - Families: []descpb.ColumnFamilyDescriptor{ - { - Name: "primary", - ID: 0, - ColumnNames: []string{"id", "active", "info"}, - ColumnIDs: []descpb.ColumnID{1, 2, 3}, - }, - }, - NextFamilyID: 1, - PrimaryIndex: descpb.IndexDescriptor{ - Name: "id", - ID: 1, - ConstraintID: 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, - NextConstraintID: 2, - FormatVersion: descpb.InterleavedFormatVersion, - } -} diff --git a/pkg/upgrade/upgrades/web_sessions_table_user_id_migration_test.go b/pkg/upgrade/upgrades/web_sessions_table_user_id_migration_test.go index d877a0160137..b568ce8897ae 100644 --- a/pkg/upgrade/upgrades/web_sessions_table_user_id_migration_test.go +++ b/pkg/upgrade/upgrades/web_sessions_table_user_id_migration_test.go @@ -18,17 +18,7 @@ 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/settings/cluster" - "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/skip" "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" "github.com/cockroachdb/cockroach/pkg/testutils/testcluster" @@ -55,18 +45,12 @@ func runTestWebSessionsUserIDMigration(t *testing.T, numUsers int) { defer leaktest.AfterTest(t)() ctx := context.Background() - settings := cluster.MakeTestingClusterSettingsWithVersions( - clusterversion.TestingBinaryVersion, - clusterversion.ByKey(clusterversion.V23_1WebSessionsTableHasUserIDColumn-1), - false, /* initializeVersion */ - ) - tc := testcluster.StartTestCluster(t, 1 /* nodes */, base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ - Settings: settings, Knobs: base.TestingKnobs{ Server: &server.TestingKnobs{ DisableAutomaticVersionUpgrade: make(chan struct{}), + BootstrapVersionKeyOverride: clusterversion.V22_2, BinaryVersionOverride: clusterversion.ByKey(clusterversion.V23_1WebSessionsTableHasUserIDColumn - 1), }, }, @@ -77,11 +61,6 @@ func runTestWebSessionsUserIDMigration(t *testing.T, numUsers int) { db := tc.ServerConn(0) defer db.Close() tdb := sqlutils.MakeSQLRunner(db) - s := tc.Server(0) - - // Inject the descriptor for the system.web_sessions table from before the - // user_id column was added. - upgrades.InjectLegacyTable(ctx, t, s, systemschema.WebSessionsTable, getTableDescForSystemWebSessionsTableBeforeUserIDCol) // Create test users. upgrades.ExecForCountInTxns(ctx, t, db, numUsers, 100 /* txCount */, func(txRunner *sqlutils.SQLRunner, i int) { @@ -137,99 +116,3 @@ VALUES ( tdb.CheckQueryResults(t, "SELECT count(*) FROM system.web_sessions", [][]string{{strconv.Itoa(numUsers)}}) tdb.CheckQueryResults(t, "SELECT count(*) FROM system.web_sessions AS a JOIN system.users AS b ON a.username = b.username AND a.user_id <> b.user_id", [][]string{{"0"}}) } - -func getTableDescForSystemWebSessionsTableBeforeUserIDCol() *descpb.TableDescriptor { - uniqueRowIDString := "unique_rowid()" - nowString := "now():::TIMESTAMP" - return &descpb.TableDescriptor{ - Name: string(catconstants.WebSessionsTableName), - ID: keys.WebSessionsTableID, - ParentID: keys.SystemDatabaseID, - UnexposedParentSchemaID: keys.PublicSchemaID, - Version: 1, - Columns: []descpb.ColumnDescriptor{ - {Name: "id", ID: 1, Type: types.Int, DefaultExpr: &uniqueRowIDString}, - {Name: "hashedSecret", ID: 2, Type: types.Bytes}, - {Name: "username", ID: 3, Type: types.String}, - {Name: "createdAt", ID: 4, Type: types.Timestamp, DefaultExpr: &nowString}, - {Name: "expiresAt", ID: 5, Type: types.Timestamp}, - {Name: "revokedAt", ID: 6, Type: types.Timestamp, Nullable: true}, - {Name: "lastUsedAt", ID: 7, Type: types.Timestamp, DefaultExpr: &nowString}, - {Name: "auditInfo", ID: 8, Type: types.String, Nullable: true}, - }, - NextColumnID: 9, - Families: []descpb.ColumnFamilyDescriptor{ - { - Name: "fam_0_id_hashedSecret_username_createdAt_expiresAt_revokedAt_lastUsedAt_auditInfo", - ID: 0, - ColumnNames: []string{ - "id", - "hashedSecret", - "username", - "createdAt", - "expiresAt", - "revokedAt", - "lastUsedAt", - "auditInfo", - }, - ColumnIDs: []descpb.ColumnID{1, 2, 3, 4, 5, 6, 7, 8}, - }, - }, - NextFamilyID: 1, - PrimaryIndex: descpb.IndexDescriptor{ - Name: "primary", - ID: 1, - Unique: true, - KeyColumnNames: []string{"id"}, - KeyColumnDirections: []catenumpb.IndexColumn_Direction{catenumpb.IndexColumn_ASC}, - KeyColumnIDs: []descpb.ColumnID{1}, - }, - Indexes: []descpb.IndexDescriptor{ - { - Name: "web_sessions_expiresAt_idx", - ID: 2, - Unique: false, - KeyColumnNames: []string{"expiresAt"}, - KeyColumnDirections: []catenumpb.IndexColumn_Direction{catenumpb.IndexColumn_ASC}, - KeyColumnIDs: []descpb.ColumnID{5}, - KeySuffixColumnIDs: []descpb.ColumnID{1}, - Version: descpb.StrictIndexColumnIDGuaranteesVersion, - }, - { - Name: "web_sessions_createdAt_idx", - ID: 3, - Unique: false, - KeyColumnNames: []string{"createdAt"}, - KeyColumnDirections: []catenumpb.IndexColumn_Direction{catenumpb.IndexColumn_ASC}, - KeyColumnIDs: []descpb.ColumnID{4}, - KeySuffixColumnIDs: []descpb.ColumnID{1}, - Version: descpb.StrictIndexColumnIDGuaranteesVersion, - }, - { - Name: "web_sessions_revokedAt_idx", - ID: 4, - Unique: false, - KeyColumnNames: []string{"revokedAt"}, - KeyColumnDirections: []catenumpb.IndexColumn_Direction{catenumpb.IndexColumn_ASC}, - KeyColumnIDs: []descpb.ColumnID{6}, - KeySuffixColumnIDs: []descpb.ColumnID{1}, - Version: descpb.StrictIndexColumnIDGuaranteesVersion, - }, - { - Name: "web_sessions_lastUsedAt_idx", - ID: 5, - Unique: false, - KeyColumnNames: []string{"lastUsedAt"}, - KeyColumnDirections: []catenumpb.IndexColumn_Direction{catenumpb.IndexColumn_ASC}, - KeyColumnIDs: []descpb.ColumnID{7}, - KeySuffixColumnIDs: []descpb.ColumnID{1}, - Version: descpb.StrictIndexColumnIDGuaranteesVersion, - }, - }, - NextIndexID: 6, - Privileges: catpb.NewCustomSuperuserPrivilegeDescriptor(privilege.ReadWriteData, username.NodeUserName()), - FormatVersion: descpb.InterleavedFormatVersion, - NextMutationID: 1, - NextConstraintID: 1, - } -}