Skip to content

Commit

Permalink
fix: deterministic index ordering when migrating (#7208)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@bamo
bamo authored Dec 6, 2024
1 parent 6bfccf8 commit f482f25
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 60 deletions.
25 changes: 14 additions & 11 deletions schema/index.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"] != "" {
Expand All @@ -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
Expand All @@ -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
}
}
}
Expand All @@ -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
}
}
}
Expand Down
97 changes: 48 additions & 49 deletions schema/index_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'",
Expand All @@ -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",
Expand All @@ -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{
Expand All @@ -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{{
Expand All @@ -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"},
Expand All @@ -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{{
Expand All @@ -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())
Expand Down Expand Up @@ -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{
Expand All @@ -214,63 +214,62 @@ func TestParseIndexWithUniqueIndexAndUnique(t *testing.T) {
{Field: &schema.Field{Name: "FieldD"}},
},
},
"uniq_field_e1_e2": {
{
Name: "uniq_field_e1_e2",
Class: "UNIQUE",
Fields: []schema.IndexOption{
{Field: &schema.Field{Name: "FieldE1"}},
{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{
{Field: &schema.Field{Name: "FieldF1"}},
{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{
{Field: &schema.Field{Name: "FieldH1", Unique: true}},
{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 {
af := ai.Fields[i]
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)
}
}

0 comments on commit f482f25

Please sign in to comment.