From f978967ef135c7c38a1709309abaca32be9f5f74 Mon Sep 17 00:00:00 2001 From: zhangwenjian Date: Tue, 8 Sep 2026 16:57:15 +0800 Subject: [PATCH] =?UTF-8?q?fix=F0=9F=90=9B:=20drop=20the=20index=20with=20?= =?UTF-8?q?SQL=20this=20dialect=20can=20parse?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The soft-delete conversion dropped the indexes on deleted_at through gorm's Migrator().DropIndex. Its PostgreSQL driver resolves a schema for the statement and falls back to an expression when it cannot: currentSchema, _ := m.CurrentSchema(stmt, stmt.Table) m.DB.Exec("DROP INDEX ?.?", currentSchema, clause.Column{Name: name}) DROP INDEX takes an identifier in that position, so what reached the server was DROP INDEX CURRENT_SCHEMA()."idx_sys_api_deleted_at" ERROR: syntax error at or near "CURRENT_SCHEMA" (SQLSTATE 42601) The schema is unresolvable for every call this migration makes, because it passes a table name as a string rather than a model. So it failed on every PostgreSQL database rather than intermittently, and stopped the whole conversion at the first table. What that looked like from outside is go-admin#919: an upgrade that could not complete, and a login rejecting a correct password, because deleted_at was still a timestamptz while the current query compares it to 0. Neither symptom names a migration. Written per dialect, for the same reason addBigIntColumn and renameColumn already are. MySQL and SQL Server name the table and have no IF EXISTS for it; PostgreSQL and SQLite name the index alone. Verified by running the shipped migration against PostgreSQL 15 and MySQL 8.0 in containers, and SQLite through this package's tests. The SQL Server form is from its documentation and has not been run - there is no SQL Server here to run it against, and saying so is better than implying four dialects were checked. --- .../1786700003000_soft_delete_marker.go | 42 ++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/cmd/migrate/migration/version/1786700003000_soft_delete_marker.go b/cmd/migrate/migration/version/1786700003000_soft_delete_marker.go index 4bc7ef85..f6cd1747 100644 --- a/cmd/migrate/migration/version/1786700003000_soft_delete_marker.go +++ b/cmd/migrate/migration/version/1786700003000_soft_delete_marker.go @@ -237,13 +237,53 @@ func dropIndexesOn(db *gorm.DB, table, column string) error { if !m.HasIndex(table, name) { continue } - if err := m.DropIndex(table, name); err != nil { + if err := db.Exec(dropIndex(db, table, name)).Error; err != nil { return fmt.Errorf("dropping index %s: %w", name, err) } } return nil } +// dropIndex spells DROP INDEX for one dialect, rather than going through +// Migrator().DropIndex. +// +// The migrator cannot be used here on PostgreSQL. Its driver resolves a schema +// for the statement and falls back to an expression when it cannot: +// +// currentSchema, _ := m.CurrentSchema(stmt, stmt.Table) // CURRENT_SCHEMA() +// m.DB.Exec("DROP INDEX ?.?", currentSchema, clause.Column{Name: name}) +// +// DROP INDEX takes an identifier in that position, not an expression, so the +// statement does not parse. The schema is unresolvable for every call made +// here, because this passes a table name as a string rather than a model - so +// it failed on every PostgreSQL database rather than intermittently, and took +// the whole conversion with it. Reported as go-admin#919, where the visible +// symptom was a login rejecting a correct password: the migration had stopped +// here, leaving deleted_at a timestamptz that the current query compares to 0. +// +// Written per dialect for the same reason addBigIntColumn and renameColumn +// already are. +// +// MySQL and SQL Server name the table in the statement and have no IF EXISTS +// for it; PostgreSQL and SQLite name the index alone, in its own namespace. +// The caller has already checked HasIndex, so IF EXISTS is only there to make +// the two that support it say nothing rather than fail on a race with another +// migrator. +// +// Verified against PostgreSQL 15, MySQL 8.0 and SQLite. The SQL Server form is +// from its documentation and has not been run - this repository has no SQL +// Server to run it against. +func dropIndex(db *gorm.DB, table, index string) string { + switch db.Dialector.Name() { + case "mysql": + return fmt.Sprintf("DROP INDEX `%s` ON `%s`", index, table) + case "sqlserver": + return fmt.Sprintf("DROP INDEX [%s] ON [%s]", index, table) + default: + return fmt.Sprintf(`DROP INDEX IF EXISTS "%s"`, index) + } +} + // indexNamesFor asks the database which indexes cover column. func indexNamesFor(db *gorm.DB, table, column string) ([]string, error) { indexes, err := db.Migrator().GetIndexes(table)