From 91bf25e5fe86d6624a4caab9cb53eee4fe1b6e1b Mon Sep 17 00:00:00 2001 From: zhangwenjian Date: Sun, 23 Aug 2026 12:56:50 +0800 Subject: [PATCH] =?UTF-8?q?fix=F0=9F=90=9B:=20the=20soft-delete=20migratio?= =?UTF-8?q?n=20could=20not=20run=20against=20the=20real=20schema?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two assumptions held on the test's table and on nothing else. It dropped deleted_at while an index still referred to it. MySQL and PostgreSQL drop dependent indexes along with the column; SQLite refuses, and the migration stopped at the first table with such an index - which is all thirteen of them. It also read the rows through a column named id. sys_dept keys on dept_id, sys_user on user_id, and only some tables on id, so the pass that carries the deletion timestamps across never ran. The test's table had an id key and no index on deleted_at, which is exactly the shape that lets both through. It now matches sys_user. --- .../1786700003000_soft_delete_marker.go | 73 ++++++++++++++++++- .../1786700003000_soft_delete_marker_test.go | 12 ++- 2 files changed, 80 insertions(+), 5 deletions(-) diff --git a/cmd/migrate/migration/version/1786700003000_soft_delete_marker.go b/cmd/migrate/migration/version/1786700003000_soft_delete_marker.go index 63ba4324..4bc7ef85 100644 --- a/cmd/migrate/migration/version/1786700003000_soft_delete_marker.go +++ b/cmd/migrate/migration/version/1786700003000_soft_delete_marker.go @@ -131,6 +131,14 @@ func convertDeletedAt(db *gorm.DB, table string) error { return err } + // SQLite refuses to drop a column an index still refers to, where MySQL and + // PostgreSQL drop the dependent indexes along with it. Drop them first, on + // every dialect: the column is about to be replaced by one of the same name, + // and gorm recreates the index from the model's tag. + if err := dropIndexesOn(db, table, "deleted_at"); err != nil { + return err + } + if err := db.Exec(dropColumn(db, table, "deleted_at")).Error; err != nil { return fmt.Errorf("dropping deleted_at: %w", err) } @@ -166,8 +174,15 @@ func copyTimestamps(db *gorm.DB, table string) error { DeletedAt *time.Time } + // The tables walked here do not agree on what the key is called: sys_dept + // keys on dept_id, sys_user on user_id, and only some on id. + key, err := primaryKeyOf(db, table) + if err != nil { + return err + } + var rows []row - q := fmt.Sprintf("SELECT id, deleted_at FROM %s WHERE deleted_at IS NOT NULL", table) + q := fmt.Sprintf("SELECT %s AS id, deleted_at FROM %s WHERE deleted_at IS NOT NULL", key, table) if err := db.Raw(q).Scan(&rows).Error; err != nil { return fmt.Errorf("reading deleted rows: %w", err) } @@ -176,7 +191,7 @@ func copyTimestamps(db *gorm.DB, table string) error { if r.DeletedAt == nil { continue } - u := fmt.Sprintf("UPDATE %s SET %s = ? WHERE id = ?", table, tempColumn) + u := fmt.Sprintf("UPDATE %s SET %s = ? WHERE %s = ?", table, tempColumn, key) if err := db.Exec(u, r.DeletedAt.UnixMilli(), r.Id).Error; err != nil { return fmt.Errorf("marking row %d deleted: %w", r.Id, err) } @@ -184,6 +199,21 @@ func copyTimestamps(db *gorm.DB, table string) error { return nil } +// primaryKeyOf asks the database which column is the primary key, rather than +// assuming the name. +func primaryKeyOf(db *gorm.DB, table string) (string, error) { + columns, err := db.Migrator().ColumnTypes(table) + if err != nil { + return "", fmt.Errorf("reading columns of %s: %w", table, err) + } + for _, c := range columns { + if isKey, ok := c.PrimaryKey(); ok && isKey { + return c.Name(), nil + } + } + return "", fmt.Errorf("no primary key on %s", table) +} + func createUniqueIndex(db *gorm.DB, table, column, name string) error { if db.Migrator().HasIndex(table, name) { return nil @@ -193,6 +223,45 @@ func createUniqueIndex(db *gorm.DB, table, column, name string) error { )).Error } +// dropIndexesOn removes every index on table that mentions column, so the +// column can be dropped. Named indexes are asked of the migrator rather than +// guessed, because the names differ between a schema gorm created and one a +// hand-written migration did. +func dropIndexesOn(db *gorm.DB, table, column string) error { + names, err := indexNamesFor(db, table, column) + if err != nil { + return fmt.Errorf("listing indexes on %s.%s: %w", table, column, err) + } + m := db.Migrator() + for _, name := range names { + if !m.HasIndex(table, name) { + continue + } + if err := m.DropIndex(table, name); err != nil { + return fmt.Errorf("dropping index %s: %w", name, err) + } + } + return nil +} + +// indexNamesFor asks the database which indexes cover column. +func indexNamesFor(db *gorm.DB, table, column string) ([]string, error) { + indexes, err := db.Migrator().GetIndexes(table) + if err != nil { + return nil, err + } + var names []string + for _, idx := range indexes { + for _, c := range idx.Columns() { + if c == column { + names = append(names, idx.Name()) + break + } + } + } + return names, nil +} + // The three statements every dialect spells differently. func addBigIntColumn(db *gorm.DB, table, column string) string { diff --git a/cmd/migrate/migration/version/1786700003000_soft_delete_marker_test.go b/cmd/migrate/migration/version/1786700003000_soft_delete_marker_test.go index 343f16fc..eaa3d768 100644 --- a/cmd/migrate/migration/version/1786700003000_soft_delete_marker_test.go +++ b/cmd/migrate/migration/version/1786700003000_soft_delete_marker_test.go @@ -10,10 +10,16 @@ import ( // oldUser is the shape of the table before this migration: deleted_at is a // nullable timestamp, and nothing constrains the username. +// +// It mirrors the real sys_user in the two respects that matter here, both of +// which the migration got wrong against a live database while an id-keyed, +// unindexed table let it pass: the key is user_id rather than id, and +// deleted_at carries an index, which SQLite will not let a column be dropped +// out from under. type oldUser struct { - Id int64 `gorm:"primaryKey;autoIncrement"` + UserId int64 `gorm:"column:user_id;primaryKey;autoIncrement"` Username string - DeletedAt *time.Time + DeletedAt *time.Time `gorm:"index"` } func (oldUser) TableName() string { return "sys_user" } @@ -53,7 +59,7 @@ func TestConvertsDeletedAtAndKeepsWhoWasDeleted(t *testing.T) { Username string DeletedAt int64 } - if err := db.Raw("SELECT username, deleted_at FROM sys_user ORDER BY id").Scan(&marks).Error; err != nil { + if err := db.Raw("SELECT username, deleted_at FROM sys_user ORDER BY user_id").Scan(&marks).Error; err != nil { t.Fatalf("read back: %v", err) } if len(marks) != 2 {