Repository navigation
Commit 8d7f906
fix(driver-sql): schema sync never drops an existing table that holds rows (#30632)
<!-- claude-projects-attribution: {"github_login":"os-bill"} -->
_Requested by **Bill** · [project
thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wgG3GBsgZPwY1z2JaG3ucvGu)_
domain:server#2 · os-bill · session_01Utp6CYE7swB6ksdveFrzwH
Fixes #30562
Clause-②: yes (narrowing) — `SqlDriver.initObjects` / `syncSchema`
refuse one case that used to succeed: an existing table with a `_id`
column, no `id` column and at least one row. It used to be dropped and
recreated empty; it is now refused (`ExistingTableAdoptionRefusal`,
naming the table, the column and the row count) and left exactly as it
was.
Metadata-protocol: none
Skills-docs: none — `git grep -n -i -E "drop(s|ped)? and recreat|legacy
_id|_id column|existing table|ExistingTableAdoption" -- 'skills/**'` has
no hit (exit 1); the `schemaMode` table in
`skills/objectstack-data/rules/datasources.md` stays true.
## 维护者速读
- 改了什么:在托管 SQL 数据源上保存对象时,如果同名的已有表用 `_id` 当主键、没有 `id`
列,而且表里有数据,结构同步不再删表重建,改为拒绝:表、列、行数原样不动,拒绝信息点名表名和 `_id` 列,并给出三条出路。空表照旧重建,有
`id` 的表照旧补列收编。
- 为什么改:实测这条路会把客户已有表的数据全部删掉(2 行变 0 行),保存还答成功;新建数据源对话框在 PR #30561
之前默认就是托管,所以这是默认路径上的静默丢数据(p1)。
- 风险与回滚:只动 driver-sql 的一个分支加一个新的拒绝类;平台自己没有任何 `_id` 键的表,不影响升级。注意:保存接口仍答
200,拒绝只写进服务器日志(error 级,#22985 的既有做法),数据不再丢失;让保存把拒绝报给调用方另立卡。回滚即撤本 PR。
- 先看:`packages/drivers/driver-sql/src/sql-driver-rotation.ts` 中
`initObjects` 的 `_id`
分支(先数行,有行就抛);`existing-table-refusal.ts`(拒绝类与文案);`sql-driver-schema.test.ts`
末尾新增的 describe。
- 两分钟验证:`pnpm --filter @objectstack/driver-sql exec vitest run
--maxWorkers=2 src/sql-driver-schema.test.ts`,预期全绿,`schema sync never
drops a table holding rows — sqlite` 下 3 条通过(PG/MySQL 无服务器时显示未跑)。
- 需要你做的:无,席位落地。
## What changes
**The rule: schema sync never drops a table that holds rows.**
- **`initObjects`
(`packages/drivers/driver-sql/src/sql-driver-rotation.ts`).** Before the
legacy-`_id` rebuild (an existing table with `_id` and no `id`: drop,
then create as the object declares), the table's rows are counted. One
row or more: the sync throws `ExistingTableAdoptionRefusal` before any
DDL, so no column is added, no index is built and no row is touched.
Zero rows: the empty table is rebuilt exactly as before. A table with
`id` is adopted as before (missing declared columns added, rows kept).
The table is still recorded as found-existing for `getSchemaSyncStats`,
since it was there when the sync arrived.
- **`ExistingTableAdoptionRefusal` (new,
`packages/drivers/driver-sql/src/existing-table-refusal.ts`).** An
`Error` with `name`, `table`, `column` and `rows`. Its message names the
table, the column and the row count, says nothing was changed, and gives
the three ways forward: rename `_id` to `id` and save again; bind the
object through an `external` datasource to read the table as it is; or
empty the table and save again. English, like its neighbours. Not
exported from `index.ts`.
- **No ADR-0112 `code`.** Measured below: the metadata save door answers
`200` for any refused schema sync and logs it at `error`
(`ensureObjectStorage`, the deliberate #22985 design), as do the boot
sync and a datasource connect. No door serves a code for a schema-sync
refusal, and a code stamped by `driver-sql` needs a ledger row in
`packages/spec` (out of this card's file surface). The pins assert the
class, its named fields and that the message names the table and the
column.
**The other drop sites in driver-sql, read and left alone.** None drops
a table holding rows that the platform did not create:
- `sql-driver-schema-sync.ts` `rebuildSqliteTablePatched`:
`dropTableIfExists('__os_mig_TABLE')` is its own scratch table;
`dropTable(table)` runs inside the same transaction after `INSERT …
SELECT` copied every kept column into the scratch table, which is then
renamed back (copy → swap, rows kept).
- `ensureRotation`: the base table is merged into the current shard
(`INSERT … SELECT *`) before it is dropped; aged-out shards
(`TABLE__rYYYYMMDD`) are dropped by the object's own declared
`lifecycle.storage` retention.
- `sql-driver-sequence-semantics.ts`: `_objectstack_sequences` (the
platform's counter table) is copied into `__rebuild` before the swap;
the scratch table is dropped on failure.
- `SqlDriver.dropTable` is the explicit drop a caller asks for (object
delete with `dropStorage`), not schema sync.
**Where the `_id` branch came from.** It arrived in `cdd1bd15c7`
(2026-03-22), the commit that brought `driver-sql` over from the
ObjectQL driver, unchanged; that driver keyed rows by `_id`. Nothing in
this repository creates a `_id`-keyed SQL table today (`git grep` for a
quoted `_id` across `packages/drivers/*/src` non-test source: the branch
itself is the only hit outside `driver-mongodb`), so refusing a
populated one breaks no shipped upgrade path; the title carries no `!`.
## Why
#30562: storing an object on a managed SQL datasource over an existing
database deleted the rows of a same-named table keyed by `_id`, and the
save answered success. The New datasource dialog defaulted to managed
until PR #30561, so this was the default path.
## Tests
New top-level describe in `sql-driver-schema.test.ts` (the package's
shared schema-sync harness), declared across the dialect matrix
(`DIALECT_CELLS` / `declareDialectCell`): SQLite always; Postgres and
MySQL where the runner provisions them. Driven through `syncSchema`, the
call the engine makes per object.
- `refuses a _id-keyed table holding rows, naming the table and the
column, and changes nothing` — 2 rows; the refusal is an
`ExistingTableAdoptionRefusal` with `table`, `column: '_id'`, `rows: 2`,
its message names both; the table's column set and every row are
byte-for-byte the snapshot taken before.
- `CONTROL: rebuilds an empty _id-keyed table as the object declares, as
before`.
- `CONTROL: adopts an id-keyed table holding rows by adding the declared
columns, rows kept`.
## Acceptance notes
- **Data no longer lost; the save still answers 200, and the refusal
goes only to the server log.** Measured live (below): after this change
`PUT /api/v1/meta/object/legacy_items` answers `200 {"success":true,…}`,
the definition is stored and bound to the managed datasource, the table
and its 2 rows are untouched, and the server logs the #22985 `error`
line followed by this refusal. Reporting a refused schema sync to the
caller of the save is the opposite of #22985's deliberate "the publish
is NOT failed", in `metadata-protocol`, outside this card; the seat
files it as its own card.
- **After the refusal the object is served on the unchanged table, as
for any refused sync.** Measured live: `GET /api/v1/data/legacy_items`
answers `200` with the 2 rows and `id: null` (the table has no `id`);
`POST` is refused `400 INVALID_FIELD` (`__search` has no column) and
writes nothing. That is the documented state after any refused sync
(#22985's line says so), not new here.
- **`os migrate plan` does not foresee this refusal.** Its additive
preview (`sql-driver-schema-sync.ts`, the `add_columns` branch) lists
the declared columns a `_id`-keyed table lacks, while `apply` refuses it
when it holds rows (and, before and after this change, rebuilds it when
empty). Noted from reading, not measured through the CLI; carrier: seat.
- **MySQL not run locally.** The pins ran on SQLite and on a local
PostgreSQL 16 (all green); the MySQL cell runs in CI's `Temporal
Conformance (live PG + MySQL)` job, which carries the full battery on
`main` (push and schedule), not on this PR.
## Verification
Code commit `8157270e`; head `9a71d244` merges `origin/main` at
`1c5cee67` (7 commits, none in `driver-sql`). Measured first at base
`bdbaf80c`.
<details><summary>Live: PUT /api/v1/meta/object/NAME on a managed SQLite
datasource over an existing file (examples/app-showcase, objectstack dev
--fresh)</summary>
Fixture: a SQLite file with `parts (part_no, part_name, qty)`, 2 rows,
and `legacy_items (_id, title)`, 2 rows. `POST /api/v1/datasources`
registers it as a runtime datasource with `schemaMode: 'managed'`
(`201`); then `PUT /api/v1/meta/object/parts` and `PUT
/api/v1/meta/object/legacy_items`, each with `datasource` set to it and
one declared field `title`.
| | `parts` (has no `_id`) | `legacy_items` (`_id`, no `id`) |
|:--|:--|:--|
| base `bdbaf80c` | `200` success · 3 to 13 columns · 2 rows kept |
`200` success · **dropped and recreated**: 11 columns, `_id` gone, **0
rows** |
| this PR | `200` success · 3 to 13 columns · 2 rows kept (unchanged) |
`200` success · **table unchanged**: 2 columns, **2 rows** |
After this PR the server log carries, for `legacy_items`, the existing
#22985 line (`[Protocol] object 'legacy_items' was published but its
storage was NOT created or altered — its schema sync was refused. The
publish answered success …`) followed by this refusal: `Schema sync
REFUSED for the existing table 'legacy_items': it has a '_id' column and
no 'id' column, and it holds 2 rows. …`. `GET
/api/v1/meta/object/legacy_items` answers `200` with `datasource` set to
the managed datasource (the definition is stored and bound). `GET
/api/v1/data/legacy_items` answers `200` with the 2 rows (`id: null`);
`POST /api/v1/data/legacy_items` answers `400 INVALID_FIELD` and the
table still holds 2 rows.
</details>
<details><summary>Tests</summary>
- `pnpm --filter @objectstack/driver-sql exec vitest run --maxWorkers=2`
with a local PostgreSQL 16 (`OS_TEST_POSTGRES_URL`, server zone
Asia/Shanghai, `TZ=America/New_York`): Test Files 247 passed, 3 skipped
(250); Tests 4885 passed, 110 skipped (4995); coverage reporter: sqlite
RAN, live postgres RAN, live mysql NOT RUN. Lock VERDICT command-exit 0.
On `8157270e`.
- `src/sql-driver-schema.test.ts` alone, same PostgreSQL: 26 passed, 1
skipped (the MySQL cell).
- `pnpm --filter @objectstack/driver-sql typecheck` (`tsc --noEmit`):
exit 0; `--listFiles` includes both changed source files and the test
file.
- Drivers that inherit `SqlDriver`: `@objectstack/driver-sqlite-wasm` 40
files, 750 tests passed; `@objectstack/driver-turso` 97 files, 2514
passed, 34 skipped. `@objectstack/service-datasource` (drives managed
datasources): 72 files, 1195 tests passed.
- eslint (`--no-inline-config`) on the three changed source and test
files: exit 0.
</details>
<details><summary>Ablations (scripts/ablation-replace.mjs; the suite
imports src directly, no build between legs)</summary>
| # | Mutation in `initObjects` | Result (SQLite + PostgreSQL cells) |
|:--|:--|:--|
| A1 | delete the row guard (`if (Number(rows) > 0) throw …`) | the
refusal pin red in both cells: `expected null to be an instance of
ExistingTableAdoptionRefusal` (2 failed, 24 passed) |
| A2 | guard threshold `> 0` to `>= 0` (an empty table refused too) |
the empty-table rebuild control red in both cells (2 failed, 24 passed)
|
Each mutation landed (anchor 1 to 0, blob changed); each restore left
the blob equal to HEAD and `git diff HEAD` empty.
</details>
<details><summary>Gates and dogfood, on head 9a71d24</summary>
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack`: 59 derived families (change set: the 4
paths of this PR vs merge base `1c5cee67`). All 59 run, exit code
captured per line before any pipe: 59 exit 0. `--ran`: 59 derived, 59
run, 0 NOT-MEASURED (a derived zero).
- `pnpm --filter @objectstack/spec check:generated` after the merge: all
14 generated artifacts up to date.
- Full dogfood under the verify lock, after a full `turbo run build` of
the merged tree (118 tasks successful): `pnpm --filter
@objectstack/dogfood test` VERDICT command-exit 0, Test Files 420
passed, 3 skipped (423), Tests 3496 passed, 36 skipped (3532).
- `pnpm lint` over the whole repository was not run locally (a
repository-wide scan, CI's `Lint & Repo Gates`); eslint ran on the
changed files.
</details>
---
_Generated by [Claude
Code](https://claude.ai/code/session_01Utp6CYE7swB6ksdveFrzwH)_
Co-authored-by: Claude <noreply@anthropic.com>1 parent b77c160 commit 8d7f906
4 files changed
Lines changed: 211 additions & 1 deletion
File tree
- .changeset
- packages/drivers/driver-sql/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
36 | 36 | | |
37 | 37 | | |
38 | 38 | | |
| 39 | + | |
39 | 40 | | |
40 | 41 | | |
41 | 42 | | |
| |||
458 | 459 | | |
459 | 460 | | |
460 | 461 | | |
461 | | - | |
| 462 | + | |
462 | 463 | | |
463 | 464 | | |
464 | 465 | | |
| |||
469 | 470 | | |
470 | 471 | | |
471 | 472 | | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
| 478 | + | |
| 479 | + | |
| 480 | + | |
472 | 481 | | |
| 482 | + | |
| 483 | + | |
473 | 484 | | |
474 | 485 | | |
475 | 486 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
| 5 | + | |
| 6 | + | |
5 | 7 | | |
6 | 8 | | |
7 | 9 | | |
| |||
494 | 496 | | |
495 | 497 | | |
496 | 498 | | |
| 499 | + | |
| 500 | + | |
| 501 | + | |
| 502 | + | |
| 503 | + | |
| 504 | + | |
| 505 | + | |
| 506 | + | |
| 507 | + | |
| 508 | + | |
| 509 | + | |
| 510 | + | |
| 511 | + | |
| 512 | + | |
| 513 | + | |
| 514 | + | |
| 515 | + | |
| 516 | + | |
| 517 | + | |
| 518 | + | |
| 519 | + | |
| 520 | + | |
| 521 | + | |
| 522 | + | |
| 523 | + | |
| 524 | + | |
| 525 | + | |
| 526 | + | |
| 527 | + | |
| 528 | + | |
| 529 | + | |
| 530 | + | |
| 531 | + | |
| 532 | + | |
| 533 | + | |
| 534 | + | |
| 535 | + | |
| 536 | + | |
| 537 | + | |
| 538 | + | |
| 539 | + | |
| 540 | + | |
| 541 | + | |
| 542 | + | |
| 543 | + | |
| 544 | + | |
| 545 | + | |
| 546 | + | |
| 547 | + | |
| 548 | + | |
| 549 | + | |
| 550 | + | |
| 551 | + | |
| 552 | + | |
| 553 | + | |
| 554 | + | |
| 555 | + | |
| 556 | + | |
| 557 | + | |
| 558 | + | |
| 559 | + | |
| 560 | + | |
| 561 | + | |
| 562 | + | |
| 563 | + | |
| 564 | + | |
| 565 | + | |
| 566 | + | |
| 567 | + | |
| 568 | + | |
| 569 | + | |
| 570 | + | |
| 571 | + | |
| 572 | + | |
| 573 | + | |
| 574 | + | |
| 575 | + | |
| 576 | + | |
| 577 | + | |
| 578 | + | |
| 579 | + | |
| 580 | + | |
| 581 | + | |
| 582 | + | |
| 583 | + | |
| 584 | + | |
| 585 | + | |
| 586 | + | |
| 587 | + | |
| 588 | + | |
| 589 | + | |
| 590 | + | |
| 591 | + | |
| 592 | + | |
| 593 | + | |
| 594 | + | |
| 595 | + | |
| 596 | + | |
| 597 | + | |
| 598 | + | |
| 599 | + | |
| 600 | + | |
| 601 | + | |
| 602 | + | |
| 603 | + | |
| 604 | + | |
| 605 | + | |
| 606 | + | |
| 607 | + | |
| 608 | + | |
| 609 | + | |
| 610 | + | |
| 611 | + | |
| 612 | + | |
| 613 | + | |
| 614 | + | |
| 615 | + | |
| 616 | + | |
| 617 | + | |
| 618 | + | |
0 commit comments