Skip to content

Commit e96bbe3

Browse files
dmealingclaude
andcommitted
fix(metamodel): the index key is @fields XOR @expr, in all five ports (#342)
`@expr` was registered, built and rendered — and unreachable. The registry has always described it as "Used INSTEAD of @fields", migrate-ts has always keyed off it (`columns: expr ? [] : cols`), the Postgres emitter has always rendered it, and introspection has always read expression keys back via pg_get_expr. Only the LOADER disagreed, requiring @fields unconditionally. So: @expr alone failed to load on both node types, and the one spelling that DID load — @fields AND @expr — had its @fields silently discarded downstream. An expression index had to be kept as hand-written SQL outside the migration ledger, which then read as permanent `verify --db` drift. The rule is now stated once and enforced once. Neither is an error, as before; BOTH is now also an error rather than accepted-and-half-honored. Failing closed matches the sealed-strict posture and the ERR_SQL_BODY_WITH_UNMANAGED precedent — @SQL vs @Unmanaged is the same "two mutually exclusive non-default states of one axis" shape. PATCH, not MINOR: a declaration that loaded while silently throwing half of itself away was never validly expressible, so refusing it corrects previously-wrong acceptance rather than changing a contract — the same call as the @min clamp in 0.19.1 and the `like` case-sensitivity fix in 0.21.6. metamodelVersion moves 0.10 -> 0.11 (@fields becomes optional on both types, which the gate classifies as additive); the package line stays a patch, which is what ADR-0035 Am. 2 severed the two numbers for. Covers identity.secondary as well as index.lookup: per ADR-0040 uniqueness lives in the TYPE, so identity.secondary IS a unique index, keys itself identically, and carries @expr from the same db provider. identity.primary / identity.reference are untouched — a primary key or an FK is always plain columns and has no @expr at all. TWO CROSS-PORT DIVERGENCES THE NEW FIXTURE FOUND, THAT NO PORT'S OWN SUITE DID. Java carried a BESPOKE @fields requirement in ValidationPhase.validateIdentityNode, applied to every identity independently of the registry — so relaxing the schema tier left Java alone refusing metadata the other three accepted. And Python emitted its index errors with an EMPTY provenance envelope while TS/Java/C# attached node.source; the corpus asserts envelope shape, which is what surfaced it. Gated by two shared fixtures — index-expr-keys-without-fields (both node types keyed by expression) and error-index-fields-and-expr — plus per-port tests. The positive fixture asserts a CLEAN load, not merely the absence of one error code: the weaker assertion passed while the child-rule check still refused it under a different code, which is how this reached an adopter. Proved by reverting the fix and watching it go red. Also folds in a one-line uv.lock catch-up left stale by the 0.24.0 version bump. TS 2481 · Java 1470 · Python 1837 · C# 1672 — all green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DhpswkF1NvwxhFWMmdAT15
1 parent ab82d0f commit e96bbe3

35 files changed

Lines changed: 633 additions & 99 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,59 @@ this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
77

88
## [Unreleased]
99

10+
## [0.24.1] — npm `0.24.1` · PyPI `0.24.1` · NuGet `0.24.1` · Maven `7.24.1`
11+
12+
### Fixed — an expression index was undeclarable, and the one spelling that loaded was half-ignored ([#342](https://github.com/metaobjectsdev/metaobjects/issues/342))
13+
14+
**`@expr` was registered, built and rendered — and unreachable.** The registry has always
15+
described it as *"Used **INSTEAD of** `@fields`"*, `migrate-ts` has always keyed off it
16+
(`columns: expr ? [] : cols`), the Postgres emitter has always rendered it, and
17+
introspection has always read expression keys back via `pg_get_expr`. Only the **loader**
18+
disagreed, requiring `@fields` unconditionally. So the three-row matrix an adopter hit was:
19+
20+
| Node | attrs | Before |
21+
|---|---|---|
22+
| `index.lookup` | `@expr` only | ❌ fails to load |
23+
| `identity.secondary` | `@expr` only | ❌ fails to load |
24+
| either | `@fields` **+** `@expr` | ⚠️ loads — and `@fields` is **silently discarded** |
25+
26+
An expression index therefore had to live as hand-written SQL outside the migration
27+
ledger, which then reported as permanent `verify --db` drift.
28+
29+
**The rule is now stated once and enforced once: the index key is `@fields` XOR `@expr`.**
30+
Declaring neither is an error (as before); declaring **both is now also an error**
31+
(`ERR_INVALID_INDEX`) rather than being accepted and half-honored. Failing closed matches
32+
the sealed-strict-registry posture and the existing `ERR_SQL_BODY_WITH_UNMANAGED`
33+
precedent — `@sql` vs `@unmanaged` is the same "two mutually exclusive non-default states
34+
of one axis" shape. **This is a PATCH, not a MINOR:** a declaration that loaded while
35+
silently throwing half of itself away was never validly expressible, so refusing it
36+
corrects previously-wrong acceptance rather than changing a contract — the same call, for
37+
the same reason, as the `@min` clamp in `0.19.1` and the `like` case-sensitivity fix in
38+
`0.21.6`.
39+
40+
**Applies to `identity.secondary` as well as `index.lookup`**, because per
41+
[ADR-0040](spec/decisions/ADR-0040-index-type-and-secondary-key-purity.md) uniqueness
42+
lives in the TYPE — `identity.secondary` IS a unique index, keys itself identically, and
43+
carries `@expr` from the same db provider. `identity.primary` and `identity.reference` are
44+
untouched: a primary key or an FK is always plain columns and carries no `@expr` at all.
45+
46+
**`metamodelVersion` moves `0.10` → `0.11`** — `@fields` becomes optional on both node
47+
types, which the gate classifies as additive. The package line stays a PATCH; that
48+
severance is exactly what [ADR-0035 Amendment 2](docs/RELEASING.md) is for, and post-1.0
49+
the changelog is the only signal on the metadata axis.
50+
51+
**Two things this took, worth recording.** The Java port needed a fix the other three did
52+
not: beyond the registry declaration, `ValidationPhase.validateIdentityNode` carried a
53+
**bespoke, Java-only** `@fields` requirement applied to every identity, so relaxing the
54+
schema tier alone left Java refusing metadata that TypeScript, C# and Python accepted —
55+
found by the new cross-port fixture, not by any port's own suite. And Python's index
56+
errors were emitted with an **empty provenance envelope** while the other three attached
57+
`node.source`; the corpus asserts envelope shape, which is what surfaced it.
58+
59+
Gated by two new shared conformance fixtures — `index-expr-keys-without-fields` (both node
60+
types keyed by expression, loads clean) and `error-index-fields-and-expr` — plus per-port
61+
tests. All five ports green.
62+
1063
## [0.24.0] — npm `0.24.0` · PyPI `0.24.0` · NuGet `0.24.0` · Maven `7.24.0`
1164

1265
> ### ⚠️ BREAKING FOR METADATA AUTHORS — five vocabulary changes in ONE window
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
{
2+
"errors": [
3+
{
4+
"code": "ERR_INVALID_INDEX",
5+
"source": {
6+
"format": "json",
7+
"files": [
8+
"meta.demo.json"
9+
],
10+
"jsonPath": "$['metadata.root'].children[0]['object.entity'].children[5]['index.lookup']"
11+
}
12+
}
13+
],
14+
"warnings": []
15+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
{
2+
"metadata.root": {
3+
"package": "demo::indexing",
4+
"children": [
5+
{
6+
"object.entity": {
7+
"name": "Notification",
8+
"children": [
9+
{ "source.rdb": { "@table": "notifications" } },
10+
{ "field.string": { "name": "id", "@required": true } },
11+
{ "field.timestamp": { "name": "deliveredAt" } },
12+
{ "field.string": { "name": "payload" } },
13+
{ "identity.primary": { "name": "pk", "@fields": ["id"] } },
14+
15+
{
16+
"index.lookup": {
17+
"name": "bothIdx",
18+
"@fields": ["deliveredAt"],
19+
"@expr": "(payload->>'device_id')"
20+
}
21+
}
22+
]
23+
}
24+
}
25+
]
26+
}
27+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
["metaobjects-core-types","metaobjects-db"]
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
{
2+
"metadata.root": {
3+
"package": "demo::indexing",
4+
"children": [
5+
{
6+
"object.entity": {
7+
"name": "Notification",
8+
"children": [
9+
{
10+
"source.rdb": {
11+
"@table": "notifications"
12+
}
13+
},
14+
{
15+
"field.string": {
16+
"name": "id",
17+
"@required": true
18+
}
19+
},
20+
{
21+
"field.timestamp": {
22+
"name": "deliveredAt"
23+
}
24+
},
25+
{
26+
"field.string": {
27+
"name": "payload"
28+
}
29+
},
30+
{
31+
"identity.primary": {
32+
"name": "pk",
33+
"@fields": [
34+
"id"
35+
]
36+
}
37+
},
38+
{
39+
"index.lookup": {
40+
"name": "pendingDeviceIdx",
41+
"@expr": "(payload->>'device_id')",
42+
"@where": "delivered_at IS NULL"
43+
}
44+
},
45+
{
46+
"identity.secondary": {
47+
"name": "lowerIdUk",
48+
"@expr": "lower(id)"
49+
}
50+
}
51+
]
52+
}
53+
}
54+
]
55+
}
56+
}
57+
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
{
2+
"metadata.root": {
3+
"package": "demo::indexing",
4+
"children": [
5+
{
6+
"object.entity": {
7+
"name": "Notification",
8+
"children": [
9+
{ "source.rdb": { "@table": "notifications" } },
10+
{ "field.string": { "name": "id", "@required": true } },
11+
{ "field.timestamp": { "name": "deliveredAt" } },
12+
{ "field.string": { "name": "payload" } },
13+
{ "identity.primary": { "name": "pk", "@fields": ["id"] } },
14+
15+
{
16+
"index.lookup": {
17+
"name": "pendingDeviceIdx",
18+
"@expr": "(payload->>'device_id')",
19+
"@where": "delivered_at IS NULL"
20+
}
21+
},
22+
23+
{
24+
"identity.secondary": {
25+
"name": "lowerIdUk",
26+
"@expr": "lower(id)"
27+
}
28+
}
29+
]
30+
}
31+
}
32+
]
33+
}
34+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
["metaobjects-core-types","metaobjects-db"]

‎fixtures/metamodel-docs/expected/types/identity.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ A secondary unique index on one or more fields. Always enforces uniqueness — u
6565
| Attribute | Type | Required | Default | Allowed values | Provider | Description |
6666
| --- | --- | --- | --- | --- | --- | --- |
6767
| `@expr` | string | no | | | metaobjects-db | Raw key EXPRESSION for a functional/expression index (e.g. "lower(email)"). Used INSTEAD of @fields — the index key is the expression rather than plain columns. RDB-physical — contributed by the db provider. |
68-
| `@fields` | string[] | yes | | | metaobjects-core-types | The field name(s) composing this identity. Single-element for a simple unique index, multiple for a composite unique constraint. |
68+
| `@fields` | string[] | no | | | metaobjects-core-types | The field name(s) composing this identity. Single-element for a simple unique index, multiple for a composite unique constraint. Required UNLESS @expr is present — a unique index keys off plain columns (@fields) or a key expression (@expr), never both; declaring both is ERR_INVALID_INDEX. |
6969
| `@orders` | string[] | no | | `asc`, `desc` | metaobjects-db | Physical index-key sort direction, positional to @fields ('asc' \| 'desc'). Omit for all-ascending (the default); a shorter array leaves trailing keys ascending. Drives DESC-ordered index keys (e.g. a recency index on a timestamp). RDB-physical — contributed by the db provider, not core identity. |
7070
| `@using` | string | no | | | metaobjects-db | Index access method (e.g. "gin", "gist", "hash"); default "btree" (not rendered). Pair with @expr for e.g. a GIN index over an array/jsonb expression. RDB-physical — contributed by the db provider. |
7171
| `@where` | string | no | | | metaobjects-db | Partial-index predicate (raw SQL, e.g. "delivered_at IS NULL"). When set, the index covers only rows matching the predicate — smaller and cheaper for queries that always filter on it. Absent = a full index over every row. RDB-physical — contributed by the db provider. |

‎fixtures/metamodel-docs/expected/types/index.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ A non-unique lookup index on one or more fields. Use for query-performance index
2121
| Attribute | Type | Required | Default | Allowed values | Provider | Description |
2222
| --- | --- | --- | --- | --- | --- | --- |
2323
| `@expr` | string | no | | | metaobjects-db | Raw key EXPRESSION for a functional/expression index (e.g. "lower(email)"). Used INSTEAD of @fields — the index key is the expression rather than plain columns. RDB-physical — contributed by the db provider. |
24-
| `@fields` | string[] | yes | | | — | The field name(s) this index covers (at least one). When @expr is present, it is the key expression derived from these fields. |
24+
| `@fields` | string[] | no | | | — | The field name(s) this index covers. Required UNLESS @expr is present — an index keys off plain columns (@fields) or a key expression (@expr), never both; declaring both is ERR_INVALID_INDEX. |
2525
| `@orders` | string[] | no | | `asc`, `desc` | metaobjects-db | Physical index-key sort direction, positional to @fields ('asc' \| 'desc'). Omit for all-ascending (the default); a shorter array leaves trailing keys ascending. Drives DESC-ordered index keys (e.g. a recency index on a timestamp). RDB-physical — contributed by the db provider. |
2626
| `@using` | string | no | | | metaobjects-db | Index access method (e.g. "gin", "gist", "hash"); default "btree" (not rendered). Pair with @expr for e.g. a GIN index over an array/jsonb expression. RDB-physical — contributed by the db provider. |
2727
| `@where` | string | no | | | metaobjects-db | Partial-index predicate (raw SQL, e.g. "delivered_at IS NULL"). When set, the index covers only rows matching the predicate — smaller and cheaper for queries that always filter on it. Absent = a full index over every row. RDB-physical — contributed by the db provider. |

‎fixtures/registry-conformance/expected-registry.json‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"metamodelVersion": "0.10",
2+
"metamodelVersion": "0.11",
33
"types": [
44
{
55
"type": "attr",
@@ -2924,8 +2924,8 @@
29242924
"name": "fields",
29252925
"valueType": "string",
29262926
"isArray": true,
2927-
"required": true,
2928-
"description": "The field name(s) composing this identity. Single-element for a simple unique index, multiple for a composite unique constraint."
2927+
"required": false,
2928+
"description": "The field name(s) composing this identity. Single-element for a simple unique index, multiple for a composite unique constraint. Required UNLESS @expr is present — a unique index keys off plain columns (@fields) or a key expression (@expr), never both; declaring both is ERR_INVALID_INDEX."
29292929
},
29302930
{
29312931
"name": "orders",
@@ -2972,8 +2972,8 @@
29722972
"name": "fields",
29732973
"valueType": "string",
29742974
"isArray": true,
2975-
"required": true,
2976-
"description": "The field name(s) this index covers (at least one). When @expr is present, it is the key expression derived from these fields."
2975+
"required": false,
2976+
"description": "The field name(s) this index covers. Required UNLESS @expr is present — an index keys off plain columns (@fields) or a key expression (@expr), never both; declaring both is ERR_INVALID_INDEX."
29772977
},
29782978
{
29792979
"name": "orders",

0 commit comments

Comments
 (0)