From f482f25c714b67562af1d487a669d80a527963b3 Mon Sep 17 00:00:00 2001 From: Bennett Amodio Date: Thu, 5 Dec 2024 18:27:44 -0800 Subject: [PATCH] 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. --- schema/index.go | 25 +++++++----- schema/index_test.go | 97 ++++++++++++++++++++++---------------------- 2 files changed, 62 insertions(+), 60 deletions(-) diff --git a/schema/index.go b/schema/index.go index a90153f8..786e2104 100644 --- a/schema/index.go +++ b/schema/index.go @@ -27,8 +27,9 @@ type IndexOption struct { } // ParseIndexes parse schema indexes -func (schema *Schema) ParseIndexes() map[string]Index { - indexes := map[string]Index{} +func (schema *Schema) ParseIndexes() []*Index { + indexesByName := map[string]*Index{} + indexes := []*Index{} for _, field := range schema.Fields { if field.TagSettings["INDEX"] != "" || field.TagSettings["UNIQUEINDEX"] != "" { @@ -38,7 +39,12 @@ func (schema *Schema) ParseIndexes() map[string]Index { break } 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 if idx.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 { 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 { if schema != nil { indexes := schema.ParseIndexes() - - if index, found := indexes[name]; found { - return &index - } - for _, index := range indexes { + if index.Name == name { + return index + } + for _, field := range index.Fields { if field.Name == name { - return &index + return index } } } diff --git a/schema/index_test.go b/schema/index_test.go index 9971562d..5c970d1b 100644 --- a/schema/index_test.go +++ b/schema/index_test.go @@ -61,17 +61,17 @@ func TestParseIndex(t *testing.T) { t.Fatalf("failed to parse user index, got error %v", err) } - results := map[string]schema.Index{ - "idx_user_indices_name": { + results := []*schema.Index{ + { Name: "idx_user_indices_name", Fields: []schema.IndexOption{{Field: &schema.Field{Name: "Name"}}}, }, - "idx_name": { + { Name: "idx_name", Class: "UNIQUE", Fields: []schema.IndexOption{{Field: &schema.Field{Name: "Name2", UniqueIndex: "idx_name"}}}, }, - "idx_user_indices_name3": { + { Name: "idx_user_indices_name3", Type: "btree", Where: "name3 != 'jinzhu'", @@ -82,19 +82,19 @@ func TestParseIndex(t *testing.T) { Length: 10, }}, }, - "idx_user_indices_name4": { + { Name: "idx_user_indices_name4", Class: "UNIQUE", Fields: []schema.IndexOption{{Field: &schema.Field{Name: "Name4", UniqueIndex: "idx_user_indices_name4"}}}, }, - "idx_user_indices_name5": { + { Name: "idx_user_indices_name5", Class: "FULLTEXT", Comment: "hello , world", Where: "age > 10", Fields: []schema.IndexOption{{Field: &schema.Field{Name: "Name5"}}}, }, - "profile": { + { Name: "profile", Comment: "hello , world", Where: "age > 10", @@ -104,21 +104,21 @@ func TestParseIndex(t *testing.T) { Expression: "ABS(age)", }}, }, - "idx_id": { + { Name: "idx_id", Fields: []schema.IndexOption{{Field: &schema.Field{Name: "MemberNumber"}}, {Field: &schema.Field{Name: "OID", UniqueIndex: "idx_oid"}}}, }, - "idx_oid": { + { Name: "idx_oid", Class: "UNIQUE", Fields: []schema.IndexOption{{Field: &schema.Field{Name: "OID", UniqueIndex: "idx_oid"}}}, }, - "type": { + { Name: "type", Type: "", Fields: []schema.IndexOption{{Field: &schema.Field{Name: "Name7"}}}, }, - "idx_user_indices_name8": { + { Name: "idx_user_indices_name8", Type: "", Fields: []schema.IndexOption{ @@ -127,7 +127,16 @@ func TestParseIndex(t *testing.T) { {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", Type: "", Fields: []schema.IndexOption{{ @@ -136,7 +145,7 @@ func TestParseIndex(t *testing.T) { Field: &schema.Field{Name: "Data0B"}, }}, }, - "idx_user_indices_comp_id1": { + { Name: "idx_user_indices_comp_id1", Fields: []schema.IndexOption{{ Field: &schema.Field{Name: "Data1A"}, @@ -146,7 +155,7 @@ func TestParseIndex(t *testing.T) { Field: &schema.Field{Name: "Data1C"}, }}, }, - "idx_user_indices_comp_id2": { + { Name: "idx_user_indices_comp_id2", Class: "UNIQUE", Fields: []schema.IndexOption{{ @@ -157,15 +166,6 @@ func TestParseIndex(t *testing.T) { 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()) @@ -195,17 +195,17 @@ func TestParseIndexWithUniqueIndexAndUnique(t *testing.T) { t.Fatalf("failed to parse user index, got error %v", err) } indices := indexSchema.ParseIndexes() - CheckIndices(t, map[string]schema.Index{ - "idx_index_tests_field_a": { + expectedIndices := []*schema.Index{ + { Name: "idx_index_tests_field_a", Fields: []schema.IndexOption{{Field: &schema.Field{Name: "FieldA", Unique: true}}}, }, - "idx_index_tests_field_c": { + { Name: "idx_index_tests_field_c", Class: "UNIQUE", 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", Class: "UNIQUE", Fields: []schema.IndexOption{ @@ -214,7 +214,7 @@ func TestParseIndexWithUniqueIndexAndUnique(t *testing.T) { {Field: &schema.Field{Name: "FieldD"}}, }, }, - "uniq_field_e1_e2": { + { Name: "uniq_field_e1_e2", Class: "UNIQUE", Fields: []schema.IndexOption{ @@ -222,11 +222,7 @@ func TestParseIndexWithUniqueIndexAndUnique(t *testing.T) { {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", Class: "UNIQUE", Fields: []schema.IndexOption{ @@ -234,12 +230,16 @@ func TestParseIndexWithUniqueIndexAndUnique(t *testing.T) { {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", Class: "UNIQUE", 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", Class: "UNIQUE", Fields: []schema.IndexOption{ @@ -247,20 +247,23 @@ func TestParseIndexWithUniqueIndexAndUnique(t *testing.T) { {Field: &schema.Field{Name: "FieldH2"}}, }, }, - }, indices) + } + CheckIndices(t, expectedIndices, indices) } -func CheckIndices(t *testing.T, expected, actual map[string]schema.Index) { - for k, ei := range expected { - t.Run(k, func(t *testing.T) { - ai, ok := actual[k] - if !ok { - t.Errorf("expected index %q but actual missing", k) - return - } +func CheckIndices(t *testing.T, expected, actual []*schema.Index) { + if len(expected) != len(actual) { + t.Errorf("expected %d indices, but got %d", len(expected), len(actual)) + 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") + 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 } 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") } }) - delete(actual, k) - } - for k := range actual { - t.Errorf("unexpected index %q", k) } }