diff --git a/docs/M3_INDEX_TYPES_DESIGN_REVIEW.md b/docs/M3_INDEX_TYPES_DESIGN_REVIEW.md new file mode 100644 index 0000000..6036680 --- /dev/null +++ b/docs/M3_INDEX_TYPES_DESIGN_REVIEW.md @@ -0,0 +1,159 @@ +# M3 design review — partial and hashed indexes + +The last line of M3's row. Written before any code, like the M2 and +`arrayFilters` reviews, and for the same reason: both times the measurement +disagreed with the plan before the implementation did. + +Everything below was measured on 2026-08-10 against mongod 8.3.7 on `:27099` +and this server at `98fde82` on `:27020`, running the identical probe against +both. + +--- + +## 1. The two halves are not the same kind of gap + +PLAN §3 names them together — "partial + hashed indexes" — as though they were +one item. They are not. + +**Hashed is honestly missing.** `createIndex({a: "hashed"})` is refused: + +``` +mongod: a_hashed +ours: ERR [2] invalid index spec +``` + +Wrong code (mongod says 67 for an unknown key direction, and has four more +specific ones besides), but the right answer. A client is told no and knows +where it stands. Queries against the collection keep working, because there is +simply no index. + +**Partial is accepted and ignored.** + +``` +mongod: listIndexes -> {"key":{"a":1},"name":"a_1","partialFilterExpression":{"a":{"$gte":5}}} +ours: listIndexes -> {"key":{"a":1},"name":"a_1"} +``` + +`createIndex` answers `a_1` and reports success. The index is built over +*every* document rather than the ones the filter selects, `listIndexes` does +not mention the option, and nothing tells the client that the index they asked +for is not the index they have. + +## 2. Ignoring it is not harmless, and here is the row that proves it + +An over-inclusive index still answers queries correctly — it contains a +superset of what it should, never a subset, so a scan through it finds +everything. Reads are safe. That is worth saying plainly, because it is the +reason this is a smaller fire than `arrayFilters` was. + +It is not, however, harmless, and `unique` is where it stops being harmless: + +```js +// createIndex({a: 1}, {unique: true, partialFilterExpression: {t: true}}) +// insertMany([{a: 1, t: false}, {a: 1, t: false}]) + +mongod: accepted -- neither document is in the index, so neither collides +ours: ERR [11000] E11000 duplicate key error ... dup key: {a: 1} +``` + +**A legal insert is refused.** The whole point of a partial unique index is +uniqueness *within a subset*: unique email among active accounts, unique +external id among synced rows. Every one of those is an insert this server +rejects and mongod accepts. The client sees a duplicate-key error naming a +constraint it deliberately scoped away. + +Two smaller divergences ride along: `listIndexes` reports an index +specification that is not the one on disk, so a client comparing specs before +creating decides wrongly; and the index carries every document, which is the +storage the option existed to avoid. + +## 3. What the gate can see + +Nothing. There is no index-management corpus here — the pinned suite this +repo fetches is crud + aggregate, and `tests/e2e/e2e5.js` and `e2e6.js` test +indexes but neither writes a partial or hashed spec. The `unique`-plus-partial +row above is not covered by a single test in the repository, in any suite. + +So M3's last line needs its own recorded corpus, exactly like the positional +operators and the update operators did. `tests/spec/indexes/`, same shape: +inputs authored in `sources/`, expectations measured from mongod, run through +`run.js --suite-dir`. + +## 4. What the fix has to be, measured + +### Partial + +The filter is not an arbitrary query. mongod restricts it, and the restriction +is what makes the planner's job possible: + +| | mongod | +|---|---| +| equality, `$gt`/`$gte`/`$lt`/`$lte`, `$exists: true`, `$type` | allowed | +| `$and`, and `$or` from 8.0 | allowed | +| `$regex` | `Error in specification ... ` (67) | +| `partialFilterExpression` that is not a document | TypeMismatch (14) | +| `partialFilterExpression` **with `sparse`** | (67) — the two may not be combined | + +The planner rule is the load-bearing one and it is not symmetric: a partial +index may only answer a query whose predicates *imply* the filter. Using it +for a query it does not cover returns too few documents, which is the one +failure mode worse than not using an index at all. Until that implication test +exists, the safe planner rule is **never use a partial index for a read** — +maintain it, enforce `unique` through it, and let reads fall back to a scan. +That is a correct server that has not yet earned the speedup, and it is where +this should land first. + +### Hashed + +| | mongod | +|---|---| +| `{a: "hashed"}` | allowed, name `a_hashed` | +| `{a: "hashed", b: 1}` | allowed — one hashed component beside range ones | +| `{a: "hashed", b: "hashed"}` | 31303, "A maximum of one index field is allowed to be hashed" | +| `unique` on a hashed index | 16764, "Currently hashed indexes cannot guarantee uniqueness" | +| an array value at the hashed path | 16766, **at insert time**, not at creation | +| `{a: "bogus"}` | 67, "Unknown index plugin" | + +A hashed index answers equality only; mongod still returns correct results for +a range query or a sort over a hashed field by not using the index. So the +planner rule is the mirror of the partial one and just as conservative: +equality predicates only, everything else scans. + +The hash itself need not match mongod's. Nothing a client can observe depends +on the value — `listIndexes` reports `"hashed"`, not a hash — so this is a +private encoding choice, and the ordering of the tree is then meaningless, +which is exactly why sorts may not use it. + +## 5. The order this should land in + +The same shape the `arrayFilters` work took, and for the same reason: the +refusal is small, correct on its own, and stops the wrong answer before the +corpus that measures it exists. + +1. **Refuse `partialFilterExpression`.** This repo already has the precedent + and states it in `cmd_update`: an update spec's `sort` is refused because + "ignoring the field would be the worst of the three possible answers — the + client asked for a specific document and would silently get a different + one." Identical logic. The two alternatives are to keep enforcing `unique` + over the wrong set, or to report the option in `listIndexes` while not + honouring it, which is a larger lie than the current one. +2. **Record `tests/spec/indexes/`** against mongod: partial creation rules, + the `unique` subset semantics, hashed creation rules, and the read paths + for both. Red by construction. +3. **Partial indexes**, maintained and `unique`-enforcing, planner declining + to read from them. +4. **Hashed indexes**, equality-only in the planner. +5. **The implication test**, which is what lets a partial index serve a read, + and is a planner change rather than an index one. + +Steps 3 and 4 both need a catalog field. `write_index_catalog` has a `flags` +byte with four bits used, so a fifth can mean "a partial filter follows" and a +sixth "this key is hashed" — old files never set them and read back +identically, so `catalog_version` stays 1. That is the same argument the free +list used for its own format change and it holds here for the same reason. + +## 6. Not covered + +Neither `$or` in a partial filter beyond accepting it, nor `2dsphere`, `text`, +`wildcard` or `collation` — none of them is in M3's row, and each is a +milestone-sized item that would arrive with its own review.