test✅: fail these tests when the database does not answer

Every Create and Raw dropped its error. Most of them would have failed
an assertion further down anyway, with a message describing the wrong
problem - but one of them would not.

The last count in the index test reads how many indexes survived and
expects zero. An unchecked query that fails leaves the variable at zero,
and zero is what success looks like: a test that cannot reach the
database reports that the indexes were dropped.

Demonstrated rather than assumed, by breaking that one query both ways:

    with the check:    FAIL  counting the indexes after: relation
                             "pg_indexes_nope" does not exist
    without it:        PASS

Raised by Copilot on #922, as a consistency point with the SQLite tests
in this package. It is that as well, but the reason it is worth doing is
the row above.

.github/workflows/go.yml

@@ -33,8 +33,27 @@ jobs:
           --health-timeout 3s
           --health-retries 10
 
+      postgres:
+        image: postgres:15-alpine
+        env:
+          POSTGRES_PASSWORD: postgres
+          POSTGRES_DB: goadmin_test
+        ports:
+          - 5432:5432
+        options: >-
+          --health-cmd "pg_isready -U postgres"
+          --health-interval 5s
+          --health-timeout 3s
+          --health-retries 10
+
     env:
       GO_ADMIN_TEST_REDIS_ADDR: 127.0.0.1:6379
+      # The soft-delete conversion drops an index, and gorm's PostgreSQL driver
+      # produced unparseable SQL for that - on SQLite, where the rest of these
+      # tests run, the same code works. The suite reported success for a
+      # migration that failed on every PostgreSQL database it was pointed at.
+      # See go-admin#919.
+      GO_ADMIN_TEST_POSTGRES_DSN: "host=127.0.0.1 port=5432 user=postgres password=postgres dbname=goadmin_test sslmode=disable"
 
     steps:
 

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)

cmd/migrate/migration/version/1786700003000_soft_delete_marker_postgres_test.go

