Two comments added in this branch cite docs-prd/008-.../数据库变更.md by path.
That directory is not tracked here, so the citation reads as a file the reader
can open and cannot. The reasoning it pointed at is short enough to state in
place.
Three comments from the previous batch cite the same path and are left alone;
they belong to a different change.
Copilot could not review this branch - the account is over its review quota -
so these are what a second pass over the uninstaller turned up. No defect: the
three cases were uncovered rather than wrong.
findOrphanPolicies batches its OR chain because a driver runs out of
placeholders long before an application runs out of endpoints, and nothing
exercised the boundary. 205 paths across three batches, the last one short,
plus one policy no key names as a control. Taking one fewer per batch,
advancing one too far, and stopping after the first batch each turn it red.
An application with apis and no menus, and one with menus and no apis, are
both normal - endpoints another service calls, or a section with no endpoints
of its own - and each leaves one of the two id lists the uninstall reads
empty.
That last pair also corrected a comment. The guard in front of the join-table
delete was described as being there because an empty IN list is a syntax
error. It is in raw SQL, but GORM renders IN with an empty slice as a
condition that matches nothing, and removing the guard leaves the new test
green. It stays as a statement of intent, and now says so.
`migrate uninstall <code>` removes what an application's install wrote and
leaves the application's own tables alone. Removing an order module is not the
same decision as destroying the orders, and nothing here can tell an operator
who is done with it from one who will reinstall tomorrow.
One transaction, and this one really is one: every statement is DML or a
SELECT, so unlike an install there is no DDL to commit it out from under
itself. Child rows go first, while the ids that identify them can still be
read from their parents, and the api paths are read before the rows carrying
them are deleted.
The two join tables need no ledger and get none. menu_id is a surrogate key,
so a sys_role_menu or sys_menu_api_rule row can only have come from a menu
this application wrote - there is no "looks like it but is not". A column on
sys_role_menu would have been worse than unnecessary: SysRole.Update deletes a
role's rows and writes them back through GORM's many2many, which does not
carry extra columns, so the column would be blanked the first time anybody
edited a role, silently. There is a test that performs that edit and then
uninstalls.
casbin_rule is the opposite case, because its key is business text somebody
else may have written for their own reasons. Policies are removed one at a
time, by exact tuple, and only the ones the ledger says this install created.
A tuple the ledger names that is no longer there is reported, not treated as a
failure - the uninstall wanted it gone and it is gone. Then, with everything
the ledger could speak for already dealt with, a read-only pass lists the
policies still naming this application's paths: those are grants somebody made
by hand, they are about to point at APIs that no longer exist, and they are
not this command's to delete. The two lists stay separate because they mean
different things - one is something of ours that had already gone, the other
is somebody else's grant now pointing at nothing - and merged into one "could
not remove" list neither would be actionable.
sys_migration's rows for the application go too. Without that a reinstall
finds every version applied, runs no migration, seeds nothing, and reports
success. It is the easiest step to leave out, because a migration record does
not look like the application's data.
A sys_app row is not required. `migrate` with no subcommand applies every
registered migration, an application's included, so an application can have
all of its rows without ever having gone through the installer - and that is
the case where nothing else can clean up after it.
Eleven degradations were applied one at a time, each red on the assertion it
was aimed at: skipping either join table, deleting sys_role_menu without its
filter, skipping sys_migration, deleting sys_migration without its filter,
matching policies by path instead of by ledger tuple, dropping the orphan
pass, treating a missing policy as a failure, leaving the ledger behind, soft
deleting sys_menu instead of removing it, and running the whole thing outside
a transaction.
`migrate install <code>` brings one application up to the version its manifest
declares: it runs that application's outstanding migrations and records what
it did in sys_app.
It goes under migrate rather than under the existing `app` command, which
already means "generate the skeleton of a new application" - a directory that
does not exist yet, not an application already compiled into this binary.
Installing one is running its migrations, which is what this command is, so
--domain, resolveDB and the guard that refuses a mistyped code instead of
reporting a successful no-op are all already here.
Three phases, each committing on its own, and they are not one transaction.
An application's versions are separate migration files, and on MySQL a DDL
statement commits the transaction around it - destroying an outer transaction
and every savepoint taken from it. So this does not promise that a
half-installed application cannot happen. It promises one is visible when it
does: phase A writes "installing" before anything that can fail, phase B runs
the migrations, phase C turns that into "installed" or into "failed" with the
version it stopped on.
What is left to apply comes from sys_migration, never from sys_app. sys_app
is a derived view - a summary, and the answer to "which version does this app
think it is at". If it were the authority, an operator who deleted
sys_migration rows by hand would be told an application is installed while its
schema is not, which is worse than not knowing. So "already installed, nothing
to do" needs all three: nothing outstanding, recorded as installed, and the
same version. A row stuck at "installing" - what it reads as after the process
was killed partway - is not installed, and retrying is just running the
command again.
An upgrade is in place and keeps the first install's time; a downgrade is
refused, and refused before phase A writes anything, so a refusal cannot cost
the operator the row that told them what they had. An unparseable recorded
version is refused the same way, while it is still readable.
The report ends by saying the code is not running yet. That is not a
pleasantry: Go links at build time and Vite resolves its import globs at build
time, so installing an application writes its menus, its APIs and its
permissions and cannot make one line of its code run - and the menus appearing
is exactly what makes an operator believe otherwise.
Ten degradations were applied one at a time to check the tests name the
behaviour rather than the shape: deciding the no-op from sys_app alone,
always writing installed_at, allowing the downgrade, keeping the previous
attempt's diagnostics on a row that now says installed, treating "installing"
as installed, truncating last_error by bytes so a Chinese message is cut
mid-rune, not recording the failure at all, skipping code normalization, and
writing phase A before either the downgrade or the version-parse refusal.
Each went red on the assertion it was aimed at. An eleventh was discarded
rather than counted: it failed in the first install's setup, not on the claim.
run() called log.Fatalf on the first migration that failed, which ended the
process from inside the migration engine. Nothing above it could record what
happened - an installer needs to write down which version an attempt stopped
on - and no test could exercise a failing migration at all without taking the
test binary with it, which is why the one test that covers a failed migration
drove the registered function directly and left the scheduler uncovered.
run(), Migrate() and MigrateApp() now return an error, and the exit moved to
the command layer where the exit code is the command's business.
Two of those errors say more than "it failed". A migration that fails comes
back as a *VersionFailure naming the version, because an installer records
that as a diagnostic snapshot - the authoritative answer to where a retry
resumes is always recomputed from sys_migration, never read back, and asking
the database what is still pending answers a different question that merely
has the same answer most of the time. An app code nothing registered under is
now an error rather than a log line, so an installer asking for one app by
name cannot be told that installing an app that does not exist succeeded; the
command layer still rejects a typo before any database work.
exitOnError is what makes the command exit non-zero, and it covers more than
it replaces. Every path out of migrateModel used to return without an exit
code: an unreachable tenant database or a failed AutoMigrate printed a line
and exited 0, so a caller that migrates before starting a server - the deploy
workflow does exactly that - carried on onto a schema that had not been
brought forward. A failing migration function was the only failure reported,
and only as a side effect of the log.Fatalf this commit removes.
Each of these was checked by degrading it and watching the named assertion
go red: returning nil instead of the failure, naming the first version rather
than the one that failed, accepting an unregistered app code, and not exiting.
One gap is left open deliberately. Go allows a call whose only result is an
error to stand as a statement, so `migration.Migrate.Migrate()` still compiles
while dropping what it returns - `go build` passed while migrateModel was
doing exactly that during this change. Both call sites now return the value,
which the compiler does check, but nothing guards against the statement form
coming back. A checksilent rule was considered and dropped: that tool parses
without type information, so it could only match the method name, and a guard
that fires on any type with a Migrate method is noise.
The check grouped by (app_code, path, action) and refused whatever
appeared more than once. GROUP BY treats two NULLs as the same value; a
unique index treats them as different ones and allows both. So a
database holding rows with a null path or action was refused for
duplicates the index it was blocking would have accepted - and the
migration stopped, on a database with nothing wrong with it.
Both columns are nullable: neither carries a not-null tag, so gorm built
them that way. Measured on MySQL 8.0, PostgreSQL 15 and SQLite: two rows
with both columns null are one group to GROUP BY, and the unique index
builds over them without complaint. Standard SQL, not a dialect quirk.
They are now excluded from the check rather than grouped. Each column
needs its own exclusion and has its own test: one null column is enough
to make the index accept the pair, so removing either condition alone
lets that half through - which is what the two subtests are for, and
each fails only for its own half.
The message could not name the rows either. MySQL's CONCAT returns NULL
when any argument is, and scanning that into a string fails with
"converting NULL to string is unsupported" - so the check reported a
driver error instead of the duplicates it exists to report. SQLite and
PostgreSQL treat a null argument as empty and say nothing, which is why
this never surfaced in the tests: they run on SQLite, and this
repository has no MySQL in CI. The comment says so, so that a
postgres-only test file is not mistaken for cover.
No COALESCE was added to paper over that. It would have had nothing left
to guard once the nulls are excluded, and it would make a future
regression quieter: someone dropping the exclusions would get a report
naming rows that are not duplicates, which reads as a real answer,
rather than a scan error that reads as a broken query.
Leaving path and action nullable is deliberate. Tightening them is a
migration of its own - existing null rows have to be given values, and
what those values should be belongs to whoever owns the data, not to a
migration whose job is adding an index.
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.
Status answers what is registered, what is applied, and what is applied
while nothing registers it - and needs a database to do it. A readiness
check needs only the first half, and it already holds the databases it
is asking about.
Without this it would have to call SetDb to reuse Status, writing this
package's shared state from a request path, for a question that does not
depend on any database at all.
The host kept its own copy of the version-naming rule, byte-identical to
the one in contract/migration: slice the leading 13 characters, no check.
Two copies of a convention that applications also have to follow is two
things to keep in step, and the copies had already stopped matching - core
now rejects a name that carries no timestamp, and this one still accepted
"add_orders.go" and registered a migration under that string as its
version, which nothing would ever match and nothing would report.
Delegate instead, so there is one implementation of the rule and an app's
migration and a host migration derive their version the same way.
The test pins the reject case, not just the happy path: a re-divergence
that only sliced would still pass the happy path.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
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
core's sdk/contract/migration keeps its own process-wide registry, because
that is the only door open to an application that must not import the host.
Nothing here ever opened it: ForApp("crm").SetVersion(...) compiled,
registered, and then never ran - no error, no mention in status, nothing.
mergedEntries unions the host's own registry with contract/migration's
Snapshot(), and status, run and AppCodes all read through it, so migrate,
status, --dry-run and --app see an application's migrations exactly as they
see the host's. Version namespacing already keeps the two apart, so a key
collision should not be reachable; the host's own registration wins if one
ever is, rather than being silently replaced.
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
--app rejects a code nothing was registered under, on all three paths. It used
to take a typo as "nothing matched" and report success: migrate said the app
was unknown and still exited 0, while --dry-run and status printed the same
words an up-to-date database produces.
The hazard had no signal at its point of contact. Someone adding a business
module is told to copy 1786700001000_demo_menu.go, which imports the frozen
seed models - correct for that file, wrong for anything ordered after the
soft-delete conversion. The frozen ModelTime carried no comment at all, so
opening it taught the reader nothing.
Documents the boundary in three places the author actually passes through:
the frozen type itself, the contributor guide, and the module-scaffolding
skill, which previously said to copy the reference file verbatim and now
says to copy its structure but not its imports.
Claude-Session: https://claude.ai/code/session_01DJhM6LvhkNPej35wy9F7Aq
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
An applied migration printed its count - a bare '1' - so a database with
seven of them wrote seven lines of '1' at every start, and a failure said
only which error, never which migration.
It now names each one as it applies, reports the total, and says so when
there is nothing to do.
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.
The pinned core dated from April, before sdk stopped being a separate module,
so the build resolved sdk packages from the old module and core packages from
the new one. Dropping the separate requirement is what makes the two agree
again.
Most of the diff is renames that came with that: the tenant accessors gained a
ByTenant suffix, GetDb now returns one database and GetAllDb the map, and
casbin moved to v3.
The change that matters is four call sites moving from GetMemoryQueue to
GetQueuePrefix. GetMemoryQueue returns a queue fixed at construction, so the
login log, the operate log and the api check ran in process no matter what the
settings file selected — a second instance saw none of it. GetQueuePrefix
returns whatever the configuration built, which is the point of being able to
configure a queue at all.
Verified against core at main: build and vet clean. The two file_store failures
are unchanged from before this branch; they need cloud credentials.
zhangwenjian