dev/MultiforaDB
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

M3: design review of the index types, and the partialFilterExpression refusal #10

Merged
dev merged 2 commits from m3-index-types into main 2026-08-10 19:19:07 +00:00
Conversation 0 Commits 2 Files Changed 3 +235
3 changed files with 235 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
PLAN.md
View File

@@ -1123,6 +1123,32 @@ has to be its own commit with its own re-recorded scorecard.
agrees. Worth one commit, and it needs its own measurements first: the
padding case (`y.3.b` past the end) is a *legal* creation on mongod, so
the fix is not "refuse a non-document element".
- **`partialFilterExpression` was accepted and ignored**, and that made
`unique` mean the wrong thing. An index built over every document instead of
the filtered subset still answers reads correctly -- it holds a superset,
never a subset -- but a *unique* one then rejects inserts mongod accepts:
```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
// ours: E11000 duplicate key error
```
Unique-within-a-subset is the whole point of the option, so every use of it
was a legal insert refused. Refused at creation now, on the same judgement
`cmd_update` already makes about an update spec's `sort`. Nothing in the
repository covered this row: the pinned suite is crud and aggregate, and
neither `e2e5.js` nor `e2e6.js` writes a partial or hashed spec. Hashed, by
contrast, was honestly missing -- refused, with the wrong code (2 where
mongod says 67) but the right answer.
`docs/M3_INDEX_TYPES_DESIGN_REVIEW.md` measures both features' rules and
argues the planner rule that decides the design: a partial index may only
answer a query whose predicates *imply* its filter, so until that test
exists the safe rule is to maintain the index and never read from it. Too
few documents is the one failure worse than no index at all.
- **M3's second corpus is `tests/spec/operators/`.** The eight operators PLAN
§3 names all answered `bad update` with code 2, one message for every
question — and `$push`'s `$slice`, `$position` and `$sort` were parsed,

159
docs/M3_INDEX_TYPES_DESIGN_REVIEW.md Normal file
View File

@@ -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.

50
src/commands.zig
View File

@@ -934,6 +934,35 @@ fn cmd_create_indexes(ctx: *Context, msg: *wire.Message, reply: *wire.Reply) !vo
if (name == .string and std.mem.eql(u8, name.string, "_id_")) {
return bad_value(reply, "cannot create index with name '_id_'");
}
// Accepted and ignored until this commit, which is the worst of the
// three possible answers -- the same judgement `cmd_update` already
// makes about an update spec's `sort`.
//
// An index built over every document instead of the filtered subset
// still answers reads correctly: it holds a superset, never a subset.
// `unique` is where that stops being true. Measured on mongod 8.3.7:
//
// createIndex({a: 1}, {unique: true, partialFilterExpression: {t: true}})
// insertMany([{a: 1, t: false}, {a: 1, t: false}])
//
// mongod accepts both -- neither document is in the index, so neither
// collides -- and this server answered E11000. Unique-within-a-subset
// is the whole point of the option, so every use of it was a legal
// insert refused. The other two answers available were to keep doing
// that, or to echo the option back from `listIndexes` while not
// honouring it, which is a larger lie than saying no.
//
// See docs/M3_INDEX_TYPES_DESIGN_REVIEW.md. The implementation is the
// rest of M3's last row; this is what stands in until then.
if (bson.get_pair(spec, "partialFilterExpression") != null) {
return reply.put_error(
@intFromEnum(ErrorCode.cannot_create_index),
"CannotCreateIndex",
"partialFilterExpression is not implemented by this server: it would be " ++
"accepted and ignored, and a unique index would then be enforced over " ++
"documents the filter excludes",
);
}
const spec_doc = bson.Document{ .arena = undefined, .pairs = spec };
_ = ctx.engine.create_index(db_name, coll_name, &spec_doc) catch |err| switch (err) {
@@ -5655,6 +5684,23 @@ test "TTL index round-trips through createIndexes/listIndexes; bad specs give 67
.{ .key = "key", .value = .{ .doc = &.{.{ .key = "_id", .value = .{ .int32 = 1 } }} } },
.{ .key = "expireAfterSeconds", .value = .{ .int32 = 60 } },
} } },
// Same rule, different option: a partial filter accepted and ignored
// would enforce `unique` over documents the filter excludes.
.{ .code = 67, .spec = .{ .doc = &.{
.{ .key = "key", .value = .{ .doc = &.{.{ .key = "a", .value = .{ .int32 = 1 } }} } },
.{ .key = "partialFilterExpression", .value = .{ .doc = &.{
.{ .key = "t", .value = .{ .bool = true } },
} } },
} } },
// And with `unique`, which is the combination that made it a wrong
// answer rather than only a missing one.
.{ .code = 67, .spec = .{ .doc = &.{
.{ .key = "key", .value = .{ .doc = &.{.{ .key = "b", .value = .{ .int32 = 1 } }} } },
.{ .key = "unique", .value = .{ .bool = true } },
.{ .key = "partialFilterExpression", .value = .{ .doc = &.{
.{ .key = "t", .value = .{ .bool = true } },
} } },
} } },
};
for (bad) |case| {
var ctx = tdb.ctx(io);
@@ -5668,6 +5714,10 @@ test "TTL index round-trips through createIndexes/listIndexes; bad specs give 67
try dispatch(&ctx, &msg, &reply);
try testing.expectEqual(case.code, bson.get_pair(reply.pairs.items, "code").?.int32);
}
// Mutation check for the two `partialFilterExpression` rows: delete the
// guard in `cmd_create_indexes` and both go green on the code -- and the
// second one's index then refuses `{a: 1, t: false}` twice, which mongod
// accepts because neither document is in the index at all.
// Nothing partial was registered by the rejected specs.
try testing.expectEqual(@as(usize, 1), tdb.engine.get_collection("test", "sessions").?.indexes.items.len);
}