plan/spec: hashed indexes are done, the implication test is next
`tests/spec/indexes/` is 42/42, so all four recorded corpora are green:
positional 51, operators 125, indexes 42, aggregate 70.
Records what implementing the row taught, including the three things the
design review got wrong. Two were already noted when the corpus was
recorded ($in is allowed; a differing filter is 86); the third is new and
cheaper than the review's version: hashed needs no flags bit, because it
belongs to a key *component* and the per-component direction byte was
already there holding 0 or 1.
Also fixes a corpus case that did not measure what it said. "equality
across numeric types" sent an int32, because a source is plain JSON and
`5.0` is `5` after JSON.parse -- it was a second copy of the case above
it. Reading sources as EJSON was tried and reverted, and the recorder now
says why: EJSON's wrapper namespace collides with the query operators
these sources are made of, so `{"$regex": "x"}` became a BSONRegExp,
`structuredClone` flattened it to `{pattern, options}`, and "a filter
using $regex is refused" silently became a filter mongod accepts. The
cross-type property is a unit test instead, which is where it belongs --
it is about how this server hashes, and mongod's hash is a different
function, so a corpus could only ever check the answer.
The case is renamed to what it does measure rather than deleted: two
documents sharing a value is still the read a hashed index exists for.
Verified: 256/256 unit tests in ReleaseFast and ReleaseSafe, 87/87 fuzz,
all four corpora 0 fail, pinned scorecard unchanged at 228/63/196, the
full e2e matrix and crash-fuzz green.
This commit was merged in pull request #13.
This commit is contained in:
67
PLAN.md
67
PLAN.md
@@ -467,7 +467,7 @@ answers, and the trade is only acceptable because the lie is removed first.
|
|||||||
| M1 | **Cursors + wire polish** | getMore / killCursors / batchSize; server-side cursor state with idle timeout; sessions plumbing (lsid accepted) as drivers send it; hello advertisement updates; **`moreToCome` on requests** (see the bug below); command-monitoring assertions in the spec runner | crud spec suite green; e2e green |
|
| M1 | **Cursors + wire polish** | getMore / killCursors / batchSize; server-side cursor state with idle timeout; sessions plumbing (lsid accepted) as drivers send it; hello advertisement updates; **`moreToCome` on requests** (see the bug below); command-monitoring assertions in the spec runner | crud spec suite green; e2e green |
|
||||||
| M2 | **The `aggregate` command surface** | `$out` and `$merge` (7 of the 13 failures), and refusing every pipeline construct the engine does not implement instead of answering `0` (amendment A6). The other 6 failures are blocked on M2.5, M4 and M8 — see `docs/M2_DESIGN_REVIEW.md` §7 | `aggregate-*.json`: 0 fail among the 7 reachable cases |
|
| M2 | **The `aggregate` command surface** | `$out` and `$merge` (7 of the 13 failures), and refusing every pipeline construct the engine does not implement instead of answering `0` (amendment A6). The other 6 failures are blocked on M2.5, M4 and M8 — see `docs/M2_DESIGN_REVIEW.md` §7 | `aggregate-*.json`: 0 fail among the 7 reachable cases |
|
||||||
| M2.5 | **The aggregation engine** | expression evaluator, per-stage document iterator, the accumulators, `$unwind`; `$lookup`/`$facet` explicitly out of the first cut (amendment A6) | a purpose-built stage corpus, every expectation measured against mongod |
|
| M2.5 | **The aggregation engine** | expression evaluator, per-stage document iterator, the accumulators, `$unwind`; `$lookup`/`$facet` explicitly out of the first cut (amendment A6) | a purpose-built stage corpus, every expectation measured against mongod |
|
||||||
| M3 | **Update operators + index types** | `distinct` (**done**); positional paths refused rather than destructive (**done**); `$`/`$[]`/`$[<ident>]` + `arrayFilters` (**done**); $setOnInsert, $addToSet, $mul, $min/$max, $pop, $pullAll, $currentDate + `$push`'s modifiers (**done**); pipeline-style updates (**done**); then partial + hashed indexes | `tests/spec/positional/` 0 fail (51 cases) and `tests/spec/operators/` 0 fail (125 cases), both recorded from mongod — **green**; the named gate could not see either, see below; remaining crud coverage; e2e3/e2e4 green |
|
| M3 | **Update operators + index types** | `distinct` (**done**); positional paths refused rather than destructive (**done**); `$`/`$[]`/`$[<ident>]` + `arrayFilters` (**done**); $setOnInsert, $addToSet, $mul, $min/$max, $pop, $pullAll, $currentDate + `$push`'s modifiers (**done**); pipeline-style updates (**done**); partial indexes (**done**); hashed indexes (**done**); then the implication test that lets a partial index serve a read | `tests/spec/positional/` 0 fail (51), `tests/spec/operators/` 0 fail (125) and `tests/spec/indexes/` 0 fail (42), all recorded from mongod — **green**; the named gate could not see any of the three, see below; remaining crud coverage; e2e3/e2e4 green |
|
||||||
| M4 | **Sessions + transactions** | logical sessions, snapshot isolation on the mmap engine, write concern at commit | sessions + transactions spec suites green |
|
| M4 | **Sessions + transactions** | logical sessions, snapshot isolation on the mmap engine, write concern at commit | sessions + transactions spec suites green |
|
||||||
| M5 | **Change streams** | change feed + resume tokens (likely log-seq based), getMore integration | change-streams spec suite green |
|
| M5 | **Change streams** | change feed + resume tokens (likely log-seq based), getMore integration | change-streams spec suite green |
|
||||||
| M6 | **Admin/ops commands** | dbStats, collStats, serverStatus, ping, buildInfo, listDatabases filters, dropDatabase durability (log it) | mongosh UX smoke; e2e green |
|
| M6 | **Admin/ops commands** | dbStats, collStats, serverStatus, ping, buildInfo, listDatabases filters, dropDatabase durability (log it) | mongosh UX smoke; e2e green |
|
||||||
@@ -1151,7 +1151,35 @@ has to be its own commit with its own re-recorded scorecard.
|
|||||||
to read from a partial index**: it holds a subset, so answering a query from
|
to read from a partial index**: it holds a subset, so answering a query from
|
||||||
it is only correct when the query implies the filter, and that implication
|
it is only correct when the query implies the filter, and that implication
|
||||||
test is the last step of the row. Too few documents is worse than no index.
|
test is the last step of the row. Too few documents is worse than no index.
|
||||||
Left open with it: `hashed.json` is still 0/18.
|
|
||||||
|
**Hashed indexes landed**; `hashed.json` is 18/18, so `tests/spec/indexes/`
|
||||||
|
is 42/42. A hashed component stores a tag byte and a 64-bit hash of the
|
||||||
|
value's *ordinary encoded* bytes — hashing the encoding rather than the
|
||||||
|
value is what makes `{a: 5}` and `{a: 5.0}` one entry for free, because
|
||||||
|
`bson.encode_key` already normalizes numerics through f128 so that the tree
|
||||||
|
can be a memcmp. Collisions are harmless under this file's governing
|
||||||
|
invariant (an index generates candidates; the full filter is re-applied to
|
||||||
|
every one); the single place that would not survive one is uniqueness, which
|
||||||
|
is why `unique` is refused rather than approximated. The planner is the
|
||||||
|
mirror of the partial rule: equality only, because a range or a sort would
|
||||||
|
read a band of leaves ordered by hash, which is an arbitrary set of values.
|
||||||
|
|
||||||
|
**The review's catalog guess was wrong and cost nothing.** It proposed a
|
||||||
|
sixth flags bit for "this key is hashed". Hashed is a property of a *key
|
||||||
|
component*, not of an index — a compound index may hold one hashed
|
||||||
|
component beside range ones — and the per-component direction byte has
|
||||||
|
always been written and has only ever held 0 or 1. A third value costs no
|
||||||
|
format change and `catalog_version` stays 1.
|
||||||
|
|
||||||
|
Recording corrected three more assumptions. `{a: "bogus"}` is 67 with
|
||||||
|
codeName `CannotCreateIndex`, while the two hashed-specific refusals are
|
||||||
|
bare location numbers (31303, 16764) whose codeName is `Location<n>`. An
|
||||||
|
array at a hashed path is 16766 as a per-document **writeError beside
|
||||||
|
`ok: 1`** on an insert or update, and a command error only from
|
||||||
|
`createIndexes` over data that already holds one. And it is refused for a
|
||||||
|
*one-element* array through the path too, which is the case a value count
|
||||||
|
cannot tell apart from a plain subdocument — so the check walks the path
|
||||||
|
rather than counting what it yields.
|
||||||
|
|
||||||
**`tests/spec/indexes/` is the gate**, recorded red at 3/39 across 42 cases.
|
**`tests/spec/indexes/` is the gate**, recorded red at 3/39 across 42 cases.
|
||||||
A case there is a *sequence* -- create, insert, read, list -- because an
|
A case there is a *sequence* -- create, insert, read, list -- because an
|
||||||
@@ -1167,6 +1195,41 @@ has to be its own commit with its own re-recorded scorecard.
|
|||||||
exists the safe rule is to maintain the index and never read from it. Too
|
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.
|
few documents is the one failure worse than no index at all.
|
||||||
|
|
||||||
|
- **What the index corpus caught that was not about indexes: `{a: null}` did
|
||||||
|
not match a missing field.** `find({a: null})` has to match a document with
|
||||||
|
no `a` as well as one holding an explicit null. This server matched only the
|
||||||
|
explicit one — with or without an index — and `{"a.b": null}` matched
|
||||||
|
nothing at all. Two layers already believed otherwise and only the matcher
|
||||||
|
did not: `index.build_entries` stores a missing field as null under a
|
||||||
|
non-sparse index, and `evaluate_index`'s sparse guard exists specifically to
|
||||||
|
stop this query reading an index that skipped those documents, so the guard
|
||||||
|
was defending a behaviour that did not exist. Fixed narrowly, for
|
||||||
|
`$eq`/`$in` and their negations only — a missing field is not null to
|
||||||
|
`$lt`, `$exists` or `$type`. The pinned crud+aggregate scorecard did not
|
||||||
|
move (228/63/196): it does not cover the question.
|
||||||
|
|
||||||
|
Three neighbours were measured at the same time and left alone, each its own
|
||||||
|
item:
|
||||||
|
- **Comparison operators are not type-bracketed.** mongod's `$lt`/`$gt`
|
||||||
|
family only matches values in the same type bracket as the operand; this
|
||||||
|
server compares across the whole BSON order. So `{a: {$lt: 5}}` matches
|
||||||
|
`{a: null}` here and not there, `{a: {$lte: null}}` misses the documents
|
||||||
|
it should match, and `{a: {$not: {$gt: 5}}}` excludes `{a: []}` and
|
||||||
|
`{a: {b: 1}}`. One root cause, four visible divergences, and it is in the
|
||||||
|
shared query path so it is one fix for every command.
|
||||||
|
- **A dotted path through an empty array is treated as absent.**
|
||||||
|
`{"a.b": null}` now matches `{a: []}` here and does not on mongod: an
|
||||||
|
array traversed with no elements yields no values, which mongod does not
|
||||||
|
read as a missing field. Distinguishing the two needs the collector to
|
||||||
|
report that it stepped through an array — the same question
|
||||||
|
`index.array_on_path` answers for hashed components.
|
||||||
|
- **A key-pattern direction may be any non-zero, non-NaN number.** mongod
|
||||||
|
accepts `{a: 2}` and `{a: -0.5}`, takes the sign as the direction and
|
||||||
|
echoes the value back verbatim from `listIndexes`; `{a: 0}` and `{a: NaN}`
|
||||||
|
are 67. This server accepts only ±1 and answers 2 for the rest. Echoing
|
||||||
|
the value back means `IndexKey` would have to carry the raw number, which
|
||||||
|
is why this is not folded into the hashed commit.
|
||||||
|
|
||||||
- **M3's second corpus is `tests/spec/operators/`.** The eight operators PLAN
|
- **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
|
§3 names all answered `bad update` with code 2, one message for every
|
||||||
question — and `$push`'s `$slice`, `$position` and `$sort` were parsed,
|
question — and `$push`'s `$slice`, `$position` and `$sort` were parsed,
|
||||||
|
|||||||
@@ -152,6 +152,32 @@ 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
|
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.
|
list used for its own format change and it holds here for the same reason.
|
||||||
|
|
||||||
|
## Outcome
|
||||||
|
|
||||||
|
Steps 1–4 landed in that order. `tests/spec/indexes/` is 42/42.
|
||||||
|
|
||||||
|
Three things this review got wrong, kept here because the point of writing it
|
||||||
|
before the code was to find out which parts would not survive contact:
|
||||||
|
|
||||||
|
1. **`$in` in a partial filter is allowed.** §4 listed it with `$ne` and
|
||||||
|
`$regex`. Recording it said otherwise.
|
||||||
|
2. **The same key with a different filter is IndexKeySpecsConflict (86)**, not
|
||||||
|
the 67 §4 implied — the filter is part of *which documents* the index is
|
||||||
|
over, not of how it behaves, and mongod splits the two codes on exactly
|
||||||
|
that line.
|
||||||
|
3. **Hashed needs no flags bit.** §5 proposed a sixth one. Hashed belongs to a
|
||||||
|
key *component*, not to an index, and the per-component direction byte the
|
||||||
|
catalog has always written has only ever held 0 or 1 — so a third value
|
||||||
|
costs no format change and `catalog_version` stays 1, which is the same
|
||||||
|
conclusion by a better route.
|
||||||
|
|
||||||
|
And one thing the corpus found that this review had no reason to look for:
|
||||||
|
`find({a: null})` did not match a document with no `a`, index or no index. See
|
||||||
|
PLAN §6.
|
||||||
|
|
||||||
|
Step 5, the implication test, is still open. Until it exists a partial index
|
||||||
|
is maintained, enforces `unique`, and is never read from.
|
||||||
|
|
||||||
## 6. Not covered
|
## 6. Not covered
|
||||||
|
|
||||||
Neither `$or` in a partial filter beyond accepting it, nor `2dsphere`, `text`,
|
Neither `$or` in a partial filter beyond accepting it, nor `2dsphere`, `text`,
|
||||||
|
|||||||
@@ -35,13 +35,21 @@ every case is about which indexes exist, so the recorder drops the collection
|
|||||||
between cases and walks each case's operations in order — stopping at the
|
between cases and walks each case's operations in order — stopping at the
|
||||||
first that throws, which is what a client would see.
|
first that throws, which is what a client would see.
|
||||||
|
|
||||||
|
Sources are plain JSON, not EJSON, and that costs something worth stating: a
|
||||||
|
source cannot name a BSON type the JSON grammar has no syntax for, so `5.0`
|
||||||
|
reaches the driver as an int32. Reading them as EJSON was tried and reverted —
|
||||||
|
EJSON's wrapper namespace collides with the query operators these sources are
|
||||||
|
made of. `{"$regex": "x"}` parses to a `BSONRegExp`, `structuredClone`
|
||||||
|
flattens it to `{pattern, options}`, and "a filter using $regex is refused"
|
||||||
|
silently became a filter mongod accepts.
|
||||||
|
|
||||||
## Where it stands
|
## Where it stands
|
||||||
|
|
||||||
Recorded against mongod 8.3.7 at 3/39 -- red by construction -- and partial
|
Recorded against mongod 8.3.7 at 3/39 -- red by construction -- and both
|
||||||
indexes have since been driven green:
|
halves have since been driven green:
|
||||||
|
|
||||||
```
|
```
|
||||||
hashed.json 0 pass 18 fail 0 skip
|
hashed.json 18 pass 0 fail 0 skip
|
||||||
partial.json 24 pass 0 fail 0 skip
|
partial.json 24 pass 0 fail 0 skip
|
||||||
```
|
```
|
||||||
|
|
||||||
@@ -50,6 +58,11 @@ change: this server indexed every document, so a query still found
|
|||||||
everything, which is the whole reason the review called the partial gap
|
everything, which is the whole reason the review called the partial gap
|
||||||
smaller than the `arrayFilters` one.
|
smaller than the `arrayFilters` one.
|
||||||
|
|
||||||
|
The last one to go green was not about indexes at all. `find({a: null})` has
|
||||||
|
to match a document with no `a`, and this server matched only an explicit
|
||||||
|
null — with or without an index. No other test in the repository asks, and
|
||||||
|
the pinned crud+aggregate scorecard did not move when it was fixed.
|
||||||
|
|
||||||
## What recording it settled
|
## What recording it settled
|
||||||
|
|
||||||
Two of the review's own guesses were wrong, which is why it was recorded
|
Two of the review's own guesses were wrong, which is why it was recorded
|
||||||
@@ -78,3 +91,17 @@ And what it confirmed:
|
|||||||
| a key direction that is not 1, -1 or `"hashed"` | 67 |
|
| a key direction that is not 1, -1 or `"hashed"` | 67 |
|
||||||
| a hashed index beside an ascending one on the same field | both exist |
|
| a hashed index beside an ascending one on the same field | both exist |
|
||||||
| a range query or a sort over a hashed field | still correct — the planner declines the index rather than misusing it |
|
| a range query or a sort over a hashed field | still correct — the planner declines the index rather than misusing it |
|
||||||
|
|
||||||
|
And what implementing hashed then measured, none of which the review had:
|
||||||
|
|
||||||
|
| | mongod |
|
||||||
|
|---|---|
|
||||||
|
| `codeName` for 31303 and 16764 | `Location31303` / `Location16764` — bare location numbers with no name |
|
||||||
|
| `codeName` for `{a: "bogus"}` | `CannotCreateIndex`, the named 67 |
|
||||||
|
| 16766 on an insert or update | a per-document **writeError beside `ok: 1`**, so the rest of the batch lands |
|
||||||
|
| 16766 from `createIndexes` | a command error, over data that already holds an array |
|
||||||
|
| a *one-element* array through the path | refused too — the case a value count cannot tell apart from a plain subdocument |
|
||||||
|
| an empty array at the path | refused |
|
||||||
|
| an array *inside* a subdocument at the path | fine — only the path itself matters |
|
||||||
|
| `hashed` with `expireAfterSeconds`, or with a partial filter | both allowed |
|
||||||
|
| a direction that is any non-zero, non-NaN number | allowed, sign taken as the direction, value echoed verbatim — this server takes only ±1 (PLAN §6) |
|
||||||
|
|||||||
File diff suppressed because one or more lines are too long
@@ -68,6 +68,15 @@ async function main() {
|
|||||||
.filter((f) => !ONLY || f === ONLY || f === ONLY + '.json')
|
.filter((f) => !ONLY || f === ONLY || f === ONLY + '.json')
|
||||||
.sort();
|
.sort();
|
||||||
for (const file of sources) {
|
for (const file of sources) {
|
||||||
|
// Plain JSON, deliberately, and the cost is worth stating: a source
|
||||||
|
// cannot name a BSON type the JSON grammar has no syntax for, so
|
||||||
|
// `5.0` reaches the driver as an int32 and no source here can ask a
|
||||||
|
// cross-type question. Reading sources as EJSON instead was tried and
|
||||||
|
// reverted -- EJSON's wrapper namespace collides with the query
|
||||||
|
// operators these sources are made of. `{"$regex": "x"}` parses to a
|
||||||
|
// BSONRegExp, `structuredClone` below then flattens it to
|
||||||
|
// `{pattern, options}`, and "a filter using $regex is refused"
|
||||||
|
// silently became a filter mongod accepts.
|
||||||
const src = JSON.parse(fs.readFileSync(path.join(SRC_DIR, file), 'utf8'));
|
const src = JSON.parse(fs.readFileSync(path.join(SRC_DIR, file), 'utf8'));
|
||||||
const name = path.basename(file, '.json');
|
const name = path.basename(file, '.json');
|
||||||
const out = await record(client, name, src);
|
const out = await record(client, name, src);
|
||||||
|
|||||||
@@ -56,10 +56,20 @@
|
|||||||
]
|
]
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"description": "equality across numeric types",
|
"description": "equality against a value two documents share",
|
||||||
|
"_comment": [
|
||||||
|
"This was 'equality across numeric types' and could not be: a source",
|
||||||
|
"is plain JSON, so 5.0 reaches the driver as an int32 and the case was",
|
||||||
|
"a second copy of the one above it. The cross-type property belongs to",
|
||||||
|
"how this server hashes anyway -- it hashes the *encoded* value, which",
|
||||||
|
"normalizes every numeric type -- and mongod's hash is a different",
|
||||||
|
"function, so a corpus could only ever check the answer. It is a unit",
|
||||||
|
"test instead: 'a hashed component answers equality across numeric",
|
||||||
|
"types and nothing else' in src/index.zig."
|
||||||
|
],
|
||||||
"ops": [
|
"ops": [
|
||||||
{ "name": "createIndex", "arguments": { "keys": { "a": "hashed" } } },
|
{ "name": "createIndex", "arguments": { "keys": { "a": "hashed" } } },
|
||||||
{ "name": "find", "arguments": { "filter": { "a": 5.0 }, "sort": { "_id": 1 } } }
|
{ "name": "find", "arguments": { "filter": { "a": 5 }, "sort": { "_id": 1 } } }
|
||||||
]
|
]
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
|
|||||||
Reference in New Issue
Block a user