diff --git a/sync.go b/sync.go index be0507c..9083751 100644 --- a/sync.go +++ b/sync.go @@ -6,6 +6,7 @@ package xorm import ( "fmt" + "sort" "strings" "xorm.io/xorm/dialects" @@ -451,6 +452,53 @@ func schemaTableForSync(table *schemas.Table) *schemas.Table { return filtered } +// matchIndexes pairs every expected index with at most one existing index. +// An existing index with the same name and an equal definition is claimed +// first, so indexes sharing a definition cannot claim each other's database +// index. The remaining expected indexes fall back to any unclaimed existing +// index with an equal definition, which keeps renamed indexes in place. +// It returns the names of the claimed existing indexes and the expected +// indexes that have no counterpart and must be created. +func matchIndexes(expected, existing map[string]*schemas.Index) (map[string]bool, map[string]*schemas.Index) { + claimed := make(map[string]bool, len(existing)) + missing := make(map[string]*schemas.Index) + + var unmatched []string + for _, name := range sortedIndexNames(expected) { + if actual, ok := existing[name]; ok && expected[name].Equal(actual) { + claimed[name] = true + continue + } + unmatched = append(unmatched, name) + } + + existingNames := sortedIndexNames(existing) + for _, name := range unmatched { + index := expected[name] + found := false + for _, existingName := range existingNames { + if !claimed[existingName] && index.Equal(existing[existingName]) { + claimed[existingName] = true + found = true + break + } + } + if !found { + missing[name] = index + } + } + return claimed, missing +} + +func sortedIndexNames(indexes map[string]*schemas.Index) []string { + names := make([]string, 0, len(indexes)) + for name := range indexes { + names = append(names, name) + } + sort.Strings(names) + return names +} + // Sync the new struct changes to database, this method will automatically add // table, column, index, unique. but will not delete or change anything. // If you change some field, you should change the database manually. @@ -595,26 +643,7 @@ func (session *Session) SyncWithOptions(opts SyncOptions, beans ...any) (*SyncRe } } - // indices found in orig table - foundIndexNames := make(map[string]bool) - // indices to be added - addedNames := make(map[string]*schemas.Index) - - // drop indices that exist in orig and new table schema but are not equal - for name, index := range syncTable.Indexes { - var oriIndex *schemas.Index - for name2, index2 := range oriTable.Indexes { - if index.Equal(index2) { - oriIndex = index2 - foundIndexNames[name2] = true - break - } - } - - if oriIndex == nil { - addedNames[name] = index - } - } + foundIndexNames, addedNames := matchIndexes(syncTable.Indexes, oriTable.Indexes) // drop all indices that do not exist in new schema or have changed for name2, index2 := range oriTable.Indexes { diff --git a/sync_index_match_test.go b/sync_index_match_test.go new file mode 100644 index 0000000..52165ed --- /dev/null +++ b/sync_index_match_test.go @@ -0,0 +1,149 @@ +// Copyright 2026 The Xorm Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package xorm + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "xorm.io/xorm/schemas" +) + +func newTestIndex(name string, indexType int, cols ...string) *schemas.Index { + index := schemas.NewIndex(name, indexType) + index.AddColumn(cols...) + return index +} + +func indexMap(indexes ...*schemas.Index) map[string]*schemas.Index { + m := make(map[string]*schemas.Index, len(indexes)) + for _, index := range indexes { + m[index.Name] = index + } + return m +} + +func TestMatchIndexes(t *testing.T) { + for _, tc := range []struct { + name string + expected map[string]*schemas.Index + existing map[string]*schemas.Index + wantClaimed []string + wantMissing []string + }{ + { + name: "same definition, both exist", + expected: indexMap( + newTestIndex("first", schemas.IndexType, "repo_id", "created_at"), + newTestIndex("second", schemas.IndexType, "repo_id", "created_at"), + ), + existing: indexMap( + newTestIndex("first", schemas.IndexType, "repo_id", "created_at"), + newTestIndex("second", schemas.IndexType, "repo_id", "created_at"), + ), + wantClaimed: []string{"first", "second"}, + }, + { + name: "same definition, first missing", + expected: indexMap( + newTestIndex("first", schemas.IndexType, "repo_id", "created_at"), + newTestIndex("second", schemas.IndexType, "repo_id", "created_at"), + ), + existing: indexMap( + newTestIndex("second", schemas.IndexType, "repo_id", "created_at"), + ), + wantClaimed: []string{"second"}, + wantMissing: []string{"first"}, + }, + { + name: "same definition, second missing", + expected: indexMap( + newTestIndex("first", schemas.IndexType, "repo_id", "created_at"), + newTestIndex("second", schemas.IndexType, "repo_id", "created_at"), + ), + existing: indexMap( + newTestIndex("first", schemas.IndexType, "repo_id", "created_at"), + ), + wantClaimed: []string{"first"}, + wantMissing: []string{"second"}, + }, + { + name: "same columns in different order", + expected: indexMap( + newTestIndex("c_u", schemas.IndexType, "user_id", "is_deleted", "created_unix"), + newTestIndex("c_u_d", schemas.IndexType, "created_unix", "user_id", "is_deleted"), + ), + existing: indexMap( + newTestIndex("c_u_d", schemas.IndexType, "created_unix", "user_id", "is_deleted"), + ), + wantClaimed: []string{"c_u_d"}, + wantMissing: []string{"c_u"}, + }, + { + name: "renamed index keeps the existing one", + expected: indexMap(newTestIndex("new_name", schemas.IndexType, "repo_id")), + existing: indexMap(newTestIndex("old_name", schemas.IndexType, "repo_id")), + wantClaimed: []string{"old_name"}, + }, + { + name: "changed definition under the same name", + expected: indexMap(newTestIndex("repo", schemas.IndexType, "repo_id", "created_at")), + existing: indexMap(newTestIndex("repo", schemas.IndexType, "created_at", "repo_id")), + wantMissing: []string{"repo"}, + }, + { + name: "index turned into a unique", + expected: indexMap(newTestIndex("repo", schemas.UniqueType, "repo_id")), + existing: indexMap(newTestIndex("repo", schemas.IndexType, "repo_id")), + wantMissing: []string{"repo"}, + }, + { + name: "same name wins over a renamed duplicate", + expected: indexMap( + newTestIndex("first", schemas.IndexType, "repo_id"), + newTestIndex("second", schemas.IndexType, "repo_id"), + ), + existing: indexMap( + newTestIndex("legacy", schemas.IndexType, "repo_id"), + newTestIndex("second", schemas.IndexType, "repo_id"), + ), + wantClaimed: []string{"legacy", "second"}, + }, + { + name: "existing index is claimed only once", + expected: indexMap( + newTestIndex("a", schemas.IndexType, "repo_id"), + newTestIndex("b", schemas.IndexType, "repo_id"), + ), + existing: indexMap(newTestIndex("legacy", schemas.IndexType, "repo_id")), + wantClaimed: []string{"legacy"}, + wantMissing: []string{"b"}, + }, + { + name: "stale index is left unclaimed", + expected: indexMap(newTestIndex("repo", schemas.IndexType, "repo_id")), + existing: indexMap( + newTestIndex("repo", schemas.IndexType, "repo_id"), + newTestIndex("stale", schemas.IndexType, "owner_id"), + ), + wantClaimed: []string{"repo"}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + for i := 0; i < 20; i++ { + claimed, missing := matchIndexes(tc.expected, tc.existing) + + var claimedNames []string + for name, ok := range claimed { + if ok { + claimedNames = append(claimedNames, name) + } + } + assert.ElementsMatch(t, tc.wantClaimed, claimedNames) + assert.ElementsMatch(t, tc.wantMissing, sortedIndexNames(missing)) + } + }) + } +} diff --git a/tests/sync_same_definition_indexes_test.go b/tests/sync_same_definition_indexes_test.go new file mode 100644 index 0000000..b028e6b --- /dev/null +++ b/tests/sync_same_definition_indexes_test.go @@ -0,0 +1,93 @@ +// Copyright 2026 The Xorm Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package tests + +import ( + "testing" + + "github.com/stretchr/testify/require" + "xorm.io/xorm/schemas" +) + +// sameDefinitionIndexes declares two indexes with an identical definition. +// Sync must keep one database index per declared index instead of letting +// both declarations claim the same database index. +type sameDefinitionIndexes struct { + Id int64 + RepoId int64 + CreatedAt int64 +} + +func (*sameDefinitionIndexes) TableIndices() []*schemas.Index { + first := schemas.NewIndex("first", schemas.IndexType) + first.AddColumn("repo_id", "created_at") + second := schemas.NewIndex("second", schemas.IndexType) + second.AddColumn("repo_id", "created_at") + return []*schemas.Index{first, second} +} + +func assertSameDefinitionIndexes(t *testing.T, bean *sameDefinitionIndexes) { + t.Helper() + tables, err := testEngine.DBMetas() + require.NoError(t, err) + require.Len(t, tables, 1) + require.Len(t, tables[0].Indexes, 2) + for _, expected := range bean.TableIndices() { + actual := tables[0].Indexes[expected.Name] + require.NotNil(t, actual, "missing index %s", expected.Name) + require.Equal(t, expected.Cols, actual.Cols) + } +} + +func TestSyncSameDefinitionIndexes(t *testing.T) { + require.NoError(t, PrepareEngine()) + bean := new(sameDefinitionIndexes) + require.NoError(t, testEngine.Sync(bean)) + assertSameDefinitionIndexes(t, bean) + + for i := 0; i < 10; i++ { + require.NoError(t, testEngine.Sync(bean)) + assertSameDefinitionIndexes(t, bean) + } + + for _, index := range bean.TableIndices() { + _, err := testEngine.Exec(testEngine.Dialect().DropIndexSQL(testEngine.TableName(bean), index)) + require.NoError(t, err) + require.NoError(t, testEngine.Sync(bean)) + assertSameDefinitionIndexes(t, bean) + } +} + +type renamedIndexBefore struct { + Id int64 + RepoId int64 `xorm:"index(old_name)"` +} + +func (*renamedIndexBefore) TableName() string { + return "renamed_index" +} + +type renamedIndexAfter struct { + Id int64 + RepoId int64 `xorm:"index(new_name)"` +} + +func (*renamedIndexAfter) TableName() string { + return "renamed_index" +} + +// TestSyncRenamedIndexKeepsExisting guards the fallback: an index whose +// declared name changed but whose definition did not is kept as it is. +func TestSyncRenamedIndexKeepsExisting(t *testing.T) { + require.NoError(t, PrepareEngine()) + require.NoError(t, testEngine.Sync(new(renamedIndexBefore))) + require.NoError(t, testEngine.Sync(new(renamedIndexAfter))) + + tables, err := testEngine.DBMetas() + require.NoError(t, err) + require.Len(t, tables, 1) + require.Len(t, tables[0].Indexes, 1) + require.NotNil(t, tables[0].Indexes["old_name"]) +}