Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .changeset/nine-parents-keep-details.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
---
'@objectstack/spec': minor
'@objectstack/objectql': minor
---

`FieldSchema` now rejects an authored `deleteBehavior: 'set_null'` on a `master_detail` field at parse time (#9689). The engine has always resolved every value except `restrict` on that type to `cascade`, so the declaration asked for the child rows to be kept and got them deleted — silently, at the moment the parent went away. The rejection names the outcome and both legal re-declarations (`restrict` refuses the parent delete while children exist — no data loss; `cascade`, or omitting the key, accepts the cascade deliberately; a `lookup` is the type to use when children must survive the parent).

Mechanism (the #7918 Option A shape): the property-level `.default('set_null')` moved off `deleteBehavior` into a post-check `.overwrite()`, so the schema can tell an authored `set_null` from a defaulted one. Parse output is byte-identical to before on every accepted input — a bare `master_detail` still materializes `deleteBehavior: 'set_null'` at its shape position, non-reference types still carry the default, and `set_null` on `lookup` stays legal. The inferred `Field` output type now declares `deleteBehavior` as optional (the same accepted cost as the currency `precision` relocation); a parsed field always carries it at runtime.

There is deliberately no automatic conversion (`field-master-detail-set-null-refused` in the migration registry): only the author knows whether they meant `restrict` (keep-my-children, as a refusal) or `cascade`. Stored rows carrying the refused combination keep loading and serving — registry validation is a diagnostic, not a gate — and are refused on their next authoring-path save.

`@objectstack/objectql`: the engine behavior is unchanged (an authored `set_null` on `master_detail` still cascades — the #9625 pin holds), but the coercion site now logs loudly (`error`, falling back to `warn`) when the combination reaches it via a raw registration or a pre-tightening stored row — the two populations parse-time rejection cannot catch.
2 changes: 1 addition & 1 deletion content/docs/data-modeling/field-types.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -337,7 +337,7 @@ Parent-child relationship (cascading delete by default).
|:---|:---|:---|:---|
| `reference` | `string` | **required** | Target (master) object name |
| `referenceFilters` | `string[]` | — | **Removed** (#2377, ADR-0049) — no longer a recognized field property. `FieldSchema` is a strict object, so an unknown key is **rejected with guidance**, not silently stripped (ADR-0078): the error echoes the offending key and prescribes the replacement. Use structured `lookupFilters` + `dependsOn` instead; see [Relationships](/docs/data-modeling/relationships) |
| `deleteBehavior` | `'restrict' \| 'cascade' \| 'set_null'` | `'cascade'` | Behavior when parent is deleted. `restrict` is the only value that deviates: master-detail cascades on everything else, so an explicit `set_null` here is **not** honored — the child is deleted with the parent |
| `deleteBehavior` | `'restrict' \| 'cascade'` | `'cascade'` | Behavior when parent is deleted. `restrict` refuses the parent delete while children exist; `cascade` (and omitting the key) deletes the children with the parent. An explicit `set_null` is a **parse-time rejection** on this type (#9689) — a detail row cannot outlive its master; use a `lookup` if children must survive the parent |
| `inlineEdit` | `boolean \| 'grid' \| 'form'` | — | Edit child records inline on the parent create/edit form (`true` = auto-pick, `'grid'`, or `'form'`) |
| `inlineColumns` | `array` | — | Optional explicit inline grid columns |
| `inlineAmountField` | `string` | — | Numeric child field used for the inline running total |
Expand Down
10 changes: 7 additions & 3 deletions content/docs/protocol/objectql/types.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -661,9 +661,13 @@ const opportunities = await engine.find('opportunity', {
> been emptied.
>
> On `master_detail` the same reading applies from the other side: `restrict`
> is the only value that deviates from `cascade`, so an explicit
> `deleteBehavior: set_null` on a master-detail reference is *not* honored —
> the child is cascaded away.
> is the only value that deviates from `cascade`. An explicit
> `deleteBehavior: set_null` on a master-detail reference is a **parse-time
> rejection** (#9689) — a detail row cannot outlive its master, so the spec
> refuses the declaration instead of silently cascading the children it asked
> to keep. Metadata that bypasses the parse (a raw registration, or a row
> stored before the tightening) still resolves to `cascade`, now with a loud
> engine log at the coercion site.
>
> The refusal carries **two** messages, for two audiences. `error` is written for
> the person who clicked delete: it is rendered in the caller's locale from the
Expand Down
128 changes: 121 additions & 7 deletions packages/objectql/src/engine-cascade-delete.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -29,9 +29,13 @@
* `multiple: true` case (see below) and a `master_detail` declaring an explicit
* `set_null`, which is silently resolved to `cascade`.
*
* Whether the spec should reject `set_null` on a `master_detail` at publish
* time rather than dropping it at delete time is still an open question,
* carded separately — not decided by this suite.
* [#9689] That question is now answered (maintainer ruling 2026-08-19):
* `FieldSchema` REJECTS an authored `set_null` on a `master_detail` at parse
* time, and this engine logs loudly when the combination still reaches the
* coercion site (raw registrations — like this suite's — and metadata stored
* before the tightening; the engine registers raw objects and never
* re-parses, so the spec-layer rejection alone measurably does not change
* anything here). The COERCION itself is unchanged and stays pinned below.
*
* ## [#9688] The multi-value refusal now judges EMPTINESS, per row
*
Expand All@@ -54,7 +58,7 @@
* An authored `deleteBehavior: 'restrict'` is untouched by any of it.
*/

import { describe, it, expect, beforeEach } from 'vitest';
import { describe, it, expect, beforeEach, vi } from 'vitest';
import { ObjectQL } from './engine.js';

const acct = {
Expand DownExpand Up@@ -468,9 +472,13 @@ describe('cascadeDeleteRelations — required FK escalates set_null → restrict

it('[#9625] a master_detail declaring an explicit deleteBehavior:set_null still cascades', async () => {
// The neighbouring resolution with the same blind spot: `restrict` is
// the only value that deviates, so `set_null` is accepted by
// `FieldSchema` on this type and then dropped here. Pinned so the
// silent coercion is a documented fact rather than an absence.
// the only value that deviates, so every other value is dropped here.
// Pinned so the coercion is a documented fact rather than an absence.
// [#9689] `FieldSchema` now rejects this combination at parse time,
// but THIS registration is raw (the engine never re-parses), so the
// combination still reaches the engine and the coercion still applies
// — this pin stays TRUE by ruling; the delete-time change is the loud
// log, pinned in its own describe below.
const a = await engine.insert('acct', { name: 'Acme' });
const l = await engine.insert('line', { parent: a.id });

Expand DownExpand Up@@ -515,3 +523,109 @@ describe('cascadeDeleteRelations — required FK escalates set_null → restrict
expect((await engine.findOne('note', { where: { id: n.id } }) as any).account).toBeNull();
});
});

// [#9689] (maintainer ruling 2026-08-19, Q3 = B): the coercion above stays,
// and it now LOGS. `FieldSchema` rejects an authored `set_null` on a
// `master_detail` at parse time, so a value that still reaches the engine came
// in around the parse seam (raw registration / pre-tightening stored row) —
// the population parse-time rejection measurably cannot catch. The log fires
// on exactly the authored combination: not on a bare master_detail, not on an
// authored cascade, and not on restrict (which never coerces).
describe('cascadeDeleteRelations — [#9689] authored set_null on master_detail logs loudly at the coercion site', () => {
// A bare master_detail — the overwhelmingly common spelling; the engine
// resolves it to cascade identically, and it must NOT log.
const stanzaBare = {
name: 'stanza',
label: 'Stanza',
fields: {
id: { name: 'id', type: 'text' as const, primaryKey: true },
parent: { name: 'parent', type: 'master_detail' as const, reference: 'acct' },
},
};
// An authored cascade — same resolved behavior, deliberate; must NOT log.
const verseCascade = {
name: 'verse',
label: 'Verse',
fields: {
id: { name: 'id', type: 'text' as const, primaryKey: true },
parent: {
name: 'parent', type: 'master_detail' as const, reference: 'acct',
deleteBehavior: 'cascade',
},
},
};

function makeSpyLogger(withError = true) {
const spy = {
info: vi.fn(), warn: vi.fn(), debug: vi.fn(),
...(withError ? { error: vi.fn() } : {}),
};
return spy as Record<'info' | 'warn' | 'debug' | 'error', ReturnType<typeof vi.fn>>;
}

// NOTE the registration set is per test: the log fires at the COERCION
// SITE — whenever the parent delete computes the child field's behavior —
// not only when that child holds rows. A misdeclared child object in the
// registry therefore logs on every parent delete (deliberate: the
// declaration is wrong whether or not rows exist today), so the negative
// control below must not register `line` at all.
async function makeEngine(logger: Record<string, unknown>, objects: unknown[]) {
const engine = new ObjectQL({ logger });
const { driver } = makeStubDriver();
engine.registerDriver(driver, true);
await engine.init();
for (const o of objects) engine.registry.registerObject(o as any);
return engine;
}

it('logs via logger.error when the parent delete coerces an authored set_null to cascade', async () => {
const logger = makeSpyLogger();
const engine = await makeEngine(logger, [acct, lineExplicitSetNull, stanzaBare, verseCascade]);
const a = await engine.insert('acct', { name: 'Acme' });
const l = await engine.insert('line', { parent: a.id });

await engine.delete('acct', { where: { id: a.id } } as any);
// The pinned behavior is unchanged: the child cascaded away.
expect(await engine.findOne('line', { where: { id: l.id } })).toBeNull();

const hits = logger.error.mock.calls.filter((c) => String(c[0]).includes("deleteBehavior: 'set_null'"));
expect(hits).toHaveLength(1);
const msg = String(hits[0][0]);
// Attribution: which declaration, on which relation, and the outcome.
expect(msg).toContain('line.parent');
expect(msg).toContain('master_detail');
expect(msg).toContain('NOT honored');
expect(msg).toContain('CASCADES');
// Actionability: both legal re-declarations are named.
expect(msg).toContain("'restrict'");
expect(msg).toContain("'cascade'");
expect(logger.warn).not.toHaveBeenCalled();
});

it('falls back to logger.warn when the sink has no error method (#9750 sanctioned shape — never an optional call)', async () => {
const logger = makeSpyLogger(false);
const engine = await makeEngine(logger, [acct, lineExplicitSetNull]);
const a = await engine.insert('acct', { name: 'Acme' });
await engine.insert('line', { parent: a.id });

await engine.delete('acct', { where: { id: a.id } } as any);
expect(logger.warn.mock.calls.some((c) => String(c[0]).includes("deleteBehavior: 'set_null'"))).toBe(true);
});

it('does NOT log for a bare master_detail or an authored cascade (same resolved behavior, no divergence)', async () => {
const logger = makeSpyLogger();
const engine = await makeEngine(logger, [acct, stanzaBare, verseCascade]);
const a = await engine.insert('acct', { name: 'Acme' });
const s = await engine.insert('stanza', { parent: a.id });
const v = await engine.insert('verse', { parent: a.id });

await engine.delete('acct', { where: { id: a.id } } as any);
// Both cascaded (resolved behavior identical to the logging case) …
expect(await engine.findOne('stanza', { where: { id: s.id } })).toBeNull();
expect(await engine.findOne('verse', { where: { id: v.id } })).toBeNull();
// … and neither logged: the divergence between declared and delivered
// exists only for the authored set_null.
const all = [...logger.error.mock.calls, ...logger.warn.mock.calls].map((c) => String(c[0]));
expect(all.filter((m) => m.includes("deleteBehavior: 'set_null'"))).toHaveLength(0);
});
});
39 changes: 34 additions & 5 deletions packages/objectql/src/engine.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -10715,16 +10715,45 @@ export class ObjectQL implements IObjectQLEngine {
//
// [#9625] "Only an explicit `restrict` deviates" is the whole of it:
// every other value a master_detail can declare — including an
// explicit `deleteBehavior: 'set_null'`, which `FieldSchema` accepts
// on this type — resolves to `cascade` here, silently. Measured and
// pinned (`engine-cascade-delete.test.ts`); whether the spec should
// reject the combination at publish time instead of the engine
// dropping it at delete time is a judgement, carded separately.
// explicit `deleteBehavior: 'set_null'` — resolves to `cascade` here,
// silently. Measured and pinned (`engine-cascade-delete.test.ts`).
let behavior: string =
fdef.type === 'master_detail'
? (fdef.deleteBehavior === 'restrict' ? 'restrict' : 'cascade')
: (fdef.deleteBehavior || 'set_null');

// [#9689] (maintainer ruling 2026-08-19, Q3 = B): the judgement the
// #9625 comment above deferred is now taken — `FieldSchema` REJECTS an
// authored `deleteBehavior: 'set_null'` on a `master_detail` at parse
// time, so the value is meaningful again: one that still reaches this
// site came in around the parse seam (a raw `registerObject`, or a
// stored/artifact row written before the tightening — the two
// populations parse-time rejection measurably cannot catch, since the
// engine registers raw objects and never re-parses). The coercion
// itself stays: this delete is about to CASCADE children whose
// declaration asked for them to be kept, and that divergence must be
// loud and attributable, not silent. Sanctioned logger shape per
// PR #9750: reach for `error`, fall back to `warn` — NEVER an optional
// call like `logger.error?.()`, which emits nothing against a sink
// with no `error`. Caveat, measured (#4447): a built app artifact
// materializes FieldSchema defaults, so an artifact-loaded BARE
// master_detail also carries `set_null` and logs here — that residual
// imprecision is the materialized-default defect tracked as #9784, not
// a reason to soften this log.
if (fdef.type === 'master_detail' && fdef.deleteBehavior === 'set_null') {
const msg =
`[cascade-delete] ${childName}.${fieldName} declares deleteBehavior: 'set_null' on a ` +
`master_detail referencing '${object}' — that value is NOT honored on master_detail: the ` +
`delete of ${object}/${String(id)} CASCADES its referencing child rows, the opposite of what ` +
`the declaration asks (children kept). FieldSchema now rejects this combination at parse time; ` +
`this row reached the engine around the parse seam (raw registration, or metadata stored ` +
`before the tightening). Re-declare the field: 'restrict' refuses the parent delete while ` +
`children exist (no data loss), 'cascade' (or omitting the key) accepts the cascade ` +
`deliberately, or make it a lookup if children must survive the parent.`;
if (typeof this.logger.error === 'function') this.logger.error(msg);
else this.logger.warn(msg);
}

// A REQUIRED foreign key cannot be nulled — set_null would issue an
// UPDATE clearing the FK, which the child's required-field validator
// rejects with a misleading "<field> is required" 400 (the field isn't
Expand Down
Loading
Loading