Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: fix migration with new system.table_statistics column #78302

Merged
merged 1 commit into from
Mar 25, 2022
+59 −11
Merged

sql: fix migration with new system.table_statistics column #78302

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion pkg/migration/migrations/alter_table_statistics_avg_size.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ import (
)

const addAvgSizeCol = `
ALTER TABLE system.table_statistics
ALTER TABLE system.table_statistics
ADD COLUMN IF NOT EXISTS "avgSize" INT8 NOT NULL DEFAULT (INT8 '0')
FAMILY "fam_0_tableID_statisticID_name_columnIDs_createdAt_rowCount_distinctCount_nullCount_histogram"
`

func alterSystemTableStatisticsAddAvgSize(
Expand Down
2 changes: 2 additions & 0 deletions pkg/migration/migrations/alter_table_statistics_avg_size_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ func TestAlterSystemTableStatisticsTable(t *testing.T) {
var (
validationSchemas = []migrations.Schema{
{Name: "avgSize", ValidationFn: migrations.HasColumn},
{Name: "fam_0_tableID_statisticID_name_columnIDs_createdAt_rowCount_distinctCount_nullCount_histogram",
ValidationFn: migrations.HasColumnFamily},
}
)

Expand Down
1 change: 1 addition & 0 deletions pkg/migration/migrations/helpers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
var (
HasColumn = hasColumn
HasIndex = hasIndex
HasColumnFamily = hasColumnFamily
CreateSystemTable = createSystemTable
)

Expand Down
44 changes: 44 additions & 0 deletions pkg/migration/migrations/schema_changes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,3 +248,47 @@ func hasIndex(storedTable, expectedTable catalog.TableDescriptor, indexName stri
}
return true, nil
}

// hasColumnFamily returns true if storedTable already has the given column
// family, comparing with expectedTable. storedTable descriptor must be read
// from system storage as compared to reading from the systemschema package. On
// the contrary, expectedTable must be accessed directly from systemschema
// package. This function returns an error if the column doesn't exist in the
// expectedTable descriptor.
func hasColumnFamily(
storedTable, expectedTable catalog.TableDescriptor, colFamily string,
) (bool, error) {
var storedFamily, expectedFamily *descpb.ColumnFamilyDescriptor
for _, fam := range storedTable.GetFamilies() {
if fam.Name == colFamily {
storedFamily = &fam
break
}
}
if storedFamily == nil {
return false, nil
}

for _, fam := range expectedTable.GetFamilies() {
if fam.Name == colFamily {
expectedFamily = &fam
break
}
}
if expectedFamily == nil {
return false, errors.Errorf("column family %s does not exist", colFamily)
}

// Check that columns match.
storedFamilyCols := storedFamily.ColumnNames
expectedFamilyCols := expectedFamily.ColumnNames
if len(storedFamilyCols) != len(expectedFamilyCols) {
return false, nil
}
for i, storedCol := range storedFamilyCols {
if storedCol != expectedFamilyCols[i] {
return false, nil
}
}
return true, nil
}
12 changes: 6 additions & 6 deletions pkg/sql/catalog/systemschema/system.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,10 +251,10 @@ CREATE TABLE system.table_statistics (
"rowCount" INT8 NOT NULL,
"distinctCount" INT8 NOT NULL,
"nullCount" INT8 NOT NULL,
"avgSize" INT8 NOT NULL DEFAULT 0,
histogram BYTES,
"avgSize" INT8 NOT NULL DEFAULT 0,
CONSTRAINT "primary" PRIMARY KEY ("tableID", "statisticID"),
FAMILY ("tableID", "statisticID", name, "columnIDs", "createdAt", "rowCount", "distinctCount", "nullCount", "avgSize", histogram)
FAMILY "fam_0_tableID_statisticID_name_columnIDs_createdAt_rowCount_distinctCount_nullCount_histogram" ("tableID", "statisticID", name, "columnIDs", "createdAt", "rowCount", "distinctCount", "nullCount", histogram, "avgSize")
);`

// locations are used to map a locality specified by a node to geographic
Expand Down Expand Up @@ -1311,12 +1311,12 @@ var (
{Name: "rowCount", ID: 6, Type: types.Int},
{Name: "distinctCount", ID: 7, Type: types.Int},
{Name: "nullCount", ID: 8, Type: types.Int},
{Name: "avgSize", ID: 9, Type: types.Int, DefaultExpr: &zeroIntString},
{Name: "histogram", ID: 10, Type: types.Bytes, Nullable: true},
{Name: "histogram", ID: 9, Type: types.Bytes, Nullable: true},
{Name: "avgSize", ID: 10, Type: types.Int, DefaultExpr: &zeroIntString},
},
[]descpb.ColumnFamilyDescriptor{
{
Name: "fam_0_tableID_statisticID_name_columnIDs_createdAt_rowCount_distinctCount_nullCount_avgSize_histogram",
Name: "fam_0_tableID_statisticID_name_columnIDs_createdAt_rowCount_distinctCount_nullCount_histogram",
ID: 0,
ColumnNames: []string{
"tableID",
Expand All @@ -1327,8 +1327,8 @@ var (
"rowCount",
"distinctCount",
"nullCount",
"avgSize",
"histogram",
"avgSize",
},
ColumnIDs: []descpb.ColumnID{1, 2, 3, 4, 5, 6, 7, 8, 9, 10},
},
Expand Down
8 changes: 4 additions & 4 deletions pkg/sql/logictest/testdata/logic_test/information_schema
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1601,14 +1601,14 @@ system public 630200280_42_8_not_null
system public 630200280_42_9_not_null system public statement_statistics CHECK NO NO
system public check_crdb_internal_aggregated_ts_app_name_fingerprint_id_node_id_plan_hash_transaction_fingerprint_id_shard_8 system public statement_statistics CHECK NO NO
system public primary system public statement_statistics PRIMARY KEY NO NO
system public 630200280_20_10_not_null system public table_statistics CHECK NO NO
system public 630200280_20_1_not_null system public table_statistics CHECK NO NO
system public 630200280_20_2_not_null system public table_statistics CHECK NO NO
system public 630200280_20_4_not_null system public table_statistics CHECK NO NO
system public 630200280_20_5_not_null system public table_statistics CHECK NO NO
system public 630200280_20_6_not_null system public table_statistics CHECK NO NO
system public 630200280_20_7_not_null system public table_statistics CHECK NO NO
system public 630200280_20_8_not_null system public table_statistics CHECK NO NO
system public 630200280_20_9_not_null system public table_statistics CHECK NO NO
system public primary system public table_statistics PRIMARY KEY NO NO
system public 630200280_50_1_not_null system public tenant_settings CHECK NO NO
system public 630200280_50_2_not_null system public tenant_settings CHECK NO NO
Expand Down Expand Up @@ -1693,14 +1693,14 @@ system public 630200280_19_3_not_null
system public 630200280_19_4_not_null createdAt IS NOT NULL
system public 630200280_19_5_not_null expiresAt IS NOT NULL
system public 630200280_19_7_not_null lastUsedAt IS NOT NULL
system public 630200280_20_10_not_null avgSize IS NOT NULL
system public 630200280_20_1_not_null tableID IS NOT NULL
system public 630200280_20_2_not_null statisticID IS NOT NULL
system public 630200280_20_4_not_null columnIDs IS NOT NULL
system public 630200280_20_5_not_null createdAt IS NOT NULL
system public 630200280_20_6_not_null rowCount IS NOT NULL
system public 630200280_20_7_not_null distinctCount IS NOT NULL
system public 630200280_20_8_not_null nullCount IS NOT NULL
system public 630200280_20_9_not_null avgSize IS NOT NULL
system public 630200280_21_1_not_null localityKey IS NOT NULL
system public 630200280_21_2_not_null localityValue IS NOT NULL
system public 630200280_21_3_not_null latitude IS NOT NULL
Expand Down Expand Up @@ -2150,11 +2150,11 @@ system public statement_statistics plan
system public statement_statistics plan_hash 4
system public statement_statistics statistics 9
system public statement_statistics transaction_fingerprint_id 3
system public table_statistics avgSize 9
system public table_statistics avgSize 10
system public table_statistics columnIDs 4
system public table_statistics createdAt 5
system public table_statistics distinctCount 7
system public table_statistics histogram 10
system public table_statistics histogram 9
system public table_statistics name 3
system public table_statistics nullCount 8
system public table_statistics rowCount 6
Expand Down