From 09b3014683050992571573ed9e09c92b69b7db4f Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 18 Dec 2018 21:47:45 +0100 Subject: [PATCH 1/3] fix only create/drop database indices if not exists/exists --- pkg/services/sqlstore/migrator/conditions.go | 31 +++++++++++++++++-- pkg/services/sqlstore/migrator/dialect.go | 3 +- pkg/services/sqlstore/migrator/migrations.go | 18 ++++------- pkg/services/sqlstore/migrator/migrator.go | 21 ++++++++----- .../sqlstore/migrator/mysql_dialect.go | 12 +++---- .../sqlstore/migrator/postgres_dialect.go | 6 ++-- .../sqlstore/migrator/sqlite_dialect.go | 7 +++-- 7 files changed, 63 insertions(+), 35 deletions(-) diff --git a/pkg/services/sqlstore/migrator/conditions.go b/pkg/services/sqlstore/migrator/conditions.go index 79bf7c7ed27..8817b43f01c 100644 --- a/pkg/services/sqlstore/migrator/conditions.go +++ b/pkg/services/sqlstore/migrator/conditions.go @@ -2,12 +2,37 @@ package migrator type MigrationCondition interface { Sql(dialect Dialect) (string, []interface{}) + IsFulfilled(results []map[string][]byte) bool } -type IfTableExistsCondition struct { +type ExistsMigrationCondition struct{} + +func (c *ExistsMigrationCondition) IsFulfilled(results []map[string][]byte) bool { + return len(results) == 1 +} + +type NotExistsMigrationCondition struct{} + +func (c *NotExistsMigrationCondition) IsFulfilled(results []map[string][]byte) bool { + return len(results) == 0 +} + +type IfIndexExistsCondition struct { + ExistsMigrationCondition TableName string + IndexName string } -func (c *IfTableExistsCondition) Sql(dialect Dialect) (string, []interface{}) { - return dialect.TableCheckSql(c.TableName) +func (c *IfIndexExistsCondition) Sql(dialect Dialect) (string, []interface{}) { + return dialect.IndexCheckSql(c.TableName, c.IndexName) +} + +type IfIndexNotExistsCondition struct { + NotExistsMigrationCondition + TableName string + IndexName string +} + +func (c *IfIndexNotExistsCondition) Sql(dialect Dialect) (string, []interface{}) { + return dialect.IndexCheckSql(c.TableName, c.IndexName) } diff --git a/pkg/services/sqlstore/migrator/dialect.go b/pkg/services/sqlstore/migrator/dialect.go index 506a01c3ed8..e8ea07e007c 100644 --- a/pkg/services/sqlstore/migrator/dialect.go +++ b/pkg/services/sqlstore/migrator/dialect.go @@ -29,10 +29,11 @@ type Dialect interface { DropTable(tableName string) string DropIndexSql(tableName string, index *Index) string - TableCheckSql(tableName string) (string, []interface{}) RenameTable(oldName string, newName string) string UpdateTableSql(tableName string, columns []*Column) string + IndexCheckSql(tableName, indexName string) (string, []interface{}) + ColString(*Column) string ColStringNoPk(*Column) string diff --git a/pkg/services/sqlstore/migrator/migrations.go b/pkg/services/sqlstore/migrator/migrations.go index fd71cc3d290..d5a4158c52a 100644 --- a/pkg/services/sqlstore/migrator/migrations.go +++ b/pkg/services/sqlstore/migrator/migrations.go @@ -109,7 +109,9 @@ type AddIndexMigration struct { } func NewAddIndexMigration(table Table, index *Index) *AddIndexMigration { - return &AddIndexMigration{tableName: table.Name, index: index} + m := &AddIndexMigration{tableName: table.Name, index: index} + m.Condition = &IfIndexNotExistsCondition{TableName: table.Name, IndexName: index.XName(table.Name)} + return m } func (m *AddIndexMigration) Table(tableName string) *AddIndexMigration { @@ -128,7 +130,9 @@ type DropIndexMigration struct { } func NewDropIndexMigration(table Table, index *Index) *DropIndexMigration { - return &DropIndexMigration{tableName: table.Name, index: index} + m := &DropIndexMigration{tableName: table.Name, index: index} + m.Condition = &IfIndexExistsCondition{TableName: table.Name, IndexName: index.XName(table.Name)} + return m } func (m *DropIndexMigration) Sql(dialect Dialect) string { @@ -179,11 +183,6 @@ func NewRenameTableMigration(oldName string, newName string) *RenameTableMigrati return &RenameTableMigration{oldName: oldName, newName: newName} } -func (m *RenameTableMigration) IfTableExists(tableName string) *RenameTableMigration { - m.Condition = &IfTableExistsCondition{TableName: tableName} - return m -} - func (m *RenameTableMigration) Rename(oldName string, newName string) *RenameTableMigration { m.oldName = oldName m.newName = newName @@ -212,11 +211,6 @@ func NewCopyTableDataMigration(targetTable string, sourceTable string, colMap ma return m } -func (m *CopyTableDataMigration) IfTableExists(tableName string) *CopyTableDataMigration { - m.Condition = &IfTableExistsCondition{TableName: tableName} - return m -} - func (m *CopyTableDataMigration) Sql(d Dialect) string { return d.CopyTableData(m.sourceTable, m.targetTable, m.sourceCols, m.targetCols) } diff --git a/pkg/services/sqlstore/migrator/migrator.go b/pkg/services/sqlstore/migrator/migrator.go index dead6f2b416..b99d397f365 100644 --- a/pkg/services/sqlstore/migrator/migrator.go +++ b/pkg/services/sqlstore/migrator/migrator.go @@ -94,8 +94,6 @@ func (mg *Migrator) Start() error { Timestamp: time.Now(), } - mg.Logger.Debug("Executing", "sql", sql) - err := mg.inTransaction(func(sess *xorm.Session) error { err := mg.exec(m, sess) if err != nil { @@ -123,18 +121,27 @@ func (mg *Migrator) exec(m Migration, sess *xorm.Session) error { condition := m.GetCondition() if condition != nil { sql, args := condition.Sql(mg.Dialect) - results, err := sess.SQL(sql).Query(args...) - if err != nil || len(results) == 0 { - mg.Logger.Debug("Skipping migration condition not fulfilled", "id", m.Id()) - return sess.Rollback() + mg.Logger.Debug("Executing migration condition sql", "id", m.Id(), "sql", sql, "args", args) + results, err := sess.SQL(sql, args...).Query() + if err != nil { + mg.Logger.Error("Executing migration condition failed", "id", m.Id(), "error", err) + return err + } + + if !condition.IsFulfilled(results) { + mg.Logger.Warn("Skipping migration: Already executed, but not recorded in migration log", "id", m.Id()) + return nil } } var err error if codeMigration, ok := m.(CodeMigration); ok { + mg.Logger.Debug("Executing code migration", "id", m.Id()) err = codeMigration.Exec(sess, mg) } else { - _, err = sess.Exec(m.Sql(mg.Dialect)) + sql := m.Sql(mg.Dialect) + mg.Logger.Debug("Executing sql migration", "id", m.Id(), "sql", sql) + _, err = sess.Exec(sql) } if err != nil { diff --git a/pkg/services/sqlstore/migrator/mysql_dialect.go b/pkg/services/sqlstore/migrator/mysql_dialect.go index 7daa4597430..b5e16d8c500 100644 --- a/pkg/services/sqlstore/migrator/mysql_dialect.go +++ b/pkg/services/sqlstore/migrator/mysql_dialect.go @@ -90,12 +90,6 @@ func (db *Mysql) SqlType(c *Column) string { return res } -func (db *Mysql) TableCheckSql(tableName string) (string, []interface{}) { - args := []interface{}{"grafana", tableName} - sql := "SELECT `TABLE_NAME` from `INFORMATION_SCHEMA`.`TABLES` WHERE `TABLE_SCHEMA`=? and `TABLE_NAME`=?" - return sql, args -} - func (db *Mysql) UpdateTableSql(tableName string, columns []*Column) string { var statements = []string{} @@ -108,6 +102,12 @@ func (db *Mysql) UpdateTableSql(tableName string, columns []*Column) string { return "ALTER TABLE " + db.Quote(tableName) + " " + strings.Join(statements, ", ") + ";" } +func (db *Mysql) IndexCheckSql(tableName, indexName string) (string, []interface{}) { + args := []interface{}{tableName, indexName} + sql := "SELECT 1 FROM " + db.Quote("INFORMATION_SCHEMA") + "." + db.Quote("STATISTICS") + " WHERE " + db.Quote("TABLE_SCHEMA") + " = DATABASE() AND " + db.Quote("TABLE_NAME") + "=? AND " + db.Quote("INDEX_NAME") + "=?" + return sql, args +} + func (db *Mysql) CleanDB() error { tables, _ := db.engine.DBMetas() sess := db.engine.NewSession() diff --git a/pkg/services/sqlstore/migrator/postgres_dialect.go b/pkg/services/sqlstore/migrator/postgres_dialect.go index ab8812a1e26..ce529920a39 100644 --- a/pkg/services/sqlstore/migrator/postgres_dialect.go +++ b/pkg/services/sqlstore/migrator/postgres_dialect.go @@ -101,9 +101,9 @@ func (db *Postgres) SqlType(c *Column) string { return res } -func (db *Postgres) TableCheckSql(tableName string) (string, []interface{}) { - args := []interface{}{"grafana", tableName} - sql := "SELECT table_name FROM information_schema.tables WHERE table_schema=? and table_name=?" +func (db *Postgres) IndexCheckSql(tableName, indexName string) (string, []interface{}) { + args := []interface{}{tableName, indexName} + sql := "SELECT 1 FROM " + db.Quote("pg_indexes") + " WHERE" + db.Quote("tablename") + "=? AND " + db.Quote("indexname") + "=?" return sql, args } diff --git a/pkg/services/sqlstore/migrator/sqlite_dialect.go b/pkg/services/sqlstore/migrator/sqlite_dialect.go index 446e3fcef12..9c0dec05727 100644 --- a/pkg/services/sqlstore/migrator/sqlite_dialect.go +++ b/pkg/services/sqlstore/migrator/sqlite_dialect.go @@ -68,9 +68,10 @@ func (db *Sqlite3) SqlType(c *Column) string { } } -func (db *Sqlite3) TableCheckSql(tableName string) (string, []interface{}) { - args := []interface{}{tableName} - return "SELECT name FROM sqlite_master WHERE type='table' and name = ?", args +func (db *Sqlite3) IndexCheckSql(tableName, indexName string) (string, []interface{}) { + args := []interface{}{tableName, indexName} + sql := "SELECT 1 FROM " + db.Quote("sqlite_master") + " WHERE " + db.Quote("type") + "='index' AND " + db.Quote("tbl_name") + "=? AND " + db.Quote("name") + "=?" + return sql, args } func (db *Sqlite3) DropIndexSql(tableName string, index *Index) string { From a451c201291e80fd4e398e34f4ccd46fae2acb20 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 18 Dec 2018 22:28:27 +0100 Subject: [PATCH 2/3] fix handling of indices with multiple columns (mysql) --- pkg/services/sqlstore/migrator/conditions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/services/sqlstore/migrator/conditions.go b/pkg/services/sqlstore/migrator/conditions.go index 8817b43f01c..b9b145de326 100644 --- a/pkg/services/sqlstore/migrator/conditions.go +++ b/pkg/services/sqlstore/migrator/conditions.go @@ -8,7 +8,7 @@ type MigrationCondition interface { type ExistsMigrationCondition struct{} func (c *ExistsMigrationCondition) IsFulfilled(results []map[string][]byte) bool { - return len(results) == 1 + return len(results) >= 1 } type NotExistsMigrationCondition struct{} From 12f3c8f9d677f00c14c44da2425990555824afd4 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 18 Dec 2018 23:02:08 +0100 Subject: [PATCH 3/3] fix only add column if not exists for mysql --- pkg/services/sqlstore/migrator/conditions.go | 10 +++++++++ pkg/services/sqlstore/migrator/dialect.go | 5 +++++ pkg/services/sqlstore/migrator/migrations.go | 4 +++- pkg/services/sqlstore/migrator/migrator.go | 21 +++++++++++-------- .../sqlstore/migrator/mysql_dialect.go | 6 ++++++ 5 files changed, 36 insertions(+), 10 deletions(-) diff --git a/pkg/services/sqlstore/migrator/conditions.go b/pkg/services/sqlstore/migrator/conditions.go index b9b145de326..b6adfdbfa29 100644 --- a/pkg/services/sqlstore/migrator/conditions.go +++ b/pkg/services/sqlstore/migrator/conditions.go @@ -36,3 +36,13 @@ type IfIndexNotExistsCondition struct { func (c *IfIndexNotExistsCondition) Sql(dialect Dialect) (string, []interface{}) { return dialect.IndexCheckSql(c.TableName, c.IndexName) } + +type IfColumnNotExistsCondition struct { + NotExistsMigrationCondition + TableName string + ColumnName string +} + +func (c *IfColumnNotExistsCondition) Sql(dialect Dialect) (string, []interface{}) { + return dialect.ColumnCheckSql(c.TableName, c.ColumnName) +} diff --git a/pkg/services/sqlstore/migrator/dialect.go b/pkg/services/sqlstore/migrator/dialect.go index e8ea07e007c..d2e3c3e0777 100644 --- a/pkg/services/sqlstore/migrator/dialect.go +++ b/pkg/services/sqlstore/migrator/dialect.go @@ -33,6 +33,7 @@ type Dialect interface { UpdateTableSql(tableName string, columns []*Column) string IndexCheckSql(tableName, indexName string) (string, []interface{}) + ColumnCheckSql(tableName, columnName string) (string, []interface{}) ColString(*Column) string ColStringNoPk(*Column) string @@ -183,6 +184,10 @@ func (db *BaseDialect) RenameTable(oldName string, newName string) string { return fmt.Sprintf("ALTER TABLE %s RENAME TO %s", quote(oldName), quote(newName)) } +func (db *BaseDialect) ColumnCheckSql(tableName, columnName string) (string, []interface{}) { + return "", nil +} + func (db *BaseDialect) DropIndexSql(tableName string, index *Index) string { quote := db.dialect.Quote name := index.XName(tableName) diff --git a/pkg/services/sqlstore/migrator/migrations.go b/pkg/services/sqlstore/migrator/migrations.go index d5a4158c52a..cb32c6212e6 100644 --- a/pkg/services/sqlstore/migrator/migrations.go +++ b/pkg/services/sqlstore/migrator/migrations.go @@ -85,7 +85,9 @@ type AddColumnMigration struct { } func NewAddColumnMigration(table Table, col *Column) *AddColumnMigration { - return &AddColumnMigration{tableName: table.Name, column: col} + m := &AddColumnMigration{tableName: table.Name, column: col} + m.Condition = &IfColumnNotExistsCondition{TableName: table.Name, ColumnName: col.Name} + return m } func (m *AddColumnMigration) Table(tableName string) *AddColumnMigration { diff --git a/pkg/services/sqlstore/migrator/migrator.go b/pkg/services/sqlstore/migrator/migrator.go index b99d397f365..ce0b2663def 100644 --- a/pkg/services/sqlstore/migrator/migrator.go +++ b/pkg/services/sqlstore/migrator/migrator.go @@ -121,16 +121,19 @@ func (mg *Migrator) exec(m Migration, sess *xorm.Session) error { condition := m.GetCondition() if condition != nil { sql, args := condition.Sql(mg.Dialect) - mg.Logger.Debug("Executing migration condition sql", "id", m.Id(), "sql", sql, "args", args) - results, err := sess.SQL(sql, args...).Query() - if err != nil { - mg.Logger.Error("Executing migration condition failed", "id", m.Id(), "error", err) - return err - } - if !condition.IsFulfilled(results) { - mg.Logger.Warn("Skipping migration: Already executed, but not recorded in migration log", "id", m.Id()) - return nil + if sql != "" { + mg.Logger.Debug("Executing migration condition sql", "id", m.Id(), "sql", sql, "args", args) + results, err := sess.SQL(sql, args...).Query() + if err != nil { + mg.Logger.Error("Executing migration condition failed", "id", m.Id(), "error", err) + return err + } + + if !condition.IsFulfilled(results) { + mg.Logger.Warn("Skipping migration: Already executed, but not recorded in migration log", "id", m.Id()) + return nil + } } } diff --git a/pkg/services/sqlstore/migrator/mysql_dialect.go b/pkg/services/sqlstore/migrator/mysql_dialect.go index b5e16d8c500..4de55c9a12f 100644 --- a/pkg/services/sqlstore/migrator/mysql_dialect.go +++ b/pkg/services/sqlstore/migrator/mysql_dialect.go @@ -108,6 +108,12 @@ func (db *Mysql) IndexCheckSql(tableName, indexName string) (string, []interface return sql, args } +func (db *Mysql) ColumnCheckSql(tableName, columnName string) (string, []interface{}) { + args := []interface{}{tableName, columnName} + sql := "SELECT 1 FROM " + db.Quote("INFORMATION_SCHEMA") + "." + db.Quote("COLUMNS") + " WHERE " + db.Quote("TABLE_SCHEMA") + " = DATABASE() AND " + db.Quote("TABLE_NAME") + "=? AND " + db.Quote("COLUMN_NAME") + "=?" + return sql, args +} + func (db *Mysql) CleanDB() error { tables, _ := db.engine.DBMetas() sess := db.engine.NewSession()