From 7ac0aa664356d68d909b91d684656b936e77733e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 03:39:08 +0000 Subject: [PATCH] feat(driver-mongodb): index lookup joins off the canonical `reference` key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `syncCollectionSchema`'s field-level join-index arm gated on `field.reference_to` — a rejected alias this driver's own schema door refuses outright. The `lookup` conjunct was therefore unreachable for every possible input, and no authored lookup had ever been indexed on MongoDB. The arm now reads `reference`, the only relationship spelling the spec declares. Verified as a complete case split over the key's value domain, not a sample: every `reference_to` value except `undefined` is refused at the door, and `undefined` is falsy, so the old conjunct could not be satisfied at all. The `user` disjunct needs no relationship key and is unchanged — which is why the feature looked healthy. Re-measured on this tree: 65 lookup fields carrying `reference` across the 52 exported platform objects gain their only join-index mechanism. The refusal door is unchanged — predicate, envelope, placement and instruction. Only the tail of its runtime message moved: it told the reader that renaming the key would not by itself get the field an index, true when written and false now. Three pins flip together, each per the direction it pre-wrote: the recorder pin in mongodb-schema-declared-indexes.test.ts, the real-server twin in mongodb-driver.test.ts, and the no-change control in the part (1) suite. The shipped README and the types.mdx callout both asserted the old behaviour and are corrected to describe the new one. Graded minor: this is a boot-time behaviour change for existing deployments. The changeset carries the operations note — cost, magnitude, and the hybrid-build statement. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L --- ...ongodb-lookup-index-canonical-reference.md | 90 +++++++++++++++++++ content/docs/protocol/objectql/types.mdx | 37 +++++--- packages/drivers/driver-mongodb/README.md | 14 ++- ...mongodb-13222-reference-to-refusal.test.ts | 89 ++++++++++++------ .../driver-mongodb/src/mongodb-driver.test.ts | 42 ++++----- .../mongodb-schema-declared-indexes.test.ts | 86 +++++++++--------- .../driver-mongodb/src/mongodb-schema.ts | 69 ++++++++++---- 7 files changed, 298 insertions(+), 129 deletions(-) create mode 100644 .changeset/mongodb-lookup-index-canonical-reference.md diff --git a/.changeset/mongodb-lookup-index-canonical-reference.md b/.changeset/mongodb-lookup-index-canonical-reference.md new file mode 100644 index 0000000000..5496100c45 --- /dev/null +++ b/.changeset/mongodb-lookup-index-canonical-reference.md @@ -0,0 +1,90 @@ +--- +"@objectstack/driver-mongodb": minor +--- + +feat(driver-mongodb): index `lookup` joins off the canonical `reference` key (#13222) + +`syncCollectionSchema`'s field-level join-index arm gated on `field.reference_to`. +That is a REJECTED ALIAS — `FieldSchema` answers `unrecognized_keys` for it on any +field type — and this driver's own schema door refuses it outright. So the arm's +`lookup` conjunct could not be satisfied by any input at all, and no authored +lookup had ever been indexed on MongoDB. The arm now reads `reference`, the only +relationship spelling the spec declares. + +Measured as a complete case split over the key's value domain rather than a +sample: every `reference_to` value except `undefined` is refused at the door, and +`undefined` is falsy, so the old conjunct was unreachable for every possible +input. The `user` disjunct was unaffected and is why the feature looked healthy — +it needs no relationship key, so `idx_owner_lookup`-shaped indexes were always +created. + +The refusal door itself is unchanged: predicate, `VALIDATION_ERROR`/400 envelope, +placement and instruction are all as they were. Only the tail of its message +moved, because it told the reader that renaming the key would not by itself get +the field an index — true when written, false now. + +A `lookup` that declares no `reference` is still not indexed. Measured on +`FieldSchema`: `{ type: 'lookup' }` and `{ type: 'lookup', reference: '' }` both +parse successfully — the spec's prose calls `reference` required for these types +but the schema does not enforce it — so this is a real authorable shape, and an +index for a join with no declared target would cost every write and buy no read. +`master_detail` and `tree` are unchanged: they reach this arm on neither spelling. + +## ⚠️ OPERATORS — this changes boot behaviour on existing deployments + +**What it costs.** The first `syncSchema` after this upgrade CREATES these +indexes on collections that already hold data. Each one is an awaited +`createIndex`: a full collection scan plus an external sort of the extracted +keys, followed by permanent index storage and a small write amplification on +every subsequent insert/update of the indexed field. ("Awaited" describes the +driver, not the server — see the build-feature note below for what the server +does during it.) Later boots are free — +`createIndex` is idempotent for an index that already exists, and this driver +already relies on that. + +⚠️ **The builds are SERIALIZED, so the times ADD.** `syncCollectionSchema` awaits +`createIndex` once per index in a sequential loop, and `syncSchemasBatch` awaits +`syncSchema` once per object the same way. Startup is extended by the SUM of +every build, not by the slowest one. This is the figure to plan the maintenance +window around. + +**How much.** Measured on this tree: **65 lookup fields carrying `reference` +across the 52 exported platform objects** — every one gains an index, and this is +the floor, not the total. Add the objects of any enabled plugin (`plugin-security` +14 lookup fields, `plugin-approvals` 13, `plugin-audit` 6, `plugin-sharing` 4) and +one index per lookup field on your own authored objects. + +Per index, as an order-of-magnitude planning figure and **not** a benchmark — get +your own numbers before sizing a window, because they depend entirely on document +count, storage and cache: + +| collection size | build time, one index | index storage added | +|---|---|---| +| empty / a few thousand docs | effectively instant (metadata-only) | negligible | +| ~1M docs | seconds | tens of MB | +| ~10M docs | ~a minute | hundreds of MB | +| ~100M docs | tens of minutes | a few GB | + +Most `sys_*` collections are small and will finish instantly. Budget for the ones +that accumulate: `sys_metadata_history`, `sys_metadata_audit`, `sys_notification`, +`sys_email`, `sys_session`, and any audit-log object. Count them first +(`db..estimatedDocumentCount()`) and multiply by the lookup fields +each carries; watch a live boot with `db.currentOp()`. + +**Build feature — hybrid builds, unconditionally.** `@objectstack/driver-mongodb` +depends on `mongodb@^7.5.0`, whose own compatibility statement is "the driver +currently supports 4.2+ servers". MongoDB 4.2 is exactly the release that made +index builds hybrid, so **every server version this driver can connect to builds +these indexes with the hybrid builder**: an exclusive lock is taken only briefly +at the start and end of each build, and the collection accepts reads AND writes +throughout the rest of it. This is not a full write stall. Two caveats that +remain: the brief exclusive lock at each end is real, and on a replica set the +build runs on every member. + +**To take the cost outside the boot window,** create the indexes ahead of the +upgrade, with the names and specs the driver uses — `{ name: 'idx_FIELD_lookup' }` +over `{ FIELD: 1 }`, no `unique` and no `sparse`. The driver's `createIndex` is +then a no-op and startup is unaffected. Matching the options matters: a +pre-existing index of the same name with different options raises +`IndexOptionsConflict`, which this driver deliberately swallows and skips, leaving +your index in place unchanged. diff --git a/content/docs/protocol/objectql/types.mdx b/content/docs/protocol/objectql/types.mdx index a79d8c84dd..262d74c55d 100644 --- a/content/docs/protocol/objectql/types.mdx +++ b/content/docs/protocol/objectql/types.mdx @@ -731,23 +731,34 @@ contacts: A relationship field authored with `reference:` gets **no database-level -`FOREIGN KEY` constraint** and **no MongoDB join index**. Both gaps have one -root cause: the driver branches that would have built them were gated on -`reference_to` — a key the spec REFUSES (`FieldSchema` answers -`unrecognized_keys` for it, on any field type) and one that `reference` never -populates. `master_detail` / `tree` did not reach either branch at all. +`FOREIGN KEY` constraint** on any driver. It does get a **MongoDB join index**. +Both branches were once gated on `reference_to` — a key the spec REFUSES +(`FieldSchema` answers `unrecognized_keys` for it, on any field type) and one +that `reference` never populates — so neither fired for authored metadata; the +two were then settled in opposite directions, on their own merits. - **SQL:** the `FOREIGN KEY` DDL is retired (#11567). A field that still carries `reference_to` when it reaches DDL is refused at the driver's door, in the schema's own words (`400 VALIDATION_ERROR`), instead of silently changing the - physical schema. -- **MongoDB:** the field-level join index `idx_FIELD_lookup` is gated on that - same refused key, so a canonically-spelled `reference` lookup is **not - indexed**. A `user` field still is — that arm needs no relationship key, which - is why the feature looked healthy. `reference_to` is refused at this driver's - door too, with the same verdict (#13222); whether a canonical `reference` - lookup should start building the index is tracked separately, because it - changes boot behaviour for deployments that already hold large collections. + physical schema. Declare an index in `indexes[]` if you want one on the + foreign-key column. +- **MongoDB:** a `lookup` field declaring `reference:` gets the field-level join + index `idx_FIELD_lookup` (#13222). A `user` field gets one too — that arm + needs no relationship key. `reference_to` is refused at this driver's door as + well, with the same verdict as the spec's. + + ⚠️ **Upgrading an existing MongoDB deployment:** this index is newer than the + driver, so the first `syncSchema` after the upgrade BUILDS it across + collections that already hold data — a real one-off IO and time cost on large + collections, and index storage that persists. The driver's `CHANGELOG.md` + entry for the release that added it carries the operational detail. Later + boots are no-ops: `createIndex` is idempotent for an index that already + exists. + +A `lookup` that omits `reference:` gets no join index — there is no declared +target to join to. `master_detail` and `tree` reach neither branch: they get no +`FOREIGN KEY` and no join index. Declare an `indexes[]` entry if a +`master_detail` child is queried by parent often enough to need one. Referential integrity is enforced by the **engine** instead: `deleteBehavior` is applied on delete, which is what produces the `409 DELETE_RESTRICTED` above. diff --git a/packages/drivers/driver-mongodb/README.md b/packages/drivers/driver-mongodb/README.md index 77f6f603b2..41c5e61b08 100644 --- a/packages/drivers/driver-mongodb/README.md +++ b/packages/drivers/driver-mongodb/README.md @@ -163,9 +163,14 @@ object's `indexes[]` — the one surface an index is declared on (a field-level `indexed` flag is not a `FieldSchema` key and never built an index, #2377 / #6810). -⚠️ Lookup fields are **not** indexed today. The lookup arm gates on -`reference_to`, a spelling `FieldSchema` refuses, so a canonically-spelled -`reference` lookup gets no join index — see #13222, which owns that fix. +A `lookup` field that declares `reference` also indexes itself, as +`idx_FIELD_lookup`, for join performance; so does a `user` field. A `lookup` +with no `reference` does not — there is no declared target to join to. + +⚠️ **Upgrading:** this join index is newer than the driver. The first +`syncSchema` after the upgrade builds it across collections that already hold +data, which costs real IO and time on large ones. See `CHANGELOG.md` for the +operational detail. Later boots are no-ops. ```typescript await driver.syncSchema('account', { @@ -177,7 +182,8 @@ await driver.syncSchema('account', { }, indexes: [{ fields: ['email'] }], }); -// Creates: idx_id_unique, idx_name_unique, idx_email +// Creates: idx_id_unique, idx_created_at, idx_updated_at, +// idx_name_unique, idx_company_id_lookup, idx_email ``` ### Aggregation diff --git a/packages/drivers/driver-mongodb/src/mongodb-13222-reference-to-refusal.test.ts b/packages/drivers/driver-mongodb/src/mongodb-13222-reference-to-refusal.test.ts index da1a9cd486..353fe98236 100644 --- a/packages/drivers/driver-mongodb/src/mongodb-13222-reference-to-refusal.test.ts +++ b/packages/drivers/driver-mongodb/src/mongodb-13222-reference-to-refusal.test.ts @@ -19,13 +19,14 @@ // module between two suites that pin OPPOSITE halves of one arm would couple // them for no gain. // -// ⛔ NOT pinned here: whether a canonically-spelled `reference` lookup should -// GET `idx_FIELD_lookup`. That is part (2) of #13222 — a separate, still-open -// ruling (it is a boot-time behaviour change for existing deployments: index -// builds on large collections). The last case below is this PR's own NO-CHANGE -// control for it, and is expected to flip in the PR that takes part (2), -// alongside `mongodb-schema-declared-indexes.test.ts`'s #12252 pin, which owns -// the fact. +// Part (2) of #13222 has since been ruled and taken: the join-index arm now +// gates on the canonical `reference`, so a lookup an author can really publish +// GETS `idx_FIELD_lookup`. The last case below was this file's NO-CHANGE +// control for that question and has flipped accordingly, in the same stroke as +// `mongodb-schema-declared-indexes.test.ts`'s #12252 pin and the real-server +// pin in `mongodb-driver.test.ts`. It still belongs here: it is what proves the +// door and the arm read DIFFERENT keys and disagree about them on purpose — +// `reference_to` refused, `reference` indexed, from one call. import { describe, it, expect } from 'vitest'; import type { Db } from 'mongodb'; @@ -187,40 +188,72 @@ describe('#13222 part (1) — driver-mongodb refuses `reference_to` at the schem expect(names(created)).toEqual(['idx_id_unique', 'idx_created_at', 'idx_updated_at']); }); - it('leaves the join-index arm exactly as it was — part (2) is NOT taken here', async () => { - // ⚠️ THE NO-CHANGE CONTROL for this PR, and load-bearing in both directions. + it('refuses `reference_to` and INDEXES `reference` — two keys, two answers, one call', async () => { + // ⚠️ THE PAIRED CONTROL for the door, and load-bearing in both directions. // - // Positive half: a `user` field still gets `idx_owner_id_lookup`, which - // proves the arm still executes and that the harness is wired to something — - // without it the negative half below would pass just as happily against a - // function that created no indexes at all. + // The door refuses one relationship spelling; the arm indexes the other. A + // suite that only ever proves the refusal cannot tell "the door works" from + // "syncCollectionSchema throws on everything", so the two answers are taken + // from ONE call here on purpose. // - // Negative half: a canonically-spelled `reference` lookup still gets NO join - // index. That is the divergence #12252 pinned and part (2) of #13222 owns. - // ⛔ This case records what the driver DOES, not what it should do: the door - // added in this PR makes the arm's `field.reference_to` conjunct unreachable - // but deliberately does not delete it, because deleting it would start - // building indexes on existing deployments' large collections — an unruled - // behaviour change. When part (2) lands, this case is expected to flip to - // `toContain`, in the same stroke as the #12252 pin in - // `mongodb-schema-declared-indexes.test.ts`. + // `user` half: still `idx_owner_id_lookup`, and still the unconditional + // disjunct — it proves the arm executed and names the shape the assertion + // below is spelled in, so neither half can pass vacuously. // - // Bound through a variable rather than written inline: the driver's own - // `FieldDef` declares no `reference` key, so a fresh object literal carrying - // it trips TypeScript's excess-property check. - const canonicalLookup = { type: 'lookup', reference: 'company' }; - + // `reference` half: FLIPPED by part (2) of #13222. This case used to assert + // the canonical lookup got NO index — the divergence #12252 pinned, which + // held because the arm gated on `reference_to`, a key this very door + // refuses, making the conjunct unreachable for every input. The arm now + // gates on `reference`, so the lookup is indexed. const { db, created } = fakeDb(); await syncCollectionSchema(db, 'lead', { name: 'lead', - fields: { company_id: canonicalLookup, owner_id: { type: 'user' } }, + fields: { + company_id: { type: 'lookup', reference: 'company' }, + owner_id: { type: 'user' }, + }, }); + // Exact set, in creation order — closes the vacuity routes a `toContain` + // pair leaves open: an index appearing under another name, the two lookup + // indexes swapping fields, or a stray fourth index nobody declared. expect(names(created)).toEqual([ 'idx_id_unique', 'idx_created_at', 'idx_updated_at', + 'idx_company_id_lookup', 'idx_owner_id_lookup', ]); }); + + it('does not index a `lookup` that declares no target — `reference` is read for truth, not presence', async () => { + // Measured on `FieldSchema` built from this tree: `{ type: 'lookup' }` with + // no `reference`, and `{ type: 'lookup', reference: '' }`, BOTH parse + // successfully — the spec's prose calls `reference` required for these + // types, but the schema does not enforce it. So this is a shape an author + // can really publish, not a hypothetical, and the arm has to answer for it. + // + // It answers by declining: `idx_FIELD_lookup` exists to serve a join, and a + // lookup with no declared target has no join to serve — the index would + // cost every write and buy no read. This is why the arm gates on + // truthiness and not on `!== undefined` like the door above does; the two + // predicates differ deliberately, because they are answering different + // questions about different keys. + for (const target of [undefined, '']) { + const { db, created } = fakeDb(); + await syncCollectionSchema(db, 'lead', { + name: 'lead', + fields: { company_id: { type: 'lookup', reference: target }, owner_id: { type: 'user' } }, + }); + + // The `user` control fires, so the arm ran and the zero below is a real + // zero rather than a harness that called nothing. + expect(names(created)).toEqual([ + 'idx_id_unique', + 'idx_created_at', + 'idx_updated_at', + 'idx_owner_id_lookup', + ]); + } + }); }); diff --git a/packages/drivers/driver-mongodb/src/mongodb-driver.test.ts b/packages/drivers/driver-mongodb/src/mongodb-driver.test.ts index 8f0b120a0b..fb67a16798 100644 --- a/packages/drivers/driver-mongodb/src/mongodb-driver.test.ts +++ b/packages/drivers/driver-mongodb/src/mongodb-driver.test.ts @@ -362,31 +362,31 @@ describe.skipIf(!sharedMongod)('MongoDBDriver', () => { expect(indexNames).toContain('idx_name_unique'); expect(indexNames).toContain('idx_email'); /** - * ⚠️ [#12252] DIVERGENCE PINNED, DISPOSITION OPEN (#13222) — a - * canonically-spelled lookup gets NO join index here. + * ⚠️ DIVERGENCE RETIRED (#13222) — a canonically-spelled lookup gets its + * join index here, verified against a REAL server rather than a recorder. * - * This fixture used to spell the field `reference_to: 'company'` and - * assert `idx_company_id_lookup` was CREATED. `reference_to` is a key - * `FieldSchema` REFUSES (`unrecognized_keys`), so the object it described - * was one no author could publish — and the assertion passed only because - * this fixture was the sole thing in the tree reaching the lookup arm of - * `mongodb-schema.ts`, which gates on `field.reference_to` and reads no - * other relationship key. + * This fixture once spelled the field `reference_to: 'company'`. That is + * a key `FieldSchema` REFUSES (`unrecognized_keys`), so the object it + * described was one no author could publish — and the assertion passed + * only because this fixture was the sole thing in the tree reaching the + * lookup arm of `mongodb-schema.ts`, which gated on `field.reference_to` + * and read no other relationship key. Correcting the spelling (#13224) + * therefore did not leave the outcome alone: the index disappeared, and + * the line was inverted to record that, with the disposition left open. * - * So correcting the spelling does not leave the outcome alone. Measured - * differentially against the real `syncCollectionSchema`, - * `idx_company_id_lookup` is the ONE index that disappears; a - * `type: 'user'` field still gets its index, so the gate is live rather - * than dead code. The consequence in production is that EVERY authored - * lookup on MongoDB is unindexed — 57/57 relationship fields across the - * 44 exported platform objects (#13222). + * #13222 settled the disposition in two parts: `reference_to` is now + * refused at the driver's door, and the arm gates on the canonical + * `reference`. So the line is inverted back — by a ruling, not by a + * re-baseline. * - * ⛔ This records what the driver DOES, not what it SHOULD do. Whether - * the lookup arm learns to read `reference` is #13222's to settle, ⛔ not - * this pin's — when it lands, this line flips back to `toContain` - * deliberately rather than the divergence reopening in silence. + * ⚠️ This copy is the one that runs against real MongoDB, which is what + * it is FOR: it proves the server actually materialized the index under + * that name, not merely that the driver asked for it. The recorder-driven + * twin in `mongodb-schema-declared-indexes.test.ts` is the copy that runs + * on every ordinary CI lane — this suite is `describe.skipIf(!sharedMongod)`. + * TWIN PIN — edit either, edit both. */ - expect(indexNames).not.toContain('idx_company_id_lookup'); + expect(indexNames).toContain('idx_company_id_lookup'); }); it('should be idempotent (safe to call multiple times)', async () => { diff --git a/packages/drivers/driver-mongodb/src/mongodb-schema-declared-indexes.test.ts b/packages/drivers/driver-mongodb/src/mongodb-schema-declared-indexes.test.ts index a426585d4e..e6a93a87eb 100644 --- a/packages/drivers/driver-mongodb/src/mongodb-schema-declared-indexes.test.ts +++ b/packages/drivers/driver-mongodb/src/mongodb-schema-declared-indexes.test.ts @@ -162,72 +162,68 @@ describe('#6810 — syncCollectionSchema materializes declared indexes[]', () => }); }); -describe('#12252 — the field-level lookup arm, on a lane that actually runs', () => { - it('gives a canonically-spelled `lookup` NO join index, while a `user` field still gets one', async () => { - // ⚠️ [#12252] DIVERGENCE PINNED, DISPOSITION OPEN (#13222). +describe('the field-level lookup arm, on a lane that actually runs', () => { + it('gives a canonically-spelled `lookup` its join index, alongside the `user` field', async () => { + // ⚠️ DIVERGENCE RETIRED — this line was inverted, and is now inverted back. // - // `mongodb-schema.ts`'s field-level join-index arm gates on - // `field.reference_to` and reads no other relationship key. `reference` is - // the CANONICAL spelling — `reference_to` is a key `FieldSchema` REFUSES - // (`unrecognized_keys`) — so a lookup any author could actually publish - // reaches that arm and falls straight through it: no `idx_company_id_lookup` - // is ever created. That is the divergence, and #13222 owns whether and how - // it closes. - // - // ⛔ This records what the driver DOES, not what it SHOULD do. When #13222 - // teaches the arm to read `reference`, THIS assertion is expected to flip to - // `toContain` — deliberately, in that PR, as the signal to retire the - // divergence note here. Going red is the whole point of the line: it is what - // stops the divergence closing (or widening) in silence. + // The history is the reason this case is worth its length. The arm used to + // gate on `field.reference_to`, and `reference` — the CANONICAL and only + // spelling `FieldSchema` declares — was read nowhere in the driver. So a + // lookup any author could actually publish reached the arm and fell + // straight through it: `idx_company_id_lookup` was never created, for any + // authored object, on any deployment. #12252 pinned that as a divergence + // with the disposition open, and #13222 carried the disposition. Part (2) + // of its ruling repointed the predicate at `reference`, which is what this + // assertion now records. // // TWIN PIN — edit either, edit both. The same fact is pinned against a real - // server by #13224, which corrects the `reference_to` fixture in - // `mongodb-driver.test.ts` (`describe('syncSchema')` -> 'should create - // collection and indexes') and inverts its `idx_company_id_lookup` - // assertion in place. That suite is `describe.skipIf(!sharedMongod)`, opt-in - // behind `OS_TEST_MONGODB_MEMORY_SERVER_ENABLED=1` because of #5517's ~123 MB - // binary download, so it runs on no ordinary CI lane — which is why the fact - // is asserted here too. THIS copy is the one that runs. + // server in `mongodb-driver.test.ts` (`describe('syncSchema')` -> 'should + // create collection and indexes'). That suite is + // `describe.skipIf(!sharedMongod)`, opt-in behind + // `OS_TEST_MONGODB_MEMORY_SERVER_ENABLED=1` because of #5517's ~123 MB + // binary download, so it runs on no ordinary CI lane — which is why the + // fact is asserted here too. THIS copy is the one that runs. // - // ⚠️ The `user` half is the LOAD-BEARING control, not decoration. - // `field.type === 'user'` is the arm's unconditional disjunct, so its index - // proves the arm executed, that the harness really called the function, and - // that `idx__lookup` is still the name it builds. Without it, - // `not.toContain` would pass just as happily against a function that created - // no indexes at all, a renamed index, or a harness wired to nothing — the - // exact vacuity that makes a negative assertion worthless. - - // Bound through a variable rather than written inline: the driver's own - // `FieldDef` declares only `reference_to`, so a fresh object literal - // carrying `reference` trips TypeScript's excess-property check — on the - // very key this case exists to record the driver does not read. - const canonicalLookup = { type: 'lookup', reference: 'company' }; + // ⚠️ The `user` half stays the LOAD-BEARING control, not decoration. + // `field.type === 'user'` is the arm's unconditional disjunct: it fired + // before this change and after it, so it is the fixed point that tells a + // real flip apart from the arm having been broken in some new way. If the + // predicate were repointed at a key nothing supplies, the `user` index + // would still appear and only the `company_id` one would vanish. const { db, created } = fakeDb(); await syncCollectionSchema(db, 'lead', { name: 'lead', fields: { - company_id: canonicalLookup, + company_id: { type: 'lookup', reference: 'company' }, owner_id: { type: 'user' }, }, }); - // Positive control: the unconditional disjunct fired, under the name the - // negative assertion below is spelled with. + // The control: the unconditional disjunct fired, under the name the + // assertion below is spelled with. const owner = byName(created, 'idx_owner_id_lookup'); expect(owner).toBeDefined(); expect(owner!.spec).toEqual({ owner_id: 1 }); - // The divergence itself. - expect(names(created)).not.toContain('idx_company_id_lookup'); - - // Exact set — closes the remaining vacuity routes in one line: a lookup - // index appearing on `company_id` under ANY other name, the `user` index - // being renamed, or the core set drifting. + // The fact this case owns: the canonical lookup is indexed, on its own + // field, ascending — the spec is asserted, not just the name. + const company = byName(created, 'idx_company_id_lookup'); + expect(company).toBeDefined(); + expect(company!.spec).toEqual({ company_id: 1 }); + // A join index, never a constraint: `unique`/`sparse` belong to the unique + // arm, and a lookup silently acquiring them would forbid two records + // pointing at the same parent. + expect(company!.options).toEqual({ name: 'idx_company_id_lookup' }); + + // Exact set — closes the remaining vacuity routes in one line: an extra + // index nobody declared, the two lookup indexes swapping fields, or the + // core set drifting. expect(names(created)).toEqual([ 'idx_id_unique', 'idx_created_at', 'idx_updated_at', + 'idx_company_id_lookup', 'idx_owner_id_lookup', ]); }); diff --git a/packages/drivers/driver-mongodb/src/mongodb-schema.ts b/packages/drivers/driver-mongodb/src/mongodb-schema.ts index fd7d19a33d..a03821f443 100644 --- a/packages/drivers/driver-mongodb/src/mongodb-schema.ts +++ b/packages/drivers/driver-mongodb/src/mongodb-schema.ts @@ -52,6 +52,27 @@ interface FieldDef { * no information the door needs. */ reference_to?: unknown; + /** + * The CANONICAL relationship key — the only spelling `@objectstack/spec` + * declares (`FieldSchema.reference`), and the one the join-index arm in + * {@link syncCollectionSchema} gates on. + * + * Typed `unknown` for the same reason its rejected sibling above is: the + * metadata reaching this seam went around Zod — `MongoDBDriver.syncSchema( + * object, schema: unknown)` casts and forwards it verbatim — so `string` + * would be a claim about untrusted input nothing on this path checked. The + * arm reads TRUTHINESS and nothing else; the value is never dereferenced. + * + * ⚠️ Truthiness rather than `!== undefined`, and that difference is load + * bearing rather than inherited. Measured on `FieldSchema` built from this + * tree: `{ type: 'lookup' }` with NO `reference`, and `{ type: 'lookup', + * reference: '' }`, both parse SUCCESSFULLY — the "required for these types" + * in the spec's own prose is not enforced by the schema. So a lookup that + * points nowhere is a shape an author can really publish, and it must not + * get `idx_FIELD_lookup`: an index for a join whose target is undeclared + * costs writes and buys no read. Truthiness declines exactly that shape. + */ + reference?: unknown; multiple?: boolean; } @@ -151,18 +172,21 @@ interface ObjectDef { * fix is a one-word rename — the same envelope every other refusal in this * package speaks. * - * ## ⛔ What this deliberately does NOT change + * ## ⛔ What this door does NOT decide + * + * Whether a canonically-spelled `reference` lookup GETS `idx_FIELD_lookup` is a + * separate question from whether the rejected alias is refused, and it was + * ruled separately. This door refuses; the arm below indexes. Part (2) of the + * ruling repointed that arm's predicate from `field.reference_to` to + * `field.reference` and touched nothing here — the refusal's predicate, + * envelope, placement and instruction are unchanged by it. * - * The field-level join-index arm below is left BYTE-IDENTICAL, and its - * `field.reference_to` conjunct is now unreachable — not oversight. Deleting - * that conjunct would make a canonically-spelled `reference` lookup start - * building `idx_FIELD_lookup`, which is a behaviour change for existing - * deployments (index builds on large collections) and is a SEPARATE, still-open - * ruling — part (2) of #13222, which the maintainer carries in a later batch. - * Whoever takes that ruling owns the arm, its comment, and the `#12252` pin in - * `mongodb-schema-declared-indexes.test.ts` in one stroke. Until then the arm's - * observable behaviour is exactly what it was: a `user` field is indexed, a - * canonical `reference` lookup is not. + * What part (2) DID change in this comment is the tail of the runtime message + * below, which until then told the reader that renaming the key would not, by + * itself, get the field an index. That was true when it was written and is + * false now: `reference` is exactly what the arm reads. A refusal that hands + * the caller a stale claim about what the fix achieves is a worse refusal, so + * the sentence moved with the behaviour it described. */ function refuseRejectedReferenceAlias(collectionName: string, fieldName: string): never { const err = new Error( @@ -172,10 +196,9 @@ function refuseRejectedReferenceAlias(collectionName: string, fieldName: string) `\`FieldSchema\` refuses this key with that same verdict (\`unrecognized_keys\`) on ANY field ` + `type — so a field still carrying it at schema-sync time went around the schema ` + `(\`syncSchema(object, schema: unknown)\` casts and forwards it verbatim, with no Zod). ` + - `Rename the key. Note that renaming does not, by itself, get the field a join index: this ` + - `driver's join-index arm still reads the refused spelling, so a canonical \`reference\` lookup ` + - `is unindexed here. That is a separate, still-open question — deliberately unchanged by the ` + - `door you just hit — and not something the rename above regresses.`, + `Rename the key. \`reference\` is also the spelling this driver's join-index arm reads, so a ` + + `\`lookup\` field declaring it gets \`idx_FIELD_lookup\` on the next schema sync — the rename ` + + `fixes the refusal and gets the join index in one step.`, ) as Error & { code?: string; status?: number }; err.code = StandardErrorCode.enum.VALIDATION_ERROR; err.status = 400; @@ -230,10 +253,20 @@ export async function syncCollectionSchema( } // Lookup + user (a lookup specialized to sys_user) fields get an index for - // join performance. A `user` field always references sys_user, so it is - // indexed even when reference_to is not explicitly set. + // join performance, gated on the CANONICAL relationship key `reference`. + // A `user` field always references sys_user, so it needs no relationship + // key at all and is indexed unconditionally. + // + // This arm read `field.reference_to` until part (2) of the ruling below. + // That key is a REJECTED ALIAS the door above refuses outright, so the + // conjunct was unreachable and NO authored lookup was ever indexed here — + // measured as a complete case split over the key's value domain, not a + // sample: every value except `undefined` is refused at the door, and + // `undefined` is falsy, so the conjunct could not be satisfied by any + // input. `reference` is the only relationship spelling `FieldSchema` + // declares, so this is the predicate that reaches authored metadata. if ( - (field.type === 'lookup' && field.reference_to) || + (field.type === 'lookup' && field.reference) || field.type === 'user' ) { indexOps.push({