Match declared and existing indexes one to one during sync
Sync paired each declared index with the first existing index that had an equal definition and did not mark that index as taken. Two declared indexes with the same definition could therefore claim the same database index: the other one was dropped as unmatched and never recreated, or a missing one was never added. Claim an existing index with the same name and an equal definition first, then let the remaining declared indexes fall back to any unclaimed existing index with an equal definition, so renamed indexes are still kept as before. Iterate in name order so the result no longer depends on map order.
This commit is contained in:
1 parent
ebe852f332
commit
0a2e162014
3 files changed
+291
-20
No files matched your search
@@ -6,6 +6,7 @@ package xorm
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"sort"
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
"xorm.io/xorm/dialects"
|
"xorm.io/xorm/dialects"
|
||||||
@@ -451,6 +452,53 @@ func schemaTableForSync(table *schemas.Table) *schemas.Table {
|
|||||||
return filtered
|
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
|
// Sync the new struct changes to database, this method will automatically add
|
||||||
// table, column, index, unique. but will not delete or change anything.
|
// table, column, index, unique. but will not delete or change anything.
|
||||||
// If you change some field, you should change the database manually.
|
// 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, addedNames := matchIndexes(syncTable.Indexes, oriTable.Indexes)
|
||||||
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
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// drop all indices that do not exist in new schema or have changed
|
// drop all indices that do not exist in new schema or have changed
|
||||||
for name2, index2 := range oriTable.Indexes {
|
for name2, index2 := range oriTable.Indexes {
|
||||||
|
|||||||
@@ -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))
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -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"])
|
||||||
|
}
|
||||||
Reference in new issue
Block a user