From f1e15e3cce56b3f4cc7e95fa962914bb6fd74387 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 1 Aug 2020 11:03:40 +0100 Subject: [PATCH 01/21] Add command to recreate tables Provides new command: `gitea doctor recreate-table` which will recreate db tables and copy the old data in to the new table. This function can be used to remove the old warning of struct defaults being out of date. Fix #8868 Fix #3265 Fix #8894 Signed-off-by: Andrew Thornton --- cmd/doctor.go | 46 ++++++ docs/content/doc/usage/command-line.en-us.md | 30 ++++ integrations/migration-test/migration_test.go | 6 + models/migrations/migrations.go | 149 +++++++++++++++++- models/migrations/v102.go | 5 +- models/models.go | 34 ++++ models/repo_language_stats.go | 2 +- 7 files changed, 269 insertions(+), 3 deletions(-) diff --git a/cmd/doctor.go b/cmd/doctor.go index 48fd3158a49f4..dccfda00a7705 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -61,6 +61,21 @@ var CmdDoctor = cli.Command{ Usage: `Name of the log file (default: "doctor.log"). Set to "-" to output to stdout, set to "" to disable`, }, }, + Subcommands: []cli.Command{ + cmdRecreateTable, + }, +} + +var cmdRecreateTable = cli.Command{ + Name: "recreate-table", + Usage: "Recreate tables from XORM definitions and copy the data.", + ArgsUsage: "[TABLE]... : (TABLEs to recreate - leave blank for all)", + Description: `The database definitions Gitea uses change across versions, sometimes changing default values and leaving old unused columns. + +This command will cause Xorm to recreate tables, copying over the data and deleting the old table. + +You should back-up your database before doing this and ensure that your database is up-to-date first.`, + Action: runRecreateTable, } type check struct { @@ -129,6 +144,37 @@ var checklist = []check{ // more checks please append here } +func runRecreateTable(ctx *cli.Context) error { + // Redirect the default golog to here + golog.SetFlags(0) + golog.SetPrefix("") + golog.SetOutput(log.NewLoggerAsWriter("INFO", log.GetLogger(log.DEFAULT))) + + setting.EnableXORMLog = false + if err := initDBDisableConsole(true); err != nil { + fmt.Println(err) + fmt.Println("Check if you are using the right config file. You can use a --config directive to specify one.") + return nil + } + + if err := models.NewEngine(context.Background(), migrations.EnsureUpToDate); err != nil { + return err + } + + args := ctx.Args() + names := make([]string, 0, ctx.NArg()) + for i := 0; i < ctx.NArg(); i++ { + names = append(names, args.Get(i)) + } + + beans, err := models.NamesToBean(names...) + if err != nil { + return err + } + + return models.NewEngine(context.Background(), migrations.RecreateTables(beans...)) +} + func runDoctor(ctx *cli.Context) error { // Silence the default loggers diff --git a/docs/content/doc/usage/command-line.en-us.md b/docs/content/doc/usage/command-line.en-us.md index 3715be7cbdcfa..deb54cd95b0f7 100644 --- a/docs/content/doc/usage/command-line.en-us.md +++ b/docs/content/doc/usage/command-line.en-us.md @@ -319,6 +319,36 @@ var checklist = []check{ This function will receive a command line context and return a list of details about the problems or error. +##### doctor recreate-table + +Sometimes when there are migrations the old columns and default values may be left +unchanged in the database schema. This may lead to warning such as: + +``` +2020/08/02 11:32:29 ...rm/session_schema.go:360:Sync2() [W] Table user Column keep_activity_private db default is , struct default is 0 +``` + +You can cause Gitea to recreate these tables and copy the old data into the new table +with the defaults set appropriately by using: + +``` +gitea doctor recreate-table user +``` + +You can ask gitea to recreate multiple tables using: + +``` +gitea doctor recreate-table table1 table2 ... +``` + +And if you would like Gitea to recreate all tables simply call: + +``` +gitea doctor recreate-table +``` + +It is highly recommended to back-up your database before running these commands. + #### manager Manage running server operations: diff --git a/integrations/migration-test/migration_test.go b/integrations/migration-test/migration_test.go index 4ee045db38348..f951d54c6f9bb 100644 --- a/integrations/migration-test/migration_test.go +++ b/integrations/migration-test/migration_test.go @@ -256,6 +256,12 @@ func doMigrationTest(t *testing.T, version string) { err = models.NewEngine(context.Background(), wrappedMigrate) assert.NoError(t, err) + + beans, _ := models.NamesToBean() + + err = models.NewEngine(context.Background(), migrations.RecreateTables(beans...)) + assert.NoError(t, err) + currentEngine.Close() } diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 7e1cf2f50a55f..08952ae097e57 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -7,6 +7,7 @@ package migrations import ( "fmt" + "reflect" "regexp" "strings" @@ -317,6 +318,153 @@ Please try upgrading to a lower version first (suggested v1.6.4), then upgrade t return nil } +// RecreateTables will recreate the tables for the provided beans using the newly provided bean definition and move all data to that new table +// WARNING: YOU MUST PROVIDE THE FULL BEAN DEFINITION +func RecreateTables(beans ...interface{}) func(*xorm.Engine) error { + return func(x *xorm.Engine) error { + sess := x.NewSession() + defer sess.Close() + if err := sess.Begin(); err != nil { + return err + } + for _, bean := range beans { + log.Info("Recreating Table: %s for Bean: %s", x.TableName(bean), reflect.Indirect(reflect.ValueOf(bean)).Type().Name()) + if err := recreateTable(sess, bean); err != nil { + return err + } + } + return sess.Commit() + } +} + +// recreateTable will recreate the table using the newly provided bean definition and move all data to that new table +// WARNING: YOU MUST PROVIDE THE FULL BEAN DEFINITION +// WARNING: YOU MUST COMMIT THE SESSION AT THE END +func recreateTable(sess *xorm.Session, bean interface{}) error { + // TODO: This will not work if there are foreign keys + + tableName := sess.Engine().TableName(bean) + tempTableName := fmt.Sprintf("temporary__%s__temporary", tableName) + + // We need to move the old table away and create a new one with the correct columns + // We will need to do this in stages to prevent data loss + // + // First let's update the old table to ensure it has all the necessary columns + if err := sess.CreateTable(bean); err != nil { + return err + } + + // Now we create the temporary table + if err := sess.Table(tempTableName).CreateTable(bean); err != nil { + return err + } + + // Work out the column names from the bean - these are the columns to select from the old table and install into the new table + table, err := sess.Engine().TableInfo(bean) + if err != nil { + return err + } + newTableColumns := table.Columns() + if len(newTableColumns) == 0 { + return fmt.Errorf("no columns in new table") + } + + if setting.Database.UseMSSQL { + sess.Exec(fmt.Sprintf("SET IDENTITY_INSERT %s ON", tempTableName)) + } + + sqlStringBuilder := &strings.Builder{} + _, _ = sqlStringBuilder.WriteString("INSERT INTO ") + _, _ = sqlStringBuilder.WriteString(tempTableName) + if setting.Database.UseMSSQL { + _, _ = sqlStringBuilder.WriteString(" (`") + _, _ = sqlStringBuilder.WriteString(newTableColumns[0].Name) + _, _ = sqlStringBuilder.WriteString("`") + for _, column := range newTableColumns[1:] { + _, _ = sqlStringBuilder.WriteString(", `") + _, _ = sqlStringBuilder.WriteString(column.Name) + _, _ = sqlStringBuilder.WriteString("`") + } + _, _ = sqlStringBuilder.WriteString(")") + } + _, _ = sqlStringBuilder.WriteString(" SELECT ") + if newTableColumns[0].Default != "" { + _, _ = sqlStringBuilder.WriteString("COALESCE(`") + _, _ = sqlStringBuilder.WriteString(newTableColumns[0].Name) + _, _ = sqlStringBuilder.WriteString("`, ") + _, _ = sqlStringBuilder.WriteString(newTableColumns[0].Default) + _, _ = sqlStringBuilder.WriteString(")") + } else { + _, _ = sqlStringBuilder.WriteString("`") + _, _ = sqlStringBuilder.WriteString(newTableColumns[0].Name) + _, _ = sqlStringBuilder.WriteString("`") + } + + for _, column := range newTableColumns[1:] { + if column.Default != "" { + _, _ = sqlStringBuilder.WriteString(", COALESCE(`") + _, _ = sqlStringBuilder.WriteString(column.Name) + _, _ = sqlStringBuilder.WriteString("`, ") + _, _ = sqlStringBuilder.WriteString(column.Default) + _, _ = sqlStringBuilder.WriteString(")") + } else { + _, _ = sqlStringBuilder.WriteString(", `") + _, _ = sqlStringBuilder.WriteString(column.Name) + _, _ = sqlStringBuilder.WriteString("`") + } + } + _, _ = sqlStringBuilder.WriteString(" FROM ") + _, _ = sqlStringBuilder.WriteString(tableName) + + if _, err := sess.Exec(sqlStringBuilder.String()); err != nil { + return err + } + + if setting.Database.UseMSSQL { + sess.Exec(fmt.Sprintf("SET IDENTITY_INSERT %s OFF", tempTableName)) + } + + switch { + case setting.Database.UseSQLite3: + fallthrough + case setting.Database.UseMySQL: + // SQLite and MySQL will drop all the constraints on the old table + if _, err := sess.Exec(fmt.Sprintf("DROP TABLE `%s`", tableName)); err != nil { + return err + } + + // SQLite and MySQL will move all the constraints from the temporary table to the new table + if _, err := sess.Exec(fmt.Sprintf("ALTER TABLE `%s` RENAME TO `%s`", tempTableName, tableName)); err != nil { + return err + } + case setting.Database.UsePostgreSQL: + // CASCADE causes postgres to drop all the constraints on the old table + if _, err := sess.Exec(fmt.Sprintf("DROP TABLE `%s` CASCADE", tableName)); err != nil { + return err + } + + // CASCADE causes postgres to move all the constraints from the temporary table to the new table + if _, err := sess.Exec(fmt.Sprintf("ALTER TABLE `%s` RENAME TO `%s`", tempTableName, tableName)); err != nil { + return err + } + case setting.Database.UseMSSQL: + // MSSQL will drop all the constraints on the old table + if _, err := sess.Exec(fmt.Sprintf("DROP TABLE `%s`", tableName)); err != nil { + return err + } + + // MSSQL sp_rename will move all the constraints from the temporary table to the new table + if _, err := sess.Exec(fmt.Sprintf("sp_rename '[%s]','[%s]'", tempTableName, tableName)); err != nil { + return err + } + + default: + log.Fatal("Unrecognized DB") + } + return nil +} + +// WARNING: YOU MUST COMMIT THE SESSION AT THE END func dropTableColumns(sess *xorm.Session, tableName string, columnNames ...string) (err error) { if tableName == "" || len(columnNames) == 0 { return nil @@ -465,7 +613,6 @@ func dropTableColumns(sess *xorm.Session, tableName string, columnNames ...strin return fmt.Errorf("Drop table `%s` columns %v: %v", tableName, columnNames, err) } - return sess.Commit() default: log.Fatal("Unrecognized DB") } diff --git a/models/migrations/v102.go b/models/migrations/v102.go index 74e8574ec320a..f04a32fe68e8a 100644 --- a/models/migrations/v102.go +++ b/models/migrations/v102.go @@ -11,5 +11,8 @@ import ( func dropColumnHeadUserNameOnPullRequest(x *xorm.Engine) error { sess := x.NewSession() defer sess.Close() - return dropTableColumns(sess, "pull_request", "head_user_name") + if err := dropTableColumns(sess, "pull_request", "head_user_name"); err != nil { + return err + } + return sess.Commit() } diff --git a/models/models.go b/models/models.go index d0703be300d4f..f0146f6d66ca2 100644 --- a/models/models.go +++ b/models/models.go @@ -10,6 +10,8 @@ import ( "database/sql" "errors" "fmt" + "reflect" + "strings" "code.gitea.io/gitea/modules/setting" @@ -210,6 +212,38 @@ func NewEngine(ctx context.Context, migrateFunc func(*xorm.Engine) error) (err e return nil } +// NamesToBean return a list of beans or an error +func NamesToBean(names ...string) ([]interface{}, error) { + beans := []interface{}{} + if len(names) == 0 { + for _, bean := range tables { + beans = append(beans, bean) + } + return beans, nil + } + // Need to map provided names to beans... + beanMap := make(map[string]interface{}) + for _, bean := range tables { + + beanMap[strings.ToLower(reflect.Indirect(reflect.ValueOf(bean)).Type().Name())] = bean + beanMap[strings.ToLower(x.TableName(bean))] = bean + beanMap[strings.ToLower(x.TableName(bean, true))] = bean + } + + gotBean := make(map[interface{}]bool) + for _, name := range names { + bean, ok := beanMap[strings.ToLower(strings.TrimSpace(name))] + if !ok { + return nil, fmt.Errorf("No table found that matches: %s", name) + } + if !gotBean[bean] { + beans = append(beans, bean) + gotBean[bean] = true + } + } + return beans, nil +} + // Statistic contains the database statistics type Statistic struct { Counter struct { diff --git a/models/repo_language_stats.go b/models/repo_language_stats.go index a15063e25a6ed..e60e441c7ab67 100644 --- a/models/repo_language_stats.go +++ b/models/repo_language_stats.go @@ -19,7 +19,7 @@ type LanguageStat struct { RepoID int64 `xorm:"UNIQUE(s) INDEX NOT NULL"` CommitID string IsPrimary bool - Language string `xorm:"VARCHAR(30) UNIQUE(s) INDEX NOT NULL"` + Language string `xorm:"VARCHAR(50) UNIQUE(s) INDEX NOT NULL"` Percentage float32 `xorm:"-"` Size int64 `xorm:"NOT NULL DEFAULT 0"` Color string `xorm:"-"` From 05d551ca0a1dd07e5431a909aad9301b69b46286 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 2 Aug 2020 11:55:32 +0100 Subject: [PATCH 02/21] placate lint Signed-off-by: Andrew Thornton --- models/models.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/models/models.go b/models/models.go index f0146f6d66ca2..80bdcdaa066f5 100644 --- a/models/models.go +++ b/models/models.go @@ -216,9 +216,7 @@ func NewEngine(ctx context.Context, migrateFunc func(*xorm.Engine) error) (err e func NamesToBean(names ...string) ([]interface{}, error) { beans := []interface{}{} if len(names) == 0 { - for _, bean := range tables { - beans = append(beans, bean) - } + beans = append(beans, tables...) return beans, nil } // Need to map provided names to beans... From bb7f07e23633cc9f1fb3bc7cf1653d79745b987e Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 2 Aug 2020 12:39:01 +0100 Subject: [PATCH 03/21] ensure that current engine is closed Signed-off-by: Andrew Thornton --- integrations/migration-test/migration_test.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/integrations/migration-test/migration_test.go b/integrations/migration-test/migration_test.go index f951d54c6f9bb..9a9117f498749 100644 --- a/integrations/migration-test/migration_test.go +++ b/integrations/migration-test/migration_test.go @@ -256,10 +256,17 @@ func doMigrationTest(t *testing.T, version string) { err = models.NewEngine(context.Background(), wrappedMigrate) assert.NoError(t, err) + currentEngine.Close() + + err = models.SetEngine() + assert.NoError(t, err) beans, _ := models.NamesToBean() - err = models.NewEngine(context.Background(), migrations.RecreateTables(beans...)) + err = models.NewEngine(context.Background(), func(x *xorm.Engine) error { + currentEngine = x + return migrations.RecreateTables(beans...)(x) + }) assert.NoError(t, err) currentEngine.Close() From 129e4cebafaa00927951c152707add1f0841ca07 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 2 Aug 2020 12:57:45 +0100 Subject: [PATCH 04/21] Quote the table name Signed-off-by: Andrew Thornton --- models/migrations/migrations.go | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 08952ae097e57..92d9be8d2b964 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -349,12 +349,7 @@ func recreateTable(sess *xorm.Session, bean interface{}) error { // We need to move the old table away and create a new one with the correct columns // We will need to do this in stages to prevent data loss // - // First let's update the old table to ensure it has all the necessary columns - if err := sess.CreateTable(bean); err != nil { - return err - } - - // Now we create the temporary table + // First create the temporary table if err := sess.Table(tempTableName).CreateTable(bean); err != nil { return err } @@ -376,17 +371,15 @@ func recreateTable(sess *xorm.Session, bean interface{}) error { sqlStringBuilder := &strings.Builder{} _, _ = sqlStringBuilder.WriteString("INSERT INTO ") _, _ = sqlStringBuilder.WriteString(tempTableName) - if setting.Database.UseMSSQL { - _, _ = sqlStringBuilder.WriteString(" (`") - _, _ = sqlStringBuilder.WriteString(newTableColumns[0].Name) + _, _ = sqlStringBuilder.WriteString(" (`") + _, _ = sqlStringBuilder.WriteString(newTableColumns[0].Name) + _, _ = sqlStringBuilder.WriteString("`") + for _, column := range newTableColumns[1:] { + _, _ = sqlStringBuilder.WriteString(", `") + _, _ = sqlStringBuilder.WriteString(column.Name) _, _ = sqlStringBuilder.WriteString("`") - for _, column := range newTableColumns[1:] { - _, _ = sqlStringBuilder.WriteString(", `") - _, _ = sqlStringBuilder.WriteString(column.Name) - _, _ = sqlStringBuilder.WriteString("`") - } - _, _ = sqlStringBuilder.WriteString(")") } + _, _ = sqlStringBuilder.WriteString(")") _, _ = sqlStringBuilder.WriteString(" SELECT ") if newTableColumns[0].Default != "" { _, _ = sqlStringBuilder.WriteString("COALESCE(`") @@ -413,8 +406,9 @@ func recreateTable(sess *xorm.Session, bean interface{}) error { _, _ = sqlStringBuilder.WriteString("`") } } - _, _ = sqlStringBuilder.WriteString(" FROM ") + _, _ = sqlStringBuilder.WriteString(" FROM `") _, _ = sqlStringBuilder.WriteString(tableName) + _, _ = sqlStringBuilder.WriteString("`") if _, err := sess.Exec(sqlStringBuilder.String()); err != nil { return err From 3cfbd35cd55f2a1b2d210933709f216c19e6a097 Mon Sep 17 00:00:00 2001 From: zeripath Date: Sun, 2 Aug 2020 17:29:05 +0100 Subject: [PATCH 05/21] Apply suggestions from code review --- models/migrations/migrations.go | 2 +- models/repo_language_stats.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 92d9be8d2b964..90d315f1e70f6 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -344,7 +344,7 @@ func recreateTable(sess *xorm.Session, bean interface{}) error { // TODO: This will not work if there are foreign keys tableName := sess.Engine().TableName(bean) - tempTableName := fmt.Sprintf("temporary__%s__temporary", tableName) + tempTableName := fmt.Sprintf("temp__%s", tableName) // We need to move the old table away and create a new one with the correct columns // We will need to do this in stages to prevent data loss diff --git a/models/repo_language_stats.go b/models/repo_language_stats.go index e60e441c7ab67..a15063e25a6ed 100644 --- a/models/repo_language_stats.go +++ b/models/repo_language_stats.go @@ -19,7 +19,7 @@ type LanguageStat struct { RepoID int64 `xorm:"UNIQUE(s) INDEX NOT NULL"` CommitID string IsPrimary bool - Language string `xorm:"VARCHAR(50) UNIQUE(s) INDEX NOT NULL"` + Language string `xorm:"VARCHAR(30) UNIQUE(s) INDEX NOT NULL"` Percentage float32 `xorm:"-"` Size int64 `xorm:"NOT NULL DEFAULT 0"` Color string `xorm:"-"` From 60479f83b9973592c29444f170d83e6ba886cf5f Mon Sep 17 00:00:00 2001 From: zeripath Date: Sun, 2 Aug 2020 18:57:25 +0100 Subject: [PATCH 06/21] Let's see if mssql will tolerate this... --- models/migrations/migrations.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 90d315f1e70f6..4a475a15577cd 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -344,7 +344,7 @@ func recreateTable(sess *xorm.Session, bean interface{}) error { // TODO: This will not work if there are foreign keys tableName := sess.Engine().TableName(bean) - tempTableName := fmt.Sprintf("temp__%s", tableName) + tempTableName := fmt.Sprintf("t__%s", tableName) // We need to move the old table away and create a new one with the correct columns // We will need to do this in stages to prevent data loss From 007afa942f6ae0bcb0ca8cfe94d72dfb4dfa607d Mon Sep 17 00:00:00 2001 From: zeripath Date: Sun, 2 Aug 2020 19:00:56 +0100 Subject: [PATCH 07/21] turns out the issue is the double underscore --- models/migrations/migrations.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 4a475a15577cd..c115547989a79 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -344,7 +344,7 @@ func recreateTable(sess *xorm.Session, bean interface{}) error { // TODO: This will not work if there are foreign keys tableName := sess.Engine().TableName(bean) - tempTableName := fmt.Sprintf("t__%s", tableName) + tempTableName := fmt.Sprintf("tempzzz_%s_zzztemp", tableName) // We need to move the old table away and create a new one with the correct columns // We will need to do this in stages to prevent data loss From 82cf751ac674e34a1274cb5fac3386ec5b245755 Mon Sep 17 00:00:00 2001 From: zeripath Date: Mon, 3 Aug 2020 15:50:57 +0100 Subject: [PATCH 08/21] Update models/migrations/migrations.go --- models/migrations/migrations.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index c115547989a79..1977989aa1352 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -344,7 +344,7 @@ func recreateTable(sess *xorm.Session, bean interface{}) error { // TODO: This will not work if there are foreign keys tableName := sess.Engine().TableName(bean) - tempTableName := fmt.Sprintf("tempzzz_%s_zzztemp", tableName) + tempTableName := fmt.Sprintf("tempzzz_%s", tableName) // We need to move the old table away and create a new one with the correct columns // We will need to do this in stages to prevent data loss From 3e79034471cd865a117a1498be10d9e904d6dd7e Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 3 Aug 2020 18:09:03 +0100 Subject: [PATCH 09/21] let us go overboard with quotes Signed-off-by: Andrew Thornton --- models/migrations/migrations.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 1977989aa1352..7ac5fbeb3d3fe 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -365,13 +365,13 @@ func recreateTable(sess *xorm.Session, bean interface{}) error { } if setting.Database.UseMSSQL { - sess.Exec(fmt.Sprintf("SET IDENTITY_INSERT %s ON", tempTableName)) + sess.Exec(fmt.Sprintf("SET IDENTITY_INSERT `%s` ON", tempTableName)) } sqlStringBuilder := &strings.Builder{} - _, _ = sqlStringBuilder.WriteString("INSERT INTO ") + _, _ = sqlStringBuilder.WriteString("INSERT INTO `") _, _ = sqlStringBuilder.WriteString(tempTableName) - _, _ = sqlStringBuilder.WriteString(" (`") + _, _ = sqlStringBuilder.WriteString("` (`") _, _ = sqlStringBuilder.WriteString(newTableColumns[0].Name) _, _ = sqlStringBuilder.WriteString("`") for _, column := range newTableColumns[1:] { @@ -415,7 +415,7 @@ func recreateTable(sess *xorm.Session, bean interface{}) error { } if setting.Database.UseMSSQL { - sess.Exec(fmt.Sprintf("SET IDENTITY_INSERT %s OFF", tempTableName)) + sess.Exec(fmt.Sprintf("SET IDENTITY_INSERT `%s` OFF", tempTableName)) } switch { @@ -448,7 +448,7 @@ func recreateTable(sess *xorm.Session, bean interface{}) error { } // MSSQL sp_rename will move all the constraints from the temporary table to the new table - if _, err := sess.Exec(fmt.Sprintf("sp_rename '[%s]','[%s]'", tempTableName, tableName)); err != nil { + if _, err := sess.Exec(fmt.Sprintf("sp_rename `%s`,`%s`", tempTableName, tableName)); err != nil { return err } From 0b041ca7b84948878c08eea32ef2945efcaa97d1 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 3 Aug 2020 19:24:22 +0100 Subject: [PATCH 10/21] Seriously MSSQL? Signed-off-by: Andrew Thornton --- models/migrations/migrations.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 7ac5fbeb3d3fe..d973b8227548a 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -344,7 +344,7 @@ func recreateTable(sess *xorm.Session, bean interface{}) error { // TODO: This will not work if there are foreign keys tableName := sess.Engine().TableName(bean) - tempTableName := fmt.Sprintf("tempzzz_%s", tableName) + tempTableName := fmt.Sprintf("tempzzz%szztemp", tableName) // We need to move the old table away and create a new one with the correct columns // We will need to do this in stages to prevent data loss From d708602671e41f3803742df812196618b2b15338 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 3 Aug 2020 21:03:30 +0100 Subject: [PATCH 11/21] Add some logging Signed-off-by: Andrew Thornton --- models/migrations/migrations.go | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index d973b8227548a..40a1672819e2e 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -351,12 +351,15 @@ func recreateTable(sess *xorm.Session, bean interface{}) error { // // First create the temporary table if err := sess.Table(tempTableName).CreateTable(bean); err != nil { + log.Error("Unable to create table %s. Error: %v", tempTableName, err) return err } // Work out the column names from the bean - these are the columns to select from the old table and install into the new table table, err := sess.Engine().TableInfo(bean) if err != nil { + log.Error("Unable to get table info. Error: %v", err) + return err } newTableColumns := table.Columns() @@ -365,7 +368,10 @@ func recreateTable(sess *xorm.Session, bean interface{}) error { } if setting.Database.UseMSSQL { - sess.Exec(fmt.Sprintf("SET IDENTITY_INSERT `%s` ON", tempTableName)) + if _, err := sess.Exec(fmt.Sprintf("SET IDENTITY_INSERT `%s` ON", tempTableName)); err != nil { + log.Error("Unable to set identity insert for table %s. Error: %v", tempTableName, err) + return err + } } sqlStringBuilder := &strings.Builder{} @@ -411,11 +417,15 @@ func recreateTable(sess *xorm.Session, bean interface{}) error { _, _ = sqlStringBuilder.WriteString("`") if _, err := sess.Exec(sqlStringBuilder.String()); err != nil { + log.Error("Unable to set copy data in to temp table %s. Error: %v", tempTableName, err) return err } if setting.Database.UseMSSQL { - sess.Exec(fmt.Sprintf("SET IDENTITY_INSERT `%s` OFF", tempTableName)) + if _, err := sess.Exec(fmt.Sprintf("SET IDENTITY_INSERT `%s` OFF", tempTableName)); err != nil { + log.Error("Unable to switch off identity insert for table %s. Error: %v", tempTableName, err) + return err + } } switch { @@ -424,31 +434,37 @@ func recreateTable(sess *xorm.Session, bean interface{}) error { case setting.Database.UseMySQL: // SQLite and MySQL will drop all the constraints on the old table if _, err := sess.Exec(fmt.Sprintf("DROP TABLE `%s`", tableName)); err != nil { + log.Error("Unable to drop old table %s. Error: %v", tableName, err) return err } // SQLite and MySQL will move all the constraints from the temporary table to the new table if _, err := sess.Exec(fmt.Sprintf("ALTER TABLE `%s` RENAME TO `%s`", tempTableName, tableName)); err != nil { + log.Error("Unable to rename %s to %s. Error: %v", tempTableName, tableName, err) return err } case setting.Database.UsePostgreSQL: // CASCADE causes postgres to drop all the constraints on the old table if _, err := sess.Exec(fmt.Sprintf("DROP TABLE `%s` CASCADE", tableName)); err != nil { + log.Error("Unable to drop old table %s. Error: %v", tableName, err) return err } // CASCADE causes postgres to move all the constraints from the temporary table to the new table if _, err := sess.Exec(fmt.Sprintf("ALTER TABLE `%s` RENAME TO `%s`", tempTableName, tableName)); err != nil { + log.Error("Unable to rename %s to %s. Error: %v", tempTableName, tableName, err) return err } case setting.Database.UseMSSQL: // MSSQL will drop all the constraints on the old table if _, err := sess.Exec(fmt.Sprintf("DROP TABLE `%s`", tableName)); err != nil { + log.Error("Unable to drop old table %s. Error: %v", tableName, err) return err } // MSSQL sp_rename will move all the constraints from the temporary table to the new table if _, err := sess.Exec(fmt.Sprintf("sp_rename `%s`,`%s`", tempTableName, tableName)); err != nil { + log.Error("Unable to rename %s to %s. Error: %v", tempTableName, tableName, err) return err } From 8ab15737e821c53d1cf3d3fdba0518b2cf529aca Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 3 Aug 2020 21:16:36 +0100 Subject: [PATCH 12/21] Finally fix MSSQL Signed-off-by: Andrew Thornton --- models/migrations/migrations.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 40a1672819e2e..df9c3b8930ffe 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -344,7 +344,7 @@ func recreateTable(sess *xorm.Session, bean interface{}) error { // TODO: This will not work if there are foreign keys tableName := sess.Engine().TableName(bean) - tempTableName := fmt.Sprintf("tempzzz%szztemp", tableName) + tempTableName := fmt.Sprintf("tempzzz__%s__zztemp", tableName) // We need to move the old table away and create a new one with the correct columns // We will need to do this in stages to prevent data loss @@ -366,8 +366,12 @@ func recreateTable(sess *xorm.Session, bean interface{}) error { if len(newTableColumns) == 0 { return fmt.Errorf("no columns in new table") } + hasID := false + for _, column := range newTableColumns { + hasID = hasID || (column.IsPrimaryKey && column.IsAutoIncrement) + } - if setting.Database.UseMSSQL { + if hasID && setting.Database.UseMSSQL { if _, err := sess.Exec(fmt.Sprintf("SET IDENTITY_INSERT `%s` ON", tempTableName)); err != nil { log.Error("Unable to set identity insert for table %s. Error: %v", tempTableName, err) return err @@ -421,7 +425,7 @@ func recreateTable(sess *xorm.Session, bean interface{}) error { return err } - if setting.Database.UseMSSQL { + if hasID && setting.Database.UseMSSQL { if _, err := sess.Exec(fmt.Sprintf("SET IDENTITY_INSERT `%s` OFF", tempTableName)); err != nil { log.Error("Unable to switch off identity insert for table %s. Error: %v", tempTableName, err) return err From 6ae198f1310d5ac324db16971a630c5e9cb050ab Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 7 Aug 2020 18:13:17 +0100 Subject: [PATCH 13/21] as per @silverwind Signed-off-by: Andrew Thornton --- models/migrations/migrations.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 7f30631269ae0..4c35d97bc0bf7 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -346,7 +346,7 @@ func recreateTable(sess *xorm.Session, bean interface{}) error { // TODO: This will not work if there are foreign keys tableName := sess.Engine().TableName(bean) - tempTableName := fmt.Sprintf("tempzzz__%s__zztemp", tableName) + tempTableName := fmt.Sprintf("recreate_tempzzz__%s__zztemp_recreate", tableName) // We need to move the old table away and create a new one with the correct columns // We will need to do this in stages to prevent data loss From aaa2131c942e57f8de3eb6b4a804f19040a7affd Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Mon, 10 Aug 2020 21:12:28 +0100 Subject: [PATCH 14/21] Ensure that the indexes are recreated too Signed-off-by: Andrew Thornton --- models/migrations/migrations.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 4c35d97bc0bf7..0482929bb2644 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -357,6 +357,16 @@ func recreateTable(sess *xorm.Session, bean interface{}) error { return err } + if err := sess.Table(tempTableName).CreateUniques(bean); err != nil { + log.Error("Unable to create uniques for table %s. Error: %v", tempTableName, err) + return err + } + + if err := sess.Table(tempTableName).CreateIndexes(bean); err != nil { + log.Error("Unable to create indexes for table %s. Error: %v", tempTableName, err) + return err + } + // Work out the column names from the bean - these are the columns to select from the old table and install into the new table table, err := sess.Engine().TableInfo(bean) if err != nil { From 2a39249b9f0e9d759acb047e13e16a2708f3cec1 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 11 Aug 2020 20:21:06 +0100 Subject: [PATCH 15/21] shorter prefix Signed-off-by: Andrew Thornton --- models/migrations/migrations.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 0482929bb2644..e6e7eb42569a7 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -346,7 +346,7 @@ func recreateTable(sess *xorm.Session, bean interface{}) error { // TODO: This will not work if there are foreign keys tableName := sess.Engine().TableName(bean) - tempTableName := fmt.Sprintf("recreate_tempzzz__%s__zztemp_recreate", tableName) + tempTableName := fmt.Sprintf("tmp_recreate__%s", tableName) // We need to move the old table away and create a new one with the correct columns // We will need to do this in stages to prevent data loss From 69c8ff9767b1f274bb35266f17593273d1586b46 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 11 Aug 2020 20:50:31 +0100 Subject: [PATCH 16/21] use the innodb storeengine Signed-off-by: Andrew Thornton --- cmd/doctor.go | 18 +++++++++++++----- models/migrations/migrations.go | 1 + 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/cmd/doctor.go b/cmd/doctor.go index dccfda00a7705..78a9c2355103c 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -25,6 +25,7 @@ import ( "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/setting" "xorm.io/builder" + "xorm.io/xorm" "github.com/urfave/cli" ) @@ -157,10 +158,6 @@ func runRecreateTable(ctx *cli.Context) error { return nil } - if err := models.NewEngine(context.Background(), migrations.EnsureUpToDate); err != nil { - return err - } - args := ctx.Args() names := make([]string, 0, ctx.NArg()) for i := 0; i < ctx.NArg(); i++ { @@ -171,8 +168,19 @@ func runRecreateTable(ctx *cli.Context) error { if err != nil { return err } + recreateTables := migrations.RecreateTables(beans...) + + if err := models.NewEngine(context.Background(), migrations.EnsureUpToDate); err != nil { + return err + } + + return models.NewEngine(context.Background(), func(x *xorm.Engine) error { + if err := migrations.EnsureUpToDate(x); err != nil { + return err + } + return recreateTables(x) + }) - return models.NewEngine(context.Background(), migrations.RecreateTables(beans...)) } func runDoctor(ctx *cli.Context) error { diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index e6e7eb42569a7..4a090fd640c3d 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -329,6 +329,7 @@ func RecreateTables(beans ...interface{}) func(*xorm.Engine) error { if err := sess.Begin(); err != nil { return err } + sess = sess.StoreEngine("InnoDB") for _, bean := range beans { log.Info("Recreating Table: %s for Bean: %s", x.TableName(bean), reflect.Indirect(reflect.ValueOf(bean)).Type().Name()) if err := recreateTable(sess, bean); err != nil { From fb015cad5562903b1dcbba02ed05f8e0f3e6749e Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 29 Aug 2020 19:25:27 +0100 Subject: [PATCH 17/21] remove extra ensureuptodate Signed-off-by: Andrew Thornton --- cmd/doctor.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/cmd/doctor.go b/cmd/doctor.go index dcba23640e097..2c60f4231f777 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -177,10 +177,6 @@ func runRecreateTable(ctx *cli.Context) error { } recreateTables := migrations.RecreateTables(beans...) - if err := models.NewEngine(context.Background(), migrations.EnsureUpToDate); err != nil { - return err - } - return models.NewEngine(context.Background(), func(x *xorm.Engine) error { if err := migrations.EnsureUpToDate(x); err != nil { return err From 25d886c3c724c4922f8864bc54f7f97c377ac951 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 29 Aug 2020 20:51:36 +0100 Subject: [PATCH 18/21] Add debug flag Signed-off-by: Andrew Thornton --- cmd/doctor.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/cmd/doctor.go b/cmd/doctor.go index 2c60f4231f777..2ca2bb5e70b6a 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -72,6 +72,12 @@ var cmdRecreateTable = cli.Command{ Name: "recreate-table", Usage: "Recreate tables from XORM definitions and copy the data.", ArgsUsage: "[TABLE]... : (TABLEs to recreate - leave blank for all)", + Flags: []cli.Flag{ + cli.BoolFlag{ + Name: "debug", + Usage: "Print SQL commands sent", + }, + }, Description: `The database definitions Gitea uses change across versions, sometimes changing default values and leaving old unused columns. This command will cause Xorm to recreate tables, copying over the data and deleting the old table. @@ -158,8 +164,15 @@ func runRecreateTable(ctx *cli.Context) error { golog.SetPrefix("") golog.SetOutput(log.NewLoggerAsWriter("INFO", log.GetLogger(log.DEFAULT))) - setting.EnableXORMLog = false - if err := initDBDisableConsole(true); err != nil { + setting.NewContext() + setting.InitDBConfig() + + setting.EnableXORMLog = ctx.Bool("debug") + setting.Database.LogSQL = ctx.Bool("debug") + setting.Cfg.Section("log").Key("XORM").SetValue(",") + + setting.NewXORMLogService(!ctx.Bool("debug")) + if err := models.SetEngine(); err != nil { fmt.Println(err) fmt.Println("Check if you are using the right config file. You can use a --config directive to specify one.") return nil From 9e6f5d212dab433564a4814eec5409645d771cb1 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Wed, 2 Sep 2020 15:47:19 +0100 Subject: [PATCH 19/21] drop and recreate indices in sqlite and postgres Signed-off-by: Andrew Thornton --- integrations/migration-test/migration_test.go | 7 +++ models/migrations/migrations.go | 44 ++++++++++++++++++- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/integrations/migration-test/migration_test.go b/integrations/migration-test/migration_test.go index 6781298bdaece..c8b07f6e4c983 100644 --- a/integrations/migration-test/migration_test.go +++ b/integrations/migration-test/migration_test.go @@ -270,6 +270,13 @@ func doMigrationTest(t *testing.T, version string) { }) assert.NoError(t, err) + // We do this a second time to ensure that there is not a problem with retained indices + err = models.NewEngine(context.Background(), func(x *xorm.Engine) error { + currentEngine = x + return migrations.RecreateTables(beans...)(x) + }) + assert.NoError(t, err) + currentEngine.Close() } diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 62da22dafcedf..aef48658310a9 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -451,9 +451,34 @@ func recreateTable(sess *xorm.Session, bean interface{}) error { switch { case setting.Database.UseSQLite3: - fallthrough + // SQLite will drop all the constraints on the old table + if _, err := sess.Exec(fmt.Sprintf("DROP TABLE `%s`", tableName)); err != nil { + log.Error("Unable to drop old table %s. Error: %v", tableName, err) + return err + } + + if err := sess.Table(tempTableName).DropIndexes(bean); err != nil { + log.Error("Unable to drop indexes on temporary table %s. Error: %v", tempTableName, err) + return err + } + + if _, err := sess.Exec(fmt.Sprintf("ALTER TABLE `%s` RENAME TO `%s`", tempTableName, tableName)); err != nil { + log.Error("Unable to rename %s to %s. Error: %v", tempTableName, tableName, err) + return err + } + + if err := sess.Table(tableName).CreateIndexes(bean); err != nil { + log.Error("Unable to recreate indexes on table %s. Error: %v", tableName, err) + return err + } + + if err := sess.Table(tableName).CreateUniques(bean); err != nil { + log.Error("Unable to recreate uniques on table %s. Error: %v", tableName, err) + return err + } + case setting.Database.UseMySQL: - // SQLite and MySQL will drop all the constraints on the old table + // MySQL will drop all the constraints on the old table if _, err := sess.Exec(fmt.Sprintf("DROP TABLE `%s`", tableName)); err != nil { log.Error("Unable to drop old table %s. Error: %v", tableName, err) return err @@ -471,11 +496,26 @@ func recreateTable(sess *xorm.Session, bean interface{}) error { return err } + if err := sess.Table(tempTableName).DropIndexes(bean); err != nil { + log.Error("Unable to drop indexes on temporary table %s. Error: %v", tempTableName, err) + return err + } + // CASCADE causes postgres to move all the constraints from the temporary table to the new table if _, err := sess.Exec(fmt.Sprintf("ALTER TABLE `%s` RENAME TO `%s`", tempTableName, tableName)); err != nil { log.Error("Unable to rename %s to %s. Error: %v", tempTableName, tableName, err) return err } + + if err := sess.Table(tableName).CreateIndexes(bean); err != nil { + log.Error("Unable to recreate indexes on table %s. Error: %v", tableName, err) + return err + } + + if err := sess.Table(tableName).CreateUniques(bean); err != nil { + log.Error("Unable to recreate uniques on table %s. Error: %v", tableName, err) + return err + } case setting.Database.UseMSSQL: // MSSQL will drop all the constraints on the old table if _, err := sess.Exec(fmt.Sprintf("DROP TABLE `%s`", tableName)); err != nil { From 6562ddcf33d2c076de6b49f9822c148dffab5f2f Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Wed, 2 Sep 2020 19:17:28 +0100 Subject: [PATCH 20/21] fix postgres index relabelling Signed-off-by: Andrew Thornton --- models/migrations/migrations.go | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index aef48658310a9..d5278c96f47cd 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -496,26 +496,29 @@ func recreateTable(sess *xorm.Session, bean interface{}) error { return err } - if err := sess.Table(tempTableName).DropIndexes(bean); err != nil { - log.Error("Unable to drop indexes on temporary table %s. Error: %v", tempTableName, err) - return err - } - // CASCADE causes postgres to move all the constraints from the temporary table to the new table if _, err := sess.Exec(fmt.Sprintf("ALTER TABLE `%s` RENAME TO `%s`", tempTableName, tableName)); err != nil { log.Error("Unable to rename %s to %s. Error: %v", tempTableName, tableName, err) return err } - if err := sess.Table(tableName).CreateIndexes(bean); err != nil { - log.Error("Unable to recreate indexes on table %s. Error: %v", tableName, err) + var indices []string + schema := sess.Engine().Dialect().URI().Schema + sess.Engine().SetSchema("") + if err := sess.Table("pg_indexes").Cols("indexname").Where("tablename = ? ", tableName).Find(&indices); err != nil { + log.Error("Unable to rename %s to %s. Error: %v", tempTableName, tableName, err) return err } + sess.Engine().SetSchema(schema) - if err := sess.Table(tableName).CreateUniques(bean); err != nil { - log.Error("Unable to recreate uniques on table %s. Error: %v", tableName, err) - return err + for _, index := range indices { + newIndexName := strings.Replace(index, "tmp_recreate__", "", 1) + if _, err := sess.Exec(fmt.Sprintf("ALTER INDEX `%s` RENAME TO `%s`", index, newIndexName)); err != nil { + log.Error("Unable to rename %s to %s. Error: %v", index, newIndexName, err) + return err + } } + case setting.Database.UseMSSQL: // MSSQL will drop all the constraints on the old table if _, err := sess.Exec(fmt.Sprintf("DROP TABLE `%s`", tableName)); err != nil { From 0eade14143c59ba721f1427e6bc21a50ac4ced7a Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 6 Sep 2020 13:23:36 +0100 Subject: [PATCH 21/21] close db between recreating tables Signed-off-by: Andrew Thornton --- integrations/migration-test/migration_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/integrations/migration-test/migration_test.go b/integrations/migration-test/migration_test.go index c8b07f6e4c983..940e4738ad389 100644 --- a/integrations/migration-test/migration_test.go +++ b/integrations/migration-test/migration_test.go @@ -269,8 +269,12 @@ func doMigrationTest(t *testing.T, version string) { return migrations.RecreateTables(beans...)(x) }) assert.NoError(t, err) + currentEngine.Close() // We do this a second time to ensure that there is not a problem with retained indices + err = models.SetEngine() + assert.NoError(t, err) + err = models.NewEngine(context.Background(), func(x *xorm.Engine) error { currentEngine = x return migrations.RecreateTables(beans...)(x)