Seeding needs something to look for before it inserts, or a retry writes
a second copy of everything it already wrote. sys_api already had one in
(app_code, path, action). sys_menu had nothing usable: menu_name is
pascalCase(appCode) + pascalCase(code), which is not injective -
"list-all", "listAll" and "list_all" all become "ListAll" - so the
original code cannot be recovered from it. Hence a new column.
seed_code is nullable, against this repository's habit of NOT NULL
DEFAULT '' for a new column, and deliberately. Every row that predates
it has no meaningful value, and under a unique index an empty string
collides with every other empty string while NULL collides with nothing.
The convention exists because deleted_at's nullability broke a unique
index; here nullability is what makes one possible.
The unique index on sys_api cannot simply be created: a live database is
known to hold historical duplicates - the demo site had eighteen. The
migration looks first and refuses while naming the offending rows,
rather than letting CREATE UNIQUE INDEX fail with a constraint error
that names none. Same shape as 1786700003000's refuseOnDuplicates.
Run against SQLite, MySQL 8.0 and PostgreSQL 15, including the CONCAT
duplicate check, which had only ever been executed by SQLite's driver.
sys_app is one row per installed application. It is physically deleted
on uninstall rather than following the millisecond soft-delete marker
the other sys_ tables use: an installed-app registry has no "deleted by
accident, needs recovering" case, and a physical delete is what lets the
same code be installed again afterwards.
status is installing/installed/failed rather than a boolean, because an
install spanning several migration files is not atomic on MySQL - DDL
commits implicitly, so a run can stop in the middle. failed_version and
last_error are diagnostic snapshots for a person to read; nothing may
decide anything from them, and the field comments say so. Where to
resume is answered by sys_migration, which cannot drift from what was
actually applied.
sys_app_casbin_grant records which casbin_rule rows an install created,
keyed by casbin_rule's own natural key. That table is not extended
instead: gorm-adapter's SavePolicy truncates and reloads it from an
in-memory model, which would drop any column added here without a word.
Built against SQLite, MySQL 8.0 and PostgreSQL 15.
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.
The rest of this package's tests run on SQLite, where dropping an index
through the migrator works. That is why a migration which failed on
every PostgreSQL database it was pointed at had a green suite: the
defect cannot occur on the backend being tested.
A test alone would not have helped either - without a service it skips,
and a test that never runs is the same as no test. So the workflow gains
a postgres service and the DSN, and the helper refuses to skip when CI
is set: a workflow that drops the service or renames the variable fails
rather than going quietly green, which is the shape of the original
defect.
Counter-proved by putting Migrator().DropIndex back, which reproduces
the statement verbatim:
DROP INDEX CURRENT_SCHEMA()."idx_sd_pg_user_deleted_at"
ERROR: syntax error at or near "CURRENT_SCHEMA" (SQLSTATE 42601)
The conversion test also checks the timestamp survives as a marker,
since a conversion that dropped it would bring deleted rows back live
while still passing a column-type assertion.
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.
sys_migration already carries app_code; sys_menu and sys_api did not, so
nothing said which application seeded a row - which is what an uninstall or
an audit would have to ask.
The migration adds the columns through the runtime models rather than
cmd/migrate/migration/models, whose frozen ModelTime is wrong for anything
ordered after the soft-delete conversion.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
`NULL NOT IN (...)` evaluates to NULL rather than true, so the previous
condition left a NULL data_scope exactly as it found it - and NULL is the one
value that most needs the repair: it scans into a Go string as "", which is
what the fail-closed default now refuses.
The column has no NOT NULL constraint, so the value is reachable from any
writer that is not the admin UI.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
The seed fix only reaches new installations. An install that imported the old
db.sql already has an administrator with an empty data_scope, and after the
fail-closed change that account sees nothing.
The migration rewrites any value outside "1".."5" to "1", which is the
behaviour those rows had before. Plain SQL rather than the frozen migration
models, per the rule that migrations after 1786700003000 must not use them.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
Migrations ordered after 1786700003000 must not seed through
cmd/migrate/migration/models. That package's ModelTime declares a nullable
gorm.DeletedAt, which is the shape the columns had until that migration
converted deleted_at to a NOT NULL millisecond marker.
Afterwards it breaks in both directions. Writes put NULL into a NOT NULL
column and fail on the first insert. Reads are scoped "WHERE deleted_at IS
NULL" while live rows hold 0, so they match nothing - and 1786700001000
looks the admin role up that way and treats ErrRecordNotFound as "roles are
not seeded yet, skip authorisation", which would leave a module seeded with
no permissions and the migration still recorded as applied.
A fresh database does not surface either one: every migration using that
package today is ordered before the conversion, so it runs while the column
is still nullable. Only a migration added afterwards hits it.
Also pulls the import scan out of importsRuntimeModels so both checks share
one implementation, and derives the version through migration.GetFilename
rather than a second filename-parsing rule.
Verified by adding a violating migration and confirming the test fails with
an actionable message, then removing it and confirming the suite passes.
Claude-Session: https://claude.ai/code/session_01DJhM6LvhkNPej35wy9F7Aq
sys_columns and sys_tables were left out of the soft-delete conversion in
1786700003000. Their runtime models embed common.ModelTime, which is the
millisecond marker, so GORM queries them with deleted_at = 0 - against a
nullable datetime column holding NULL. Every row was invisible.
The repository carries two ModelTime types: the one under
cmd/migrate/migration/models still has a nullable gorm.DeletedAt and is
what builds the tables, while common/models has the marker and is what
queries them. Nothing connected the two, so a table could be built one
way and read the other with no signal at all.
The test now walks app/ for models embedding the marker and requires a
migration to cover each. tb_demo is exempt and says why: nothing reads it
at runtime.
sort is gorm:"size:4", which MySQL builds as a tinyint holding -128..127.
The demo menu seeded Sort: 900, so on MySQL the run stopped at
1786700001000 with Error 1264, and every migration after it - including
the soft-delete conversion - never ran.
deleted_at therefore stayed NULL while the code queries deleted_at = 0,
and the login returned 'incorrect Username or Password' on a database
whose password hash was correct all along.
sqlite ignores the declared width, so a fresh install there passed and
the fault only appeared on MySQL.
Every import of the module changes, not only the seven packages that
moved out of sdk/pkg: Go requires the major version in the path from v2
on. Both happen in one pass —
go run github.com/go-admin-team/go-admin-core/tools/coreupgrade@v2.0.0 -w -v2 .
go mod tidy
— which is the command the release notes give, run here as a consumer
would run it. 210 imports across 95 files.
The compatibility shims this used are gone in v2, so the paths that
moved had to move: sdk/pkg/captcha, sdk/pkg/jwtauth and its user
package, sdk/pkg/response and sdk/pkg/casbin.
The count of unformatted files is unchanged at 34, none of them touched
by this: the tool reformats a file only if it was already gofmt clean,
so a migration cannot disappear into whitespace.
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.
sys_user.username, sys_role.role_key and sys_dict_type.dict_type had no
unique index. Uniqueness was a SELECT COUNT followed by an INSERT, which
two concurrent requests both pass — and login resolves a username with
First, so which of the two accounts answers is whichever the database
returns.
The index cannot be on the key alone, because a soft-deleted row keeps
occupying the name and a deleted user's username could never be used
again. It has to include the delete marker, and the marker has to be
non-null: two live rows are (alice, NULL) and (alice, NULL), and NULL is
not equal to NULL, so an index over a nullable marker admits both. That
is the worst of the three states — a constraint that reads as protection
and binds nothing — and there is a test that demonstrates it rather than
asserting it.
ModelTime.DeletedAt is milliseconds since the epoch now, zero while the
row is live. Sixteen tables carry it; the migration converts each one,
preserving when each deleted row was deleted, then adds the three
indexes.
Written to be re-runnable rather than transactional, because DDL does not
roll back on MySQL and an operator whose first attempt failed halfway
should have nothing to do but run it again. It refuses before altering
anything if a table already holds duplicates, naming them, rather than
letting the index fail and leaving the operator to guess.
The timestamp conversion happens in Go: turning a timestamp into epoch
milliseconds is spelled differently by every dialect this supports, and
these row counts do not justify four versions of it.
zhangwenjian