fix: deterministic index ordering when migrating (#7208)

Issue: We observed that, when creating a database based on the same
gORM schema multiple times, indexes could appear in different orders,
hurting determinism for use-cases like schema comparison.

In order to fix this, it's simple to switch the ParseIndexes function
to return a list of indices rather than a map, so the callers will
iterate in deterministic order.
This commit is contained in:
Bennett Amodio 2024-12-05 18:27:44 -08:00 committed by GitHub
parent 6bfccf8afa
commit f482f25c71
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 62 additions and 60 deletions

View File

@ -27,8 +27,9 @@ type IndexOption struct {
} }
// ParseIndexes parse schema indexes // ParseIndexes parse schema indexes
func (schema *Schema) ParseIndexes() map[string]Index { func (schema *Schema) ParseIndexes() []*Index {
indexes := map[string]Index{} indexesByName := map[string]*Index{}
indexes := []*Index{}
for _, field := range schema.Fields { for _, field := range schema.Fields {
if field.TagSettings["INDEX"] != "" || field.TagSettings["UNIQUEINDEX"] != "" { if field.TagSettings["INDEX"] != "" || field.TagSettings["UNIQUEINDEX"] != "" {
@ -38,7 +39,12 @@ func (schema *Schema) ParseIndexes() map[string]Index {
break break
} }
for _, index := range fieldIndexes { for _, index := range fieldIndexes {
idx := indexes[index.Name] idx := indexesByName[index.Name]
if idx == nil {
idx = &Index{Name: index.Name}
indexesByName[index.Name] = idx
indexes = append(indexes, idx)
}
idx.Name = index.Name idx.Name = index.Name
if idx.Class == "" { if idx.Class == "" {
idx.Class = index.Class idx.Class = index.Class
@ -60,8 +66,6 @@ func (schema *Schema) ParseIndexes() map[string]Index {
sort.Slice(idx.Fields, func(i, j int) bool { sort.Slice(idx.Fields, func(i, j int) bool {
return idx.Fields[i].priority < idx.Fields[j].priority return idx.Fields[i].priority < idx.Fields[j].priority
}) })
indexes[index.Name] = idx
} }
} }
} }
@ -76,15 +80,14 @@ func (schema *Schema) ParseIndexes() map[string]Index {
func (schema *Schema) LookIndex(name string) *Index { func (schema *Schema) LookIndex(name string) *Index {
if schema != nil { if schema != nil {
indexes := schema.ParseIndexes() indexes := schema.ParseIndexes()
for _, index := range indexes {
if index, found := indexes[name]; found { if index.Name == name {
return &index return index
} }
for _, index := range indexes {
for _, field := range index.Fields { for _, field := range index.Fields {
if field.Name == name { if field.Name == name {
return &index return index
} }
} }
} }

View File

@ -61,17 +61,17 @@ func TestParseIndex(t *testing.T) {
t.Fatalf("failed to parse user index, got error %v", err) t.Fatalf("failed to parse user index, got error %v", err)
} }
results := map[string]schema.Index{ results := []*schema.Index{
"idx_user_indices_name": { {
Name: "idx_user_indices_name", Name: "idx_user_indices_name",
Fields: []schema.IndexOption{{Field: &schema.Field{Name: "Name"}}}, Fields: []schema.IndexOption{{Field: &schema.Field{Name: "Name"}}},
}, },
"idx_name": { {
Name: "idx_name", Name: "idx_name",
Class: "UNIQUE", Class: "UNIQUE",
Fields: []schema.IndexOption{{Field: &schema.Field{Name: "Name2", UniqueIndex: "idx_name"}}}, Fields: []schema.IndexOption{{Field: &schema.Field{Name: "Name2", UniqueIndex: "idx_name"}}},
}, },
"idx_user_indices_name3": { {
Name: "idx_user_indices_name3", Name: "idx_user_indices_name3",
Type: "btree", Type: "btree",
Where: "name3 != 'jinzhu'", Where: "name3 != 'jinzhu'",
@ -82,19 +82,19 @@ func TestParseIndex(t *testing.T) {
Length: 10, Length: 10,
}}, }},
}, },
"idx_user_indices_name4": { {
Name: "idx_user_indices_name4", Name: "idx_user_indices_name4",
Class: "UNIQUE", Class: "UNIQUE",
Fields: []schema.IndexOption{{Field: &schema.Field{Name: "Name4", UniqueIndex: "idx_user_indices_name4"}}}, Fields: []schema.IndexOption{{Field: &schema.Field{Name: "Name4", UniqueIndex: "idx_user_indices_name4"}}},
}, },
"idx_user_indices_name5": { {
Name: "idx_user_indices_name5", Name: "idx_user_indices_name5",
Class: "FULLTEXT", Class: "FULLTEXT",
Comment: "hello , world", Comment: "hello , world",
Where: "age > 10", Where: "age > 10",
Fields: []schema.IndexOption{{Field: &schema.Field{Name: "Name5"}}}, Fields: []schema.IndexOption{{Field: &schema.Field{Name: "Name5"}}},
}, },
"profile": { {
Name: "profile", Name: "profile",
Comment: "hello , world", Comment: "hello , world",
Where: "age > 10", Where: "age > 10",
@ -104,21 +104,21 @@ func TestParseIndex(t *testing.T) {
Expression: "ABS(age)", Expression: "ABS(age)",
}}, }},
}, },
"idx_id": { {
Name: "idx_id", Name: "idx_id",
Fields: []schema.IndexOption{{Field: &schema.Field{Name: "MemberNumber"}}, {Field: &schema.Field{Name: "OID", UniqueIndex: "idx_oid"}}}, Fields: []schema.IndexOption{{Field: &schema.Field{Name: "MemberNumber"}}, {Field: &schema.Field{Name: "OID", UniqueIndex: "idx_oid"}}},
}, },
"idx_oid": { {
Name: "idx_oid", Name: "idx_oid",
Class: "UNIQUE", Class: "UNIQUE",
Fields: []schema.IndexOption{{Field: &schema.Field{Name: "OID", UniqueIndex: "idx_oid"}}}, Fields: []schema.IndexOption{{Field: &schema.Field{Name: "OID", UniqueIndex: "idx_oid"}}},
}, },
"type": { {
Name: "type", Name: "type",
Type: "", Type: "",
Fields: []schema.IndexOption{{Field: &schema.Field{Name: "Name7"}}}, Fields: []schema.IndexOption{{Field: &schema.Field{Name: "Name7"}}},
}, },
"idx_user_indices_name8": { {
Name: "idx_user_indices_name8", Name: "idx_user_indices_name8",
Type: "", Type: "",
Fields: []schema.IndexOption{ Fields: []schema.IndexOption{
@ -127,7 +127,16 @@ func TestParseIndex(t *testing.T) {
{Field: &schema.Field{Name: "Name8"}, Collate: "utf8"}, {Field: &schema.Field{Name: "Name8"}, Collate: "utf8"},
}, },
}, },
"idx_user_indices_comp_id0": { {
Class: "UNIQUE",
Name: "idx_user_indices_idx_compname_1",
Option: "NULLS NOT DISTINCT",
Fields: []schema.IndexOption{
{Field: &schema.Field{Name: "CompName1", NotNull: true}},
{Field: &schema.Field{Name: "CompName2"}},
},
},
{
Name: "idx_user_indices_comp_id0", Name: "idx_user_indices_comp_id0",
Type: "", Type: "",
Fields: []schema.IndexOption{{ Fields: []schema.IndexOption{{
@ -136,7 +145,7 @@ func TestParseIndex(t *testing.T) {
Field: &schema.Field{Name: "Data0B"}, Field: &schema.Field{Name: "Data0B"},
}}, }},
}, },
"idx_user_indices_comp_id1": { {
Name: "idx_user_indices_comp_id1", Name: "idx_user_indices_comp_id1",
Fields: []schema.IndexOption{{ Fields: []schema.IndexOption{{
Field: &schema.Field{Name: "Data1A"}, Field: &schema.Field{Name: "Data1A"},
@ -146,7 +155,7 @@ func TestParseIndex(t *testing.T) {
Field: &schema.Field{Name: "Data1C"}, Field: &schema.Field{Name: "Data1C"},
}}, }},
}, },
"idx_user_indices_comp_id2": { {
Name: "idx_user_indices_comp_id2", Name: "idx_user_indices_comp_id2",
Class: "UNIQUE", Class: "UNIQUE",
Fields: []schema.IndexOption{{ Fields: []schema.IndexOption{{
@ -157,15 +166,6 @@ func TestParseIndex(t *testing.T) {
Field: &schema.Field{Name: "Data2B"}, Field: &schema.Field{Name: "Data2B"},
}}, }},
}, },
"idx_user_indices_idx_compname_1": {
Class: "UNIQUE",
Name: "idx_user_indices_idx_compname_1",
Option: "NULLS NOT DISTINCT",
Fields: []schema.IndexOption{
{Field: &schema.Field{Name: "CompName1", NotNull: true}},
{Field: &schema.Field{Name: "CompName2"}},
},
},
} }
CheckIndices(t, results, user.ParseIndexes()) CheckIndices(t, results, user.ParseIndexes())
@ -195,17 +195,17 @@ func TestParseIndexWithUniqueIndexAndUnique(t *testing.T) {
t.Fatalf("failed to parse user index, got error %v", err) t.Fatalf("failed to parse user index, got error %v", err)
} }
indices := indexSchema.ParseIndexes() indices := indexSchema.ParseIndexes()
CheckIndices(t, map[string]schema.Index{ expectedIndices := []*schema.Index{
"idx_index_tests_field_a": { {
Name: "idx_index_tests_field_a", Name: "idx_index_tests_field_a",
Fields: []schema.IndexOption{{Field: &schema.Field{Name: "FieldA", Unique: true}}}, Fields: []schema.IndexOption{{Field: &schema.Field{Name: "FieldA", Unique: true}}},
}, },
"idx_index_tests_field_c": { {
Name: "idx_index_tests_field_c", Name: "idx_index_tests_field_c",
Class: "UNIQUE", Class: "UNIQUE",
Fields: []schema.IndexOption{{Field: &schema.Field{Name: "FieldC", UniqueIndex: "idx_index_tests_field_c"}}}, Fields: []schema.IndexOption{{Field: &schema.Field{Name: "FieldC", UniqueIndex: "idx_index_tests_field_c"}}},
}, },
"idx_index_tests_field_d": { {
Name: "idx_index_tests_field_d", Name: "idx_index_tests_field_d",
Class: "UNIQUE", Class: "UNIQUE",
Fields: []schema.IndexOption{ Fields: []schema.IndexOption{
@ -214,7 +214,7 @@ func TestParseIndexWithUniqueIndexAndUnique(t *testing.T) {
{Field: &schema.Field{Name: "FieldD"}}, {Field: &schema.Field{Name: "FieldD"}},
}, },
}, },
"uniq_field_e1_e2": { {
Name: "uniq_field_e1_e2", Name: "uniq_field_e1_e2",
Class: "UNIQUE", Class: "UNIQUE",
Fields: []schema.IndexOption{ Fields: []schema.IndexOption{
@ -222,11 +222,7 @@ func TestParseIndexWithUniqueIndexAndUnique(t *testing.T) {
{Field: &schema.Field{Name: "FieldE2"}}, {Field: &schema.Field{Name: "FieldE2"}},
}, },
}, },
"idx_index_tests_field_f1": { {
Name: "idx_index_tests_field_f1",
Fields: []schema.IndexOption{{Field: &schema.Field{Name: "FieldF1"}}},
},
"uniq_field_f1_f2": {
Name: "uniq_field_f1_f2", Name: "uniq_field_f1_f2",
Class: "UNIQUE", Class: "UNIQUE",
Fields: []schema.IndexOption{ Fields: []schema.IndexOption{
@ -234,12 +230,16 @@ func TestParseIndexWithUniqueIndexAndUnique(t *testing.T) {
{Field: &schema.Field{Name: "FieldF2"}}, {Field: &schema.Field{Name: "FieldF2"}},
}, },
}, },
"idx_index_tests_field_g": { {
Name: "idx_index_tests_field_f1",
Fields: []schema.IndexOption{{Field: &schema.Field{Name: "FieldF1"}}},
},
{
Name: "idx_index_tests_field_g", Name: "idx_index_tests_field_g",
Class: "UNIQUE", Class: "UNIQUE",
Fields: []schema.IndexOption{{Field: &schema.Field{Name: "FieldG", Unique: true, UniqueIndex: "idx_index_tests_field_g"}}}, Fields: []schema.IndexOption{{Field: &schema.Field{Name: "FieldG", Unique: true, UniqueIndex: "idx_index_tests_field_g"}}},
}, },
"uniq_field_h1_h2": { {
Name: "uniq_field_h1_h2", Name: "uniq_field_h1_h2",
Class: "UNIQUE", Class: "UNIQUE",
Fields: []schema.IndexOption{ Fields: []schema.IndexOption{
@ -247,20 +247,23 @@ func TestParseIndexWithUniqueIndexAndUnique(t *testing.T) {
{Field: &schema.Field{Name: "FieldH2"}}, {Field: &schema.Field{Name: "FieldH2"}},
}, },
}, },
}, indices) }
CheckIndices(t, expectedIndices, indices)
} }
func CheckIndices(t *testing.T, expected, actual map[string]schema.Index) { func CheckIndices(t *testing.T, expected, actual []*schema.Index) {
for k, ei := range expected { if len(expected) != len(actual) {
t.Run(k, func(t *testing.T) { t.Errorf("expected %d indices, but got %d", len(expected), len(actual))
ai, ok := actual[k]
if !ok {
t.Errorf("expected index %q but actual missing", k)
return return
} }
for i, ei := range expected {
t.Run(ei.Name, func(t *testing.T) {
ai := actual[i]
tests.AssertObjEqual(t, ai, ei, "Name", "Class", "Type", "Where", "Comment", "Option") tests.AssertObjEqual(t, ai, ei, "Name", "Class", "Type", "Where", "Comment", "Option")
if len(ei.Fields) != len(ai.Fields) { if len(ei.Fields) != len(ai.Fields) {
t.Errorf("expected index %q field length is %d but actual %d", k, len(ei.Fields), len(ai.Fields)) t.Errorf("expected index %q field length is %d but actual %d", ei.Name, len(ei.Fields), len(ai.Fields))
return return
} }
for i, ef := range ei.Fields { for i, ef := range ei.Fields {
@ -268,9 +271,5 @@ func CheckIndices(t *testing.T, expected, actual map[string]schema.Index) {
tests.AssertObjEqual(t, af, ef, "Name", "Unique", "UniqueIndex", "Expression", "Sort", "Collate", "Length", "NotNull") tests.AssertObjEqual(t, af, ef, "Name", "Unique", "UniqueIndex", "Expression", "Sort", "Collate", "Length", "NotNull")
} }
}) })
delete(actual, k)
}
for k := range actual {
t.Errorf("unexpected index %q", k)
} }
} }