Repository navigation
Commit 4663801
fix(objectql): a tree field's deleteBehavior takes effect on a parent delete (#23055)
Fixes #22957
Clause-②: no
The engine's referential delete walk now follows `tree` fields. The
`deleteBehavior` the spec declares on a `tree` (materialized as
`set_null` when the author writes none) takes effect on a by-id delete
of the parent. This follows triage's ruling (comment 6119365636): the
runtime honours the declared behaviour, and the spec's materialization
stays as it is.
## What changed
- `packages/objectql/src/engine.ts`: one family predicate,
`statesDeleteSemantics` over `DELETE_SEMANTICS_REFERENCE_TYPES`
(`master_detail`, `lookup`, `tree`), replaces the `master_detail` /
`lookup` literal at both readers of the delete walk that tested the
family. A `tree` resolves like a `lookup`: an authored value is used as
written, and an absent one resolves to `set_null`.
- `packages/objectql/src/engine-cascade-delete-tree.test.ts`: 18 pins,
listed below.
- `.changeset/22957-tree-delete-behavior.md`: `patch` for
`@objectstack/objectql`.
## Readers of the walk (read on `origin/main` `9f4b652e3a`)
| reader | family test before | now |
|---|---|---|
| `cascadeRelationBehavior` | `master_detail` / `lookup` literal | the
predicate |
| `collectCascadeDeleteSet` (phase 1) | reads `cascadeRelationBehavior`
| code unchanged, widened through it |
| `walkReferencingRelations` (phase 2, the dependents walk) | reads
`cascadeRelationBehavior` | code unchanged, widened through it |
| `planCascadeAtomicity` and its `referenceCarrierOf` probe |
`master_detail` / `lookup` literal | the predicate, so a subtree cascade
is one unit of work |
| the #9625 resolved-behaviour reads: the `master_detail` coercion to
`cascade`, the `master_detail` + `set_null` warning, and the required
`set_null` escalation to `restrict` | read the resolved behaviour |
unchanged. A `tree` takes the `lookup` branch, so a required tree's
`set_null` escalates like a required lookup's (pinned) |
| the `referenceCarrierOf` call in `buildSummaryIndex` | `master_detail`
/ `lookup` literal | **left as is.** It resolves a roll-up summary's
foreign key and is not part of the delete walk |
## Measured before and after
### `os verify --app examples/app-showcase/objectstack.config.ts
--delete-behaviors --json`
Before: `9f4b652e3a`. After: objectql built from `2030d882ac`.
`engine.ts` is byte-identical from there through this head,
`85abb2ed96`.
| | before | after |
|---|---|---|
| exit | 1 | 0 |
| `hardFailures` | 1 | 0 |
| `showcase_field_zoo.f_tree` (declared `set_null`) | contradicted. REST
answered 200 and the system door succeeded, and both left the holder on
the deleted id | proven. Both doors cleared it (`holderValue: null`) |
| the other 23 rows | 8 proven, 1 inconclusive
(`showcase_invoice.contact`, refused by an app validation rule), 11
excluded, 3 not-derivable | identical, row by row |
### Real driver: ObjectQL + `SqlDriver` (better-sqlite3), a scratch
probe (not committed)
| case | before | after |
|---|---|---|
| tree, `set_null` materialized by `FieldSchema.parse` | child keeps the
deleted id | child's parent is `null` |
| tree registered raw, no key | child keeps the deleted id | child's
parent is `null` |
| tree `cascade`: delete `a` from root, a, b, c (a chain) plus a2 and an
unrelated pair | only `a` deleted | `a`, `b`, `c` deleted; root, a2 and
the pair kept |
| tree `restrict` | parent deleted, child left dangling |
`DELETE_RESTRICTED` 409 naming the tree's own object; both rows kept;
the leaf then deletes |
| tree `cascade`, a 15-level chain | 1 of 15 deleted | 15 of 15 deleted
|
| tree `cascade`, a corrupt cycle (root's parent is its grandchild) and
a row that is its own parent | only the deleted row went | terminates (7
ms); every row of the cycle deleted once |
| tree registered raw with `multiple: true` | child keeps the deleted id
| child's parent is `null` (read single-valued) |
| tree `cascade` with a `beforeDelete` veto on a deep node | root
deleted, children kept (nothing walked) | refused 409; all four rows
kept (rolled back) |
| CONTROL: `lookup` default `set_null`, `lookup` `cascade`,
`master_detail` default | cleared / deleted / deleted | identical |
| CONTROL: `lookup` and `master_detail` `restrict` | both
`DELETE_RESTRICTED` 409 | identical |
### The four questions the dispatch asked
- **`multiple: true`.** `FieldSchema` refuses `multiple: true` on a
`tree` at parse: it is declarable only on select, lookup, user, file and
image. `lookup` has a multi-valued path (the `$contains` probe and
member removal). A `tree` cannot reach that path through the contract.
The walk's multi-value arbiter `declaredMultiValued` asks the spec's
`isMultiValueField`, which reads a raw-registered multi `tree` as
single-valued, as every other engine site does. Nothing new was added
for it.
- **The default.** The spec's parse puts `set_null` on a bare `tree`,
and the test pins that as its premise. The engine reads the field raw,
and an absent key also resolves to `set_null`. Both are pinned.
- **Depth and cycles.** `MAX_CASCADE_DEPTH` (10) bounds only
`planCascadeAtomicity`'s schema walk. `cascadeDeleteRelations` has a
`depth` parameter, but the nested deletes never advance it (the one call
site passes the default), so the cascade over rows is bounded by its
set. `collectCascadeDeleteSet` expands each record once, and the
deleting walk skips a record it has already entered. Pinned: a 15-level
chain is deleted whole, and a corrupt cycle and a self-parent both
terminate.
- **Atomicity.** An object whose only relation is its own hierarchy used
to plan `'none'` and opened no transaction. It now plans `'atomic'`, so
a refusal deep in a subtree rolls back the nodes deleted before it.
## Pins (`engine-cascade-delete-tree.test.ts`, in-file driver with
snapshot rollback)
- set_null: the parse premise; then materialized, authored, and absent:
the child's parent is cleared, the child is kept, and a bystander is
untouched (3 cases).
- cascade: a subtree with a single commit; 15 levels deleted whole; a
corrupt cycle terminates; a self-parent terminates; a deep veto rolls
everything back.
- restrict: the parent delete is refused with `code` + `status` +
`dependentObject`, and both rows are kept; a leaf deletes.
- required: a required tree's `set_null` escalates to
`DELETE_RESTRICTED`.
- multiple: the parse refuses `multiple: true` on a tree; a raw
registration is read single-valued.
- CONTROLS: lookup default and cascade plus the master_detail default;
lookup and master_detail `restrict` (2 cases); an object nothing
references opens no transaction.
## Ablation (from the committed head `5ce34e0670`, through
`scripts/ablation-replace.mjs`)
Each leg's anchor hit 1 to 0 and the blob changed. After each leg the
restore read blob == HEAD and `git diff HEAD` was empty.
- **Leg A**: `tree` dropped from `DELETE_SEMANTICS_REFERENCE_TYPES` gave
11 failed and 7 passed. Red: all 11 tree-behaviour pins. Green: the 2
parse premises, the restrict leaf delete, and the 4 CONTROL tests.
- **Leg B**: only `planCascadeAtomicity`'s reader put back to the old
literal gave 2 failed and 16 passed: the subtree's single-commit
assertion and the deep-veto rollback. So the atomicity widening is
pinned on its own.
## Tests
- `pnpm --filter @objectstack/objectql exec vitest run --maxWorkers=2
src/engine-cascade-delete-tree.test.ts`: 18 passed, at `5ce34e0670`.
- The full package suite (`vitest run --project local --maxWorkers=2`)
ran at `2030d882ac` plus the then-uncommitted required pin: 406 files
and 7953 tests passed, 1 failed. The failure was the required pin as
first written: its fixture referenced a row that did not exist, and the
engine's write-time reference check refused it. The fixture was
corrected and passes in the single-file run above.
- `pnpm --filter @objectstack/objectql typecheck`: exit 0, covering
`tsc` and `check:test-typecheck`. The new file is in
`tsconfig.test.json`'s program (`--listFiles` count 1).
- `@objectstack/objectql`'s public surface is unchanged (no export, no
type), so no importing package owes a test.
## Gates
`dispatch-gates --commands` on this head `85abb2ed96` derives 68
commands: the dispatched 57 plus 11 that the changeset brings in. All 68
exit 0. The `--ran` reconciliation reads 68 derived, 68 run, 0
NOT-MEASURED, and that zero is derived from the recorded exit codes.
`check:dual-build-cjs-loads` first answered PREREQUISITE NOT MET because
8 packages outside this diff had no `dist/`. They were built (all turbo
cache hits), and the gate re-ran with exit 0.
## Deviations
- The pins use an in-file driver with snapshot rollback, the same shape
as the five sibling `engine-cascade-delete-*` suites, not a real driver.
`packages/objectql` lists no driver package among its dependencies, and
adding one (to `package.json`, `pnpm-lock.yaml` and the
test-source-alias ledger) is outside the claim's file surface. The
real-driver evidence is the showcase verify (sqlite-wasm, through REST
and the system door) and the scratch `SqlDriver` probe above.
- `origin/main` gained two docs-only commits (ADR text and anchor JSON)
after the base. They were merged into this branch as `85abb2ed96`; no
package changed.
## Acceptance notes
- A `tree` that omits `reference` is still not walked. `ObjectSchema`
accepts it as a self-reference (the `refuseForeignTreeReference` rule:
the reference stays optional), and the parse materializes `set_null` on
it. But `referenceCarrierOf` reads no carrier, and `referenceTargetOf`
calls a referenceless `tree` targetless. Measured on this head through
REST: `DELETE` answered 200 and the child kept the deleted id. Repairing
it is a spec decision: which of the two readings is the contract, and
whether a referenceless tree's target should resolve to its owning
object. That is not a consumer-side fallback to add here. Reported to
the seat.
- Two comments in `packages/spec/src/data/field.zod.ts` (the
`deleteBehavior` TSDoc and the `.overwrite` block) still say that the
engine reads the key only behind a `master_detail` / `lookup` guard.
That now includes `tree`. These are comments only. Carrier: none. Noted,
not filed.
- An authored `deleteBehavior` on a `user` field still parses and is
still not walked. That is the spec's existing line for `user`, and this
PR leaves it untouched.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01ADzJtzYTLUfgrRZHxkagkX)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent e448d3c commit 4663801
3 files changed
Lines changed: 484 additions & 11 deletions
File tree
- .changeset
- packages/objectql/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 | + | |
0 commit comments