From 273f88721e68e186c886076e5ce8e4b8cf601836 Mon Sep 17 00:00:00 2001 From: MHSanaei Date: Wed, 1 Jul 2026 23:07:05 +0200 Subject: [PATCH] fix(database): stop noisy per-startup errors in the Postgres server log Two statements failed server-side on every panel start after a SQLite to Postgres migration, flooding the postgres log even though the Go side suppressed them: - resyncPostgresSequences issued SELECT MAX(id) against client_inbounds, whose composite primary key has no id column; Postgres validates the SELECT list at parse time, so the WHERE pg_get_serial_sequence(...) guard never got a chance to no-op it. Skip models whose GORM schema maps no id column before issuing the statement. - AutoMigrate detects existing columns via information_schema filtered by table_catalog = CURRENT_DATABASE(), which misdetects on some setups and re-issues ALTER TABLE ... ADD for columns that already exist. HasColumn/ HasIndex query without that filter and are reliable (the existing duplicate-column suppressor depends on exactly that), so skip AutoMigrate outright when the table, every column, and every index already exist. Closes #5665 --- internal/database/db.go | 27 +++++++++++++ internal/database/db_settled_test.go | 57 ++++++++++++++++++++++++++++ internal/database/migrate_data.go | 26 ++++++++----- 3 files changed, 101 insertions(+), 9 deletions(-) create mode 100644 internal/database/db_settled_test.go diff --git a/internal/database/db.go b/internal/database/db.go index 4de124d48..2b33997d6 100644 --- a/internal/database/db.go +++ b/internal/database/db.go @@ -83,6 +83,9 @@ func initModels() error { &model.OutboundSubscription{}, } for _, mdl := range models { + if IsPostgres() && postgresModelSettled(mdl) { + continue + } if err := db.AutoMigrate(mdl); err != nil { if isIgnorableDuplicateColumnErr(err, mdl) { log.Printf("Ignoring duplicate column during auto migration for %T: %v", mdl, err) @@ -119,6 +122,30 @@ func initModels() error { return nil } +// postgresModelSettled skips AutoMigrate when table, columns, and indexes all exist: +// its catalog-filtered column probe misdetects on some setups and re-ADDs columns forever (#5665). +func postgresModelSettled(mdl any) bool { + migrator := db.Migrator() + if !migrator.HasTable(mdl) { + return false + } + stmt := &gorm.Statement{DB: db} + if err := stmt.Parse(mdl); err != nil || stmt.Schema == nil { + return false + } + for _, dbName := range stmt.Schema.DBNames { + if !migrator.HasColumn(mdl, dbName) { + return false + } + } + for _, idx := range stmt.Schema.ParseIndexes() { + if !migrator.HasIndex(mdl, idx.Name) { + return false + } + } + return true +} + func dropLegacyForeignKeys() error { if !IsPostgres() { return nil diff --git a/internal/database/db_settled_test.go b/internal/database/db_settled_test.go new file mode 100644 index 000000000..e2ffb1720 --- /dev/null +++ b/internal/database/db_settled_test.go @@ -0,0 +1,57 @@ +package database + +import ( + "path/filepath" + "testing" + + "github.com/mhsanaei/3x-ui/v3/internal/database/model" +) + +// Locks the #5665 guard: composite-PK client_inbounds has no id column, so the +// sequence-reset SQL must never be issued for it. +func TestTableWithIdColumn_SkipsCompositeKeyModels(t *testing.T) { + if err := InitDB(filepath.Join(t.TempDir(), "x-ui.db")); err != nil { + t.Fatalf("InitDB: %v", err) + } + t.Cleanup(func() { _ = CloseDB() }) + + if table, ok := tableWithIdColumn(db, &model.ClientInbound{}); ok { + t.Errorf("ClientInbound (table %q) has no id column but was not skipped", table) + } + table, ok := tableWithIdColumn(db, &model.Inbound{}) + if !ok { + t.Fatal("Inbound has an id column but was reported as skippable") + } + if table != "inbounds" { + t.Errorf("Inbound table = %q, want inbounds", table) + } +} + +// Exercises the #5665 AutoMigrate skip on SQLite (the check is dialect-agnostic): +// settled after InitDB, not settled with a missing column or table. +func TestPostgresModelSettled_TracksSchemaPresence(t *testing.T) { + if err := InitDB(filepath.Join(t.TempDir(), "x-ui.db")); err != nil { + t.Fatalf("InitDB: %v", err) + } + t.Cleanup(func() { _ = CloseDB() }) + + for _, mdl := range []any{&model.ClientRecord{}, &model.ClientGroup{}, &model.ClientInbound{}} { + if !postgresModelSettled(mdl) { + t.Errorf("%T not settled right after InitDB", mdl) + } + } + + if err := db.Migrator().DropColumn(&model.ClientGroup{}, "reset_up"); err != nil { + t.Fatalf("drop column: %v", err) + } + if postgresModelSettled(&model.ClientGroup{}) { + t.Error("ClientGroup settled despite missing reset_up column") + } + + if err := db.Migrator().DropTable(&model.ClientGroup{}); err != nil { + t.Fatalf("drop table: %v", err) + } + if postgresModelSettled(&model.ClientGroup{}) { + t.Error("ClientGroup settled despite missing table") + } +} diff --git a/internal/database/migrate_data.go b/internal/database/migrate_data.go index be00ee8d4..00ebaa3e4 100644 --- a/internal/database/migrate_data.go +++ b/internal/database/migrate_data.go @@ -270,19 +270,14 @@ func resetPostgresSequences(dst *gorm.DB) error { return resyncPostgresSequences(dst, migrationModels()) } -// resyncPostgresSequences sets each model's id sequence to MAX(id) so the next -// auto-increment INSERT won't collide with an existing row. Table names are -// resolved from the models themselves (not hardcoded), so they always match the -// migrated tables. The statement is a no-op for tables without an id sequence -// (e.g. composite-PK tables), and idempotent on a healthy DB, so it is safe to -// run both after migration and on every Postgres startup. +// resyncPostgresSequences sets each model's id sequence to MAX(id); idempotent. Id-less +// composite-PK tables are skipped — Postgres rejects MAX(id) at parse time and logs it (#5665). func resyncPostgresSequences(db *gorm.DB, models []any) error { for _, m := range models { - stmt := &gorm.Statement{DB: db} - if err := stmt.Parse(m); err != nil { + t, ok := tableWithIdColumn(db, m) + if !ok { continue } - t := stmt.Table // t comes from the trusted model set parsed by GORM, not user input, so // interpolating it as an identifier is safe. We ignore errors per-table. _ = db.Exec( @@ -293,3 +288,16 @@ func resyncPostgresSequences(db *gorm.DB, models []any) error { } return nil } + +// tableWithIdColumn resolves a model's table name and reports whether its GORM +// schema maps an "id" database column. +func tableWithIdColumn(db *gorm.DB, m any) (string, bool) { + stmt := &gorm.Statement{DB: db} + if err := stmt.Parse(m); err != nil { + return "", false + } + if stmt.Schema == nil || stmt.Schema.LookUpField("id") == nil { + return "", false + } + return stmt.Table, true +}