@@ -0,0 +1,156 @@
+package version
+
+import (
+	"os"
+	"testing"
+	"time"
+
+	"gorm.io/driver/postgres"
+	"gorm.io/gorm"
+)
+
+// postgresDSNEnv points these tests at a database. They are skipped without
+// it, so a developer with no PostgreSQL running still gets a green run.
+//
+// The whole file exists because the rest of this package's tests run on
+// SQLite, where the defect they cover cannot happen: dropping an index through
+// gorm's migrator works there and produces unparseable SQL on PostgreSQL. A
+// suite that only ever exercised SQLite reported success for a migration that
+// failed on every PostgreSQL database it was pointed at - go-admin#919.
+const postgresDSNEnv = "GO_ADMIN_TEST_POSTGRES_DSN"
+
+func postgresDB(t *testing.T) *gorm.DB {
+	t.Helper()
+
+	dsn := os.Getenv(postgresDSNEnv)
+	if dsn == "" {
+		// Skipping locally is the point; skipping in CI is the failure this
+		// file exists to prevent. A workflow that renamed the variable or
+		// dropped the service would otherwise go green while these tests
+		// quietly did nothing - the same shape as the defect they cover.
+		if os.Getenv("CI") != "" {
+			t.Fatalf("%s is not set while CI is: the PostgreSQL migration tests must not skip here", postgresDSNEnv)
+		}
+		t.Skipf("%s is not set; skipping the PostgreSQL migration tests", postgresDSNEnv)
+	}
+
+	db, err := gorm.Open(postgres.Open(dsn), &gorm.Config{})
+	if err != nil {
+		t.Fatalf("connecting to %s: %v", postgresDSNEnv, err)
+	}
+	return db
+}
+
+// pgOldUser is the pre-migration shape: a nullable timestamp with an index on
+// it, which is what makes dropping the column require dropping the index.
+type pgOldUser struct {
+	UserId    int64 `gorm:"column:user_id;primaryKey;autoIncrement"`
+	Username  string
+	DeletedAt *time.Time `gorm:"index"`
+}
+
+func (pgOldUser) TableName() string { return "sd_pg_user" }
+
+// The conversion completes on PostgreSQL.
+//
+// It did not. dropIndexesOn went through Migrator().DropIndex, whose
+// PostgreSQL driver falls back to an expression when it cannot resolve a
+// schema - which is every call made here, because the migration passes a table
+// name as a string:
+//
+//	DROP INDEX CURRENT_SCHEMA()."idx_sd_pg_user_deleted_at"
+//
+// DROP INDEX takes an identifier there, so it failed to parse and took the
+// whole conversion with it. Every PostgreSQL deployment stopped at this
+// migration, and the visible symptom was a login rejecting a correct password
+// because deleted_at was still a timestamptz being compared to 0.
+func TestConversionCompletesOnPostgres(t *testing.T) {
+	db := postgresDB(t)
+	t.Cleanup(func() { db.Migrator().DropTable(&pgOldUser{}) })
+
+	db.Migrator().DropTable(&pgOldUser{})
+	if err := db.AutoMigrate(&pgOldUser{}); err != nil {
+		t.Fatalf("building the old shape: %v", err)
+	}
+
+	deleted := time.Now().Add(-time.Hour)
+	// Checked rather than fired and forgotten: a failed insert leaves the
+	// assertions below reading an empty table, and "no rows" is a shape some
+	// of them cannot tell from success.
+	if err := db.Create(&pgOldUser{Username: "gone", DeletedAt: &deleted}).Error; err != nil {
+		t.Fatalf("seeding the deleted row: %v", err)
+	}
+	if err := db.Create(&pgOldUser{Username: "live"}).Error; err != nil {
+		t.Fatalf("seeding the live row: %v", err)
+	}
+
+	if err := convertDeletedAt(db, "sd_pg_user"); err != nil {
+		t.Fatalf("convertDeletedAt: %v", err)
+	}
+
+	var dataType string
+	if err := db.Raw(`SELECT data_type FROM information_schema.columns
+	        WHERE table_name = 'sd_pg_user' AND column_name = 'deleted_at'`).Scan(&dataType).Error; err != nil {
+		t.Fatalf("reading the column type: %v", err)
+	}
+	if dataType != "bigint" {
+		t.Errorf("deleted_at is %q after the conversion, want bigint", dataType)
+	}
+
+	// The marker has to carry the timestamp across, or a row that was deleted
+	// comes back live.
+	var markers []int64
+	if err := db.Raw(`SELECT deleted_at FROM sd_pg_user ORDER BY user_id`).Scan(&markers).Error; err != nil {
+		t.Fatalf("reading the markers: %v", err)
+	}
+	if len(markers) != 2 {
+		t.Fatalf("read %d rows, want 2", len(markers))
+	}
+	if markers[0] == 0 {
+		t.Error("the deleted row came back live")
+	}
+	if markers[1] != 0 {
+		t.Errorf("the live row is marked deleted at %d", markers[1])
+	}
+}
+
+// The index on deleted_at is gone afterwards, which is the step that failed.
+//
+// Asserted separately from the conversion because the conversion can succeed
+// on a table with no index at all, and this migration exists for tables that
+// have one.
+func TestTheIndexOnDeletedAtIsDroppedOnPostgres(t *testing.T) {
+	db := postgresDB(t)
+	t.Cleanup(func() { db.Migrator().DropTable(&pgOldUser{}) })
+
+	db.Migrator().DropTable(&pgOldUser{})
+	if err := db.AutoMigrate(&pgOldUser{}); err != nil {
+		t.Fatalf("building the old shape: %v", err)
+	}
+
+	var before int64
+	if err := db.Raw(`SELECT count(*) FROM pg_indexes
+	        WHERE tablename = 'sd_pg_user' AND indexdef LIKE '%deleted_at%'`).Scan(&before).Error; err != nil {
+		t.Fatalf("counting the indexes before: %v", err)
+	}
+	if before == 0 {
+		t.Fatal("the old shape has no index on deleted_at, so this test asserts nothing")
+	}
+
+	if err := dropIndexesOn(db, "sd_pg_user", "deleted_at"); err != nil {
+		t.Fatalf("dropIndexesOn: %v", err)
+	}
+
+	// This one is why the errors are checked at all rather than as a matter of
+	// habit: a query that fails leaves after at zero, and zero is what success
+	// looks like. An unchecked error here is a test that passes when it cannot
+	// reach the database.
+	var after int64
+	if err := db.Raw(`SELECT count(*) FROM pg_indexes
+	        WHERE tablename = 'sd_pg_user' AND indexdef LIKE '%deleted_at%'`).Scan(&after).Error; err != nil {
+		t.Fatalf("counting the indexes after: %v", err)
+	}
+	if after != 0 {
+		t.Errorf("%d index(es) on deleted_at survived", after)
+	}
+}