Skip to content

Commit 72c3c86

Browse files
os-zhuangclaude
andauthored
refactor(spec)!: retire indexes[].typeindexes[].partial —— 两个零 DDL 消费者的声明键 (#5248, #4943) (#5842)
* refactor(spec)!: retire `indexes[].type` and `indexes[].partial` (#5248, #4943) Both keys were authorable with zero DDL consumers. `syncDeclaredIndexes` creates every declared index through knex's `table.index()` / `table.unique()`, and the drift differ's `DeclaredIndexInput` carries `name`/`fields`/`unique`/`nullSafeColumns` — so an authored `type` selected no access method and an authored `partial` produced a FULL index with the predicate silently discarded. `type` additionally carried `.default('btree')`, materializing an inert knob into every parse output (ADR-0078). Retirement kit: `retiredKey()` tombstones at the bottom of the IndexSchema shape (#5606 renderer note), ADR-0087 conversion `object-index-type-partial-removed` opening the protocol-18 step, both real producers flipped (`sys_metadata`, `sys_view_definition`) with their comments corrected, published skill + docs + liveness note + generated baselines. DDL equivalence is proven against the statements SQLite actually stores, not asserted: packages/drivers/driver-sql/src/declared-index-retired-keys.test.ts Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018fxLGQdatPbBUvCgiVxg6D * fixup: retarget the IndexSchema retirement to protocol 17 (PM ruling) `toMajor: 18` → `17`; `step18` deleted and `object-index-type-partial-removed` wired into the existing step-17 chain (its rationale extended). Tombstone prescriptions now say "removed in @objectstack/spec 17.0.0" and point at `os migrate meta --from 16`, matching every other toMajor-17 entry. Skill, docs, liveness note and changeset retargeted; generated artifacts (spec-changes.json, upgrade guide, reference docs) regenerated. Changeset stays `major`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018fxLGQdatPbBUvCgiVxg6D --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2d14b35 commit 72c3c86

20 files changed

Lines changed: 707 additions & 189 deletions

File tree

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
---
2+
"@objectstack/spec": major
3+
"@objectstack/metadata-core": patch
4+
---
5+
6+
refactor(spec)!: retire `indexes[].type` and `indexes[].partial` — two authorable index keys no driver ever read (#5248, #4943)
7+
8+
`IndexSchema` declared five keys; only three of them ever reached a `CREATE
9+
INDEX`. `SqlDriver.syncDeclaredIndexes` builds every declared index through
10+
knex's `table.index(fields, name)` / `table.unique(fields, { indexName })`, and
11+
the drift differ's `DeclaredIndexInput` carries `name` / `fields` / `unique` /
12+
`nullSafeColumns`. So:
13+
14+
- **`partial`** — documented as *"Partial index condition (SQL WHERE clause)"*
15+
produced a **full** index with the predicate silently discarded. This was the
16+
damaging half, because it reads as a correctness control: the platform's own
17+
`sys_metadata` declared `partial: "state = 'active'"` for overlay uniqueness,
18+
and what the declaration alone materialized was an *unrestricted* unique index.
19+
- **`type`** additionally carried `.default('btree')`, so it appeared in **every**
20+
parse output of **every** index — an access-method knob that had never
21+
influenced a single statement, rendered as live configuration. (It was pinned
22+
as such in a `sys_presence` test, on an object that never declared it.)
23+
24+
Both are the ADR-0078 no-silently-inert / ADR-0049 enforce-or-remove shape.
25+
Remove was chosen over enforce: enforcing needs per-dialect algorithm mapping
26+
(`gin`/`gist` Postgres-only, `fulltext` MySQL-family), raw-SQL `CREATE INDEX …
27+
WHERE` on the dialects that have partial indexes at all (MySQL does not), and a
28+
redesign of how `isSyncReproducibleIndex` excludes partial indexes from
29+
incremental sync — design cost for a capability with no demand. If a real need
30+
appears it returns enforce-first.
31+
32+
## Migration
33+
34+
| FROM | TO |
35+
| :--- | :--- |
36+
| `indexes: [{ fields: […], type: 'gin' }]` | `indexes: [{ fields: […] }]` — create the specialised index from a database-layer migration |
37+
| `indexes: [{ fields: […], partial: "state = 'active'" }]` | `indexes: [{ fields: […] }]` — issue `CREATE [UNIQUE] INDEX … WHERE …` from a runtime migration |
38+
39+
**One-line fix: delete the key.** Neither removal changes any DDL, because no
40+
DDL ever depended on them — verified byte-for-byte against the `CREATE INDEX`
41+
statements SQLite actually stores
42+
(`packages/drivers/driver-sql/src/declared-index-retired-keys.test.ts`).
43+
44+
Both capabilities remain available where they are implementable. The index
45+
method is the driver/dialect's choice. A partial index is issued as raw SQL from
46+
a runtime migration — exactly what `metadata-protocol`'s `ensureOverlayIndex`
47+
already does for `sys_metadata`, and what actually delivers that table's
48+
active-row-scoped uniqueness today.
49+
50+
⚠️ **Not affected:** driver-sql's own `partial` flag (`parseIndexDdl` /
51+
`introspectIndexes` / `isSyncReproducibleIndex`). That is a boolean parsed back
52+
out of the *database's own* DDL for drift detection — the opposite direction —
53+
so migration-created partial indexes stay recognized and exempt from incremental
54+
sync, unchanged.
55+
56+
## The retirement kit
57+
58+
- `retiredKey()` tombstones at `IndexSchema` (the shape is deliberately
59+
`.strip()`, so a plain delete would swap one silent no-op for another): writing
60+
either key is now a `tsc` error and a parse error carrying the prescription.
61+
They sit at the bottom of the shape per the #5606 renderer note.
62+
- **ADR-0087 D2 conversion + D3 chain step** (`object-index-type-partial-removed`,
63+
`toMajor: 17`, wired into the existing step-17 chain): strips both keys from
64+
`objects[]` and `objectExtensions[]`; `os migrate meta --from 16` rewrites sources
65+
mechanically. A pure lossless delete — there was no effect to lose.
66+
- **Producers flipped:** `sys_metadata` (`idx_sys_metadata_overlay_active`, the
67+
case #4943 named) and `sys_view_definition` (`idx_sys_view_def_active`), both
68+
with their comments corrected to say what is actually materialized.
69+
- Published skill (`objectstack-data`), `content/docs/data-modeling/objects.mdx`,
70+
liveness ledger note and generated baselines updated.

content/docs/data-modeling/objects.mdx

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -209,20 +209,34 @@ Optimize query performance:
209209
{/* os:check */}
210210
```typescript
211211
indexes: [
212-
{ fields: ['name'], type: 'btree', unique: false },
213-
{ fields: ['email'], type: 'btree', unique: 'organization' },
214-
{ fields: ['type', 'status'], type: 'btree', unique: false },
212+
{ fields: ['name'] },
213+
{ fields: ['email'], unique: 'organization' },
214+
{ fields: ['type', 'status'] },
215215
]
216216
```
217217

218218
| Property | Type | Required | Description |
219219
| :--- | :--- | :--- | :--- |
220220
| `fields` | `string[]` || Fields in the index |
221-
| `type` | `enum` | optional | `'btree'`, `'hash'`, `'gin'`, `'gist'`, `'fulltext'` (default: `'btree'`) |
222-
| `unique` | `boolean` | optional | Enforce uniqueness (default: `false`) |
223-
| `partial` | `string` | optional | Conditional index (SQL WHERE clause) |
221+
| `unique` | `boolean \| 'global' \| 'organization'` | optional | Enforce uniqueness, and at which scope (default: `false`) |
224222
| `name` | `string` | optional | Index name (auto-generated if omitted) |
225223

224+
<Callout type="warn">
225+
**`type` and `partial` were retired in protocol 17** (#5248, #4943). Neither
226+
had a driver consumer: declared indexes are created through knex's
227+
`table.index()` / `table.unique()`, so an authored `type` selected no access
228+
method and an authored `partial` produced a **full** index with the predicate
229+
silently discarded. Writing either now fails `tsc` and the parse with a
230+
migration prescription — run `os migrate meta --from 16` to strip them.
231+
232+
Both capabilities remain available where they are actually implementable: the
233+
index method is the driver/dialect's choice, and a partial index is issued as
234+
raw SQL from a runtime migration (`CREATE [UNIQUE] INDEX … WHERE …`, the way
235+
`metadata-protocol` builds `sys_metadata`'s overlay index). Drift detection
236+
reads partiality back from the database's own DDL, so migration-created
237+
partial indexes are recognized and left alone.
238+
</Callout>
239+
226240
### Additional Properties
227241

228242
| Property | Type | Description |
@@ -347,8 +361,8 @@ export const ProjectTask = ObjectSchema.create({
347361
},
348362

349363
indexes: [
350-
{ fields: ['status'], type: 'btree', unique: false },
351-
{ fields: ['project', 'status'], type: 'btree', unique: false },
364+
{ fields: ['status'] },
365+
{ fields: ['project', 'status'] },
352366
],
353367

354368
enable: {

content/docs/references/data/object.mdx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,9 @@ const result = ApiMethod.parse(data);
6767
| :--- | :--- | :--- | :--- |
6868
| **name** | `string` | optional | Index name (auto-generated if not provided) |
6969
| **fields** | `string[]` || Fields included in the index |
70-
| **type** | `Enum<'btree' \| 'hash' \| 'gin' \| 'gist' \| 'fulltext'>` || Index algorithm type |
7170
| **unique** | `boolean \| 'global' \| 'organization'` || Whether the index enforces uniqueness, and at which scope (ADR-0120). 'global' = materialized over exactly `fields`, no organization column injected — one holder across the whole installation; 'organization' = the driver prepends the NULL-safe organization key part (COALESCE(organization_id, '__global__')) at registration — one holder per organization; bare true = deprecated positional spelling of 'global' (warned in 17.x by lint unique/unscoped-declared-index, rejected at protocol 18, #5082) — state the scope. 'tenant'/'org' are rejected — the word is 'organization' |
72-
| **partial** | `string` | optional | Partial index condition (SQL WHERE clause for conditional indexes) |
71+
| **type** | `any` | optional | [REMOVED] `indexes[].type` was removed in @objectstack/spec 17.0.0 (#5248, ADR-0049) — no driver ever read it. `SqlDriver.syncDeclaredIndexes` creates every declared index through knex's `table.index()` / `table.unique()`, which cannot express an access method, so the value changed no DDL; its `.default('btree')` merely made an inert knob show up in every parse output. Delete the key. The index method is the driver/dialect's decision (Postgres defaults to B-tree; `gin`/`gist`/`fulltext` are dialect-specific and are chosen by a database-layer migration when a workload actually needs one). Run `os migrate meta --from 16` to rewrite it automatically. |
72+
| **partial** | `any` | optional | [REMOVED] `indexes[].partial` was removed in @objectstack/spec 17.0.0 (#5248, #4943, ADR-0049) — no driver ever emitted the `WHERE` clause, so a declared partial index was materialized as a FULL index and the predicate silently did nothing. Delete the key. Partial indexes are built at the database layer, not the declaration surface: issue `CREATE [UNIQUE] INDEX … WHERE <predicate>` from a runtime migration (this is what `metadata-protocol`'s `ensureOverlayIndex` already does for `sys_metadata`). Drift detection is unaffected — it reads partiality back from the database's own DDL, never from this key. Run `os migrate meta --from 16` to rewrite it automatically. |
7373

7474

7575
---
@@ -122,7 +122,7 @@ const result = ApiMethod.parse(data);
122122
| **datasource** | `string` | optional | Target Datasource ID. "default" is the primary DB. |
123123
| **external** | `{ remoteName?: string; remoteSchema?: string; writable?: boolean; columnMap?: Record<string, string>; … }` | optional | Remote table binding for federated (external) objects. |
124124
| **fields** | `Record<string, { name?: string; label?: string; type: Enum<'text' \| 'textarea' \| 'email' \| 'url' \| 'phone' \| 'password' \| 'secret' \| 'markdown' \| 'html' \| 'richtext' \| 'number' \| 'currency' \| 'percent' \| 'date' \| 'datetime' \| 'time' \| 'boolean' \| 'toggle' \| 'select' \| 'multiselect' \| 'radio' \| 'checkboxes' \| 'lookup' \| 'master_detail' \| 'tree' \| 'user' \| 'image' \| 'file' \| 'avatar' \| 'video' \| 'audio' \| 'formula' \| 'summary' \| 'autonumber' \| 'composite' \| 'repeater' \| 'record' \| 'location' \| 'address' \| 'code' \| 'json' \| 'color' \| 'rating' \| 'slider' \| 'signature' \| 'qrcode' \| 'progress' \| 'tags' \| 'vector'>; description?: string; … }>` || Field definitions map. Keys must be snake_case identifiers. |
125-
| **indexes** | `{ name?: string; fields: string[]; type?: Enum<'btree' \| 'hash' \| 'gin' \| 'gist' \| 'fulltext'>; unique?: boolean \| 'global' \| 'organization'; … }[]` | optional | Database performance indexes |
125+
| **indexes** | `{ name?: string; fields: string[]; unique?: boolean \| 'global' \| 'organization'; type?: any; … }[]` | optional | Database performance indexes |
126126
| **fieldGroups** | `{ key: string; label: string; icon?: string; description?: string; … }[]` | optional | Ordered list of field groups (array order = display order). See ObjectFieldGroupSchema. |
127127
| **tenancy** | `{ enabled: boolean; tenantField?: string }` | optional | Multi-tenancy configuration for SaaS applications |
128128
| **access** | `{ default?: Enum<'public' \| 'private'> }` | optional | [ADR-0066 D2] Object exposure posture (public-by-default vs private secure-by-default). |
@@ -196,7 +196,7 @@ const result = ApiMethod.parse(data);
196196
| **pluralLabel** | `string` | optional | Override plural label for the extended object |
197197
| **description** | `string` | optional | Override description for the extended object |
198198
| **validations** | `any[]` | optional | Additional validation rules to merge into the target object |
199-
| **indexes** | `{ name?: string; fields: string[]; type?: Enum<'btree' \| 'hash' \| 'gin' \| 'gist' \| 'fulltext'>; unique?: boolean \| 'global' \| 'organization'; … }[]` | optional | Additional indexes to merge into the target object |
199+
| **indexes** | `{ name?: string; fields: string[]; unique?: boolean \| 'global' \| 'organization'; type?: any; … }[]` | optional | Additional indexes to merge into the target object |
200200
| **priority** | `integer` | optional | Merge priority (higher = applied later) |
201201

202202

docs/protocol-upgrade-guide.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,8 @@ The same is true of the protocol-17 retirement that closes this list, and the pa
208208

209209
The last enforce-or-remove entry of this step is on the RUNTIME context rather than on anything authorable: `HookContext.session.roles` (#5050). It was declared in `data/hook.zod.ts`, read by exactly two consumers — the approvals record lock and the delegation write guard, each opening with `session.roles?.includes('admin')` — and produced by nobody on the hook path: ObjectQL's `buildSession()` writes the session field by field (`userId`, `organizationId`, `accessToken`, `isSystem`, `actor`, the skip flags) and has no `roles` write, here or in `cloud`, whose hook consumers read `hookContext?.session?.userId` and nothing else (an ACTION body's `ctx.session` is a different untyped object that does carry one, tracked apart). So both branches were dead on every real engine path: an authorization decision in shape only, and — worse for a reader — a SECOND admin dialect competing with the one ADR-0095 D3 sanctions. #4839 (PR #5049) deleted the two readers on the maintainer's ruling; this step removes the declaration that outlived them, which is what ADR-0049 asks for once a key has neither end. Nothing observable changes: a key nobody wrote and nothing read cannot alter a single decision. It is tombstoned rather than deleted because `HookContextSchema` is deliberately NOT `.strict()` (strictness there would make an engine-internal enrichment a breaking change for anyone parsing a context they were handed, as `provenance` was in #3712), so a plain delete would strip the key in silence — the #3733 / ADR-0104 failure this whole pass exists to end. There is NO conversion and no source rewrite: a HookContext is built per operation by the engine and never stored, so no `sys_metadata` row, example or template can carry the key — the `openApi31` / `activationEvents` shape, one semantic TODO for hook authors. The live vocabulary is untouched and deliberately elsewhere: gate on `session.userId` / `session.isSystem` in the hook, and judge PRIVILEGE through the security service, which reads capability grants (`permissions`), placements (`positions`) and the derived posture off the execution context.
210210

211+
Finally it retires the two inert `IndexSchema` keys, `indexes[].type` and `indexes[].partial` (#5248, #4943). Neither ever had a DDL consumer: `SqlDriver.syncDeclaredIndexes` creates declared indexes through knex's `table.index()` / `table.unique()`, and the drift differ's `DeclaredIndexInput` carries only `name`/`fields`/`unique`/`nullSafeColumns` — so an authored `type` selected no access method and an authored `partial` produced a FULL index with its predicate discarded. `partial` was the more damaging of the two because it read as a correctness control: the platform's own `sys_metadata` declared it for overlay uniqueness, and what the declaration alone materialized was an unrestricted unique index (the active-row scoping is delivered by a runtime migration, `metadata-protocol`'s `ensureOverlayIndex`, not by the key). `type` was the louder: its `.default('btree')` put an inert knob into every parse output, so it read as live configuration — the ADR-0078 no-silently-inert shape. Remove was chosen over enforce (maintainer ruling, 2026-08-06): enforcing needs per-dialect algorithm mapping (`gin`/`gist` Postgres-only, `fulltext` MySQL-family), raw-SQL `CREATE INDEX … WHERE` on the dialects that have partial indexes at all (MySQL does not), and a redesign of how `isSyncReproducibleIndex` excludes partial indexes from incremental sync — design cost for a capability nothing has asked for. Both are lossless deletes: no DDL changes, because no DDL ever depended on them. Drift detection is untouched — the `partial` flag it consumes is parsed back out of the database's OWN `CREATE INDEX` DDL and never came from this key.
212+
211213
### Mechanical (applied for you)
212214

213215
| Conversion | Surface | Change | Load window |
@@ -255,6 +257,7 @@ The last enforce-or-remove entry of this step is on the RUNTIME context rather t
255257
| `connector-rate-limit-config-removed` | `connector.rateLimitConfig` | connector key 'rateLimitConfig' removed (#4911 — no outbound rate-limiting engine exists; the runtime's only token bucket limits INBOUND requests, so every knob here was inert while reading like a configured cap. The whole ConnectorRateLimitConfig shape went with it) | retired — `migrate meta` only |
256258
| `theme-inert-token-scales-removed` | `theme.typography.fontSize / theme.typography.fontWeight / theme.typography.lineHeight / theme.typography.letterSpacing / theme.typography.fontFamily.heading / theme.typography.fontFamily.mono / theme.animation / theme.zIndex` | theme keys 'typography.fontSize'/'fontWeight'/'lineHeight'/'letterSpacing', 'typography.fontFamily.heading'/'mono', 'animation' and 'zIndex' removed (#5021, ADR-0049 — the engine emitted --font-size-*, --font-weight-*, --line-height-*, --letter-spacing-*, --duration-*, --timing-*, --z-*, --font-heading and --font-mono faithfully, and no first-party component or stylesheet has ever read one. Re-declare any variable you actually consume under customVars, which emits it verbatim) | retired — `migrate meta` only |
257259
| `page-header-subtitle-alias` | `page.component.page-header.description` | page-header component prop 'description' → 'subtitle' (objectui#3226 — the `subtitle ?? description` fallback retires) | live — protocol 17 loader accepts the old shape |
260+
| `object-index-type-partial-removed` | `object.indexes[].type / object.indexes[].partial` | object index keys 'indexes[].type'/'indexes[].partial' removed (#5248, #4943 — no driver ever read either: the index method is the dialect's choice and a partial index is built by a database-layer migration, not declared) | retired — `migrate meta` only |
258261

259262
### Semantic (delegated to you, with acceptance criteria)
260263

0 commit comments

Comments
 (0)