From ebd99f1d95147c229aace13ce5339c5620e54650 Mon Sep 17 00:00:00 2001 From: Michal Fox Date: Tue, 29 Sep 2026 11:39:12 +0200 Subject: [PATCH] fix(db): keep same-column indexes in different order on sync xorm pairs declared and existing indexes by column set, ignoring name and column order, so on every sync the `action` table could drop one of `c_u` and `c_u_d`, and never recreate a missing one. Tables with such index pairs are now synced with IgnoreIndices and their plain indexes are reconciled by name. Fix https://github.com/go-gitea/gitea/issues/39244 --- models/db/engine.go | 83 ++++++++++++++++++++++++++++++++++++++-- models/db/engine_test.go | 42 ++++++++++++++++++++ 2 files changed, 121 insertions(+), 4 deletions(-) diff --git a/models/db/engine.go b/models/db/engine.go index 5cfb201..614a98b 100644 --- a/models/db/engine.go +++ b/models/db/engine.go @@ -122,10 +122,85 @@ func ResetModels() { // SyncAllTables sync the schemas of all tables, is required by unit test code func SyncAllTables() error { - _, err := xormEngine.StoreEngine("InnoDB").SyncWithOptions(xorm.SyncOptions{ - WarnIfDatabaseColumnMissed: true, - }, registeredModels...) - return err + var beans, sameColumnIndexBeans []any + var sameColumnIndexTables []*schemas.Table + for _, bean := range registeredModels { + table, err := xormEngine.TableInfo(bean) + if err != nil { + return err + } + if hasSameColumnIndexes(table) { + sameColumnIndexBeans = append(sameColumnIndexBeans, bean) + sameColumnIndexTables = append(sameColumnIndexTables, table) + } else { + beans = append(beans, bean) + } + } + + opts := xorm.SyncOptions{WarnIfDatabaseColumnMissed: true} + if _, err := xormEngine.StoreEngine("InnoDB").SyncWithOptions(opts, beans...); err != nil { + return err + } + if len(sameColumnIndexBeans) == 0 { + return nil + } + + // xorm pairs indexes by column set regardless of name and order, so it can drop one of two + // indexes over the same columns (https://github.com/go-gitea/gitea/issues/39244). Sync those by name instead. + opts.IgnoreIndices = true + if _, err := xormEngine.StoreEngine("InnoDB").SyncWithOptions(opts, sameColumnIndexBeans...); err != nil { + return err + } + for _, table := range sameColumnIndexTables { + if err := syncIndexesByName(table); err != nil { + return err + } + } + return nil +} + +func hasSameColumnIndexes(table *schemas.Table) bool { + for name, index := range table.Indexes { + for otherName, other := range table.Indexes { + if name != otherName && index.Type == schemas.IndexType && index.Equal(other) { + return true + } + } + } + return false +} + +func syncIndexesByName(table *schemas.Table) error { + dialect := xormEngine.Dialect() + tableName := dialects.TableNameWithSchema(dialect, table.Name) + existing, err := dialect.GetIndexes(xormEngine.DB(), context.Background(), table.Name) + if err != nil { + return err + } + for name, index := range existing { + if index.Type != schemas.IndexType { + continue + } + if declared, ok := table.Indexes[name]; ok && declared.Type == schemas.IndexType && declared.Equal(index) { + continue + } + if _, err := xormEngine.Exec(dialect.DropIndexSQL(tableName, index)); err != nil { + return err + } + delete(existing, name) + } + for name, index := range table.Indexes { + if index.Type != schemas.IndexType { + continue + } + if _, ok := existing[name]; ok { + continue + } + if _, err := xormEngine.Exec(dialect.CreateIndexSQL(tableName, index)); err != nil { + return err + } + } + return nil } // NamesToBean return a list of beans or an error diff --git a/models/db/engine_test.go b/models/db/engine_test.go index 69e30aa..9a51c64 100644 --- a/models/db/engine_test.go +++ b/models/db/engine_test.go @@ -7,6 +7,7 @@ import ( "path/filepath" "testing" + activities_model "gitea.dev/models/activities" "gitea.dev/models/db" issues_model "gitea.dev/models/issues" "gitea.dev/models/unittest" @@ -16,6 +17,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "xorm.io/xorm/schemas" ) func TestDumpDatabase(t *testing.T) { @@ -34,6 +36,46 @@ func TestDumpDatabase(t *testing.T) { } } +func TestSyncAllTablesKeepsSameColumnIndexes(t *testing.T) { + // action declares c_u and c_u_d with the same columns in different order + require.NoError(t, unittest.PrepareTestDatabase()) + x := db.GetXORMEngineForTesting() + + table, err := x.TableInfo(new(activities_model.Action)) + require.NoError(t, err) + declared := map[string][]string{} + for name, index := range table.Indexes { + declared[name] = index.Cols + } + require.Contains(t, declared, "c_u") + require.Contains(t, declared, "c_u_d") + + existing := func() (map[string][]string, map[string]*schemas.Index) { + indexes, err := x.Dialect().GetIndexes(x.DB(), t.Context(), table.Name) + require.NoError(t, err) + cols := map[string][]string{} + for name, index := range indexes { + cols[name] = index.Cols + } + return cols, indexes + } + + for range 10 { + require.NoError(t, db.SyncAllTables()) + cols, _ := existing() + require.Equal(t, declared, cols) + } + + for _, name := range []string{"c_u", "c_u_d"} { + _, indexes := existing() + _, err = x.Exec(x.Dialect().DropIndexSQL(table.Name, indexes[name])) + require.NoError(t, err) + require.NoError(t, db.SyncAllTables()) + cols, _ := existing() + assert.Equal(t, declared, cols, "index %s should be restored", name) + } +} + func TestDeleteOrphanedObjects(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase())