fix(cli): a generated migration carries the field-level unique index driver-sql creates - #17208
Conversation
…the driver creates Both migration formats emitted the table and none of the object's declared uniqueness. Measured on live PostgreSQL 16.13, one object through all three producers, pg_indexes per schema: driver probe_pkey, uniq_probe_keyed_unique sql gen probe_pkey ts gen probe_pkey Two rows with the same value in a `unique: true` field were refused by the platform's table and accepted by both generated ones, with nothing reporting it. The key set was already computed here for #16091's column widths; only the index it implies was missing. The sql format emits an inline `CONSTRAINT <name> UNIQUE (...)` — what knex's `table.unique(columns, { indexName })` compiles to on PostgreSQL, so both pg_indexes and pg_constraint agree with the driver — and the ts format emits that knex call itself. Names come from a transcription of driver-sql's `buildIndexName` (#5726 forbids a static driver import from a CLI production module), pinned against the driver's own export. Two shapes stay unemitted and are now NAMED in the generated file rather than dropped: the ADR-0120 D3 organization-scoped composite, whose COALESCE key part knex's schema builder cannot express, and object-level `indexes[]`, which a second normalizer reads with different token semantics. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
…nerate-migration-emits-indexes
📓 Docs Drift CheckThis PR changes 1 package(s): 4 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 23 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin cb2fcd8d8e1d7d8ac1ee9b5d483dc66465fc6e23 && git checkout cb2fcd8d8e1d7d8ac1ee9b5d483dc66465fc6e23
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 22c94e53b652d95289e93aaa78c71e9c8436ec92 074a7b56e999f64c793e55dc86b5538616bc2d03 && git checkout -B drift-repro 22c94e53b652d95289e93aaa78c71e9c8436ec92 && git merge --no-ff 074a7b56e999f64c793e55dc86b5538616bc2d03
node scripts/docs-audit/affected-docs.mjs --json 22c94e53b652d95289e93aaa78c71e9c8436ec92
|
Fixes #16317
Clause-②: no
A generated migration emitted the table and none of the object's declared uniqueness. The key set was not missing —
generate.tsalready computes it to size the keyed text family's columns (#16091); only the index it implies was never emitted. The file said so in its own comment atindexKeyColumns: "A generated migration still emits noCREATE INDEX". That sentence is now false, and it has been rewritten rather than left standing.The acceptance, re-run — live PostgreSQL 16.13
Triage did not re-run the card's measurement, so it was re-run here from scratch: a scratch cluster (
initdb+pg_ctl, PostgreSQL 16.13), the card's own probe object driven through all three producers into three schemas,pg_indexesread back per schema.driver-sqlviainitObjectsprobe_pkey,uniq_probe_keyed_uniqueos generate migration --format sqlprobe_pkeyprobe_pkey,uniq_probe_keyed_unique--format ts(module imported,up(db)called)probe_pkeyprobe_pkey,uniq_probe_keyed_uniqueThe card's baseline reproduced exactly, and the card's consequence with it. A second row carrying the same
keyed_uniquevalue, offered to each table:② The positive control — the column width — is unmoved.
keyed_uniquereadscharacter varying(100)on all three producers both before and after, and so does every other column on the table (idvarchar(255), both audit columnstimestamp with time zone). #16298's result is not spent.One layer the acceptance did not ask for and got anyway:
pg_constraintagrees too —uniq_probe_keyed_unique/uon all three — because the sql format emits an inline table constraint rather than a followingCREATE UNIQUE INDEX, which is what knex'stable.unique(columns, { indexName })compiles to on PostgreSQL.What the change is
--format sqlemitsCONSTRAINT "NAME" UNIQUE (COLUMNS)inside the existingCREATE TABLE IF NOT EXISTS. Chosen over a followingALTER TABLE ... ADD CONSTRAINTbecause that spelling has noIF NOT EXISTS, and over a bareCREATE UNIQUE INDEXbecause the constraint form is what the driver's own knex call produces, so both catalogs agree.--format tsemitstable.unique([COLUMNS], { indexName: 'NAME' })— the driver's own line, withtablebound to the create-table builder instead of an alter-table one. TheindexNameis load-bearing: it is what makessyncDeclaredIndexesrecognise the constraint as already present on its first boot against a generated table, instead of adding a second under its own name and then reporting the generated one as an orphan to drop.buildIndexName, the__global__sentinel and the ADR-0120 D1/D3 scoping rule are transcribed, following the precedent the column widths set (objectstack dev 在工作区未构建时刷 12 段无关命令的 MODULE_NOT_FOUND,唯一可执行的那条却指向错误修法 #5726 forbids a static driver value-import from a CLI production module, and these generators are synchronous). Every transcription is pinned against the driver's own export.What it deliberately does not emit — and now says so out loud
Two shapes stay unemitted. They used to be dropped silently; each is now named in the generated file itself, with its key parts and the reason:
UNIQUE,(organization_id, field)constrains no row that has no organization — on a single-tenant stack, every row (driver-sql: 单租户栈上 organization_id 恒为 NULL,#3696 的 (tenant, col) 复合 UNIQUE 因 NULL-distinct 而完全不生效 —— 字段级 unique: true 静默零约束 #5030). That is a constraint advertised and not delivered.formulafield). The driver skips exactly this case with a warning; emitting it produces DDL that cannot run at all.Object-level
indexes[]remains unemitted by both formats. It is normalized bynormalizeDeclaredIndex, which reads the sameunique: truetoken differently (verbatim as global, a maintainer ruling), so it is a second transcription with a second pin — not a loop added to this one. The new pin asserts it is still unemitted, so whoever adds it lands there and reads why first.⭐ The step-2 measurement triage asked for: can knex express a
COALESCE(...)key part?Measured rather than assumed, knex 3.3.0 against the same live cluster:
table.unique([knex.raw("COALESCE(...)"), 'f'], { indexName })ALTER TABLE ... ADD CONSTRAINT ... UNIQUE (COALESCE(...), "f")— PostgreSQL refuses:syntax error at or near "(". A UNIQUE constraint takes no expression key part; only a unique index does.table.unique(["COALESCE(...)", 'f'], { indexName })column "COALESCE(""organization_id"", '__global__')" named in key does not existdb.raw('CREATE UNIQUE INDEX ... (COALESCE("organization_id", \'__global__\'), "f")')CREATE UNIQUE INDEX k_c ON t USING btree (COALESCE(organization_id, '__global__'::character varying), f)Conclusion, for whoever takes step 2: the two formats are not unequally capable. knex's schema builder cannot express the key part in either format — but the emitted
up(db)receives a knex handle, anddb.rawis exactly the seamSqlDriver.createNullSafeUniqueIndexalready uses for this. So the scoped form costs a raw statement in the ts format rather than anothertable.unique(...)line; it does not cost a capability. One detail for that work: the driver's index materialises the sentinel with a cast taken from the organization column's own type ('__global__'::character varyingabove), so name-and-definition convergence should be measured, not assumed.The pin, and its ablation
packages/cli/src/commands/generate-declared-unique-index.pin.test.ts(18 cases). Its authority is the real chain, not a transcribed literal: all three producers driven into one in-memory better-sqlite3 database, one table each, indexes read back out of the database's own catalog (PRAGMA index_list/index_info, primary key excluded by SQLite's ownorigin), the duplicate row offered to each table, and every expected name recomputed fromdriver-sql's exporteduniqueIndexesFromFields/buildIndexName/GLOBAL_TENANT. Underneath it sits the leaf differential over a 12-shape corpus.CONSTRAINT NAME UNIQUE (...)and materialisessqlite_autoindex_TABLE_N. The sql format's DDL is a PostgreSQL claim by #15521; what the SQLite chain carries for that format is the key parts and the enforcement, while its constraint name is asserted textually against the driver's computed one, plus on the live cluster above.⑥ Ablation — five legs, directions predicted in writing before any leg ran, all five as predicted (RED). Each leg proved the mutation reached disk (anchor occurrence count 1 to 0, plus a blob hash off the HEAD blob), rebuilt the package and ran
scripts/ablation-dist-preflight.mjsin both directions, and proved the restore clean (git diff HEADempty, blob equal to HEAD's, whole-treegit status --porcelainempty,dist/commands/generate.jsback to its pre-ablation digest), under atrap ... EXIT INT TERMwith absolute paths.table.uniqueemissionBaseline, unmutated: 18 passed / 18.
Verification
Run at
074a7b56e9(this branch's head, aftergit merge origin/main).pnpm --filter '@objectstack/cli...' --filter '@objectstack/driver-sql...' build— exit 0.pnpm --filter @objectstack/cli test— 230 files / 2983 tests passed, both tiers (this diff adds an integration-tier file, so the integration project was run locally rather than declared to CI).pnpm --filter @objectstack/cli typecheck— exit 0;tsc --listFilesconfirms the new pin file is in the program.node scripts/pm/dispatch-gates.mjs --commandsderived 60 families; all 60 run, exit code captured by redirect-then-$?(never through a pipe). Two first returned aPREREQUISITE NOT METrefusal (exit 3, not a pass) —check:dual-build-cjs-loadsandcheck:i18n-coverage, both wanting a whole-workspacedist/. Afterpnpm buildboth were re-run and are genuinely green, so 0 NOT MEASURED.--ranreconciles: 60 derived, 60 run, 0 NOT-MEASURED, 0 UNRUN.pnpm lint— the whole repo,eslint . --no-inline-config, exit 0. No narrowing, so no narrowing evidence is owed.Scope
Held to the dispatched half. ⛔ The NUMERIC arm is untouched (#16318 is
needs-user-decisionand orthogonal); ⛔ absent/unknowntypedefaulting is untouched (#16319);--format sqlstays PostgreSQL-only (#15521); nopackages/specpath, no*.zod.ts, no error-code ledger, and no new export, authorable key or declared-payload key — the ② stop condition did not fire.⑤ Single-writer re-measured from the open PR list, never from remote branches: each of the 18 open PRs diffed against its own merge base.
packages/cli/src/commands/generate.tsis held by none. Positive control from the same run, so the detector is demonstrably not blind to this subtree:packages/cli/src/commands/db/clean.tsis held by #17073 andpackages/cli/package.jsonby #17076. #16887 and #17143 are both merged and both ancestors of this branch's base (comparereportsbehind_by: 0for each), sogenerate.tswas read as they left it.Generated by Claude Code