index/db: enforce _id uniqueness through the _id_ index
_id uniqueness was a `coll.docs.contains` probe. The docs hashmap is going
away (PLAN A3), so it has to move to the _id_ tree -- and the tree answers
better, because it is keyed on bson.encode_key, which is canonical where
serialize_value is not. int32 1, int64 1 and double 1.0 are now one _id, as
they are in MongoDB (A4).
_id_ is built and checked first, so a write violating both it and a unique
secondary reports _id_, which is what MongoDB reports. It returns
error.DuplicateKey with `dup_index` left null, which is exactly what
commands.zig's E11000 rendering already treats as "the _id_ index", so the
wire-visible message is unchanged and that file needed no edit.
check_unique's exclude-self became optional and is null on an insert. That was
a latent bug of its own: a replace must ignore its own existing entries, but an
insert has none, and passing the document's id there hides a collision whose
entry carries that same id -- precisely the case _id_ exists to catch. Only
_id_ could reach it, since a secondary collision is between different
documents.
Two corrections found while doing this, both worth reading:
PLAN A4 claimed a database already holding {_id: int32 1} and {_id: int64 1}
loses one on reopen. It does not. Replay evicts through the docs map, keyed on
serialize_value, so both survive; the tree is bulk-built afterwards with
enforcement off, which tolerates duplicate keys and warns. The loss arrives
only with the commit that drops the map, and that is where it needs a
pre-flight scan. Amended.
dispatch_insert asserted only `ok: 1`, but a rejected document comes back as a
writeError alongside it -- so the mixed-type corpus silently shrank from ten
documents to nine when _id_ became unique, and every test over it still passed.
The helper now rejects writeErrors and asserts the inserted count; it caught
the shrink immediately. The corpus keeps an int64 _id on a distinct value, and
the collision it used to stand in for is asserted directly.
Also adds Index.lookup_exact, which the commands that currently probe the docs
map will need. Exact byte equality rather than cmp_prefix, because {a: 1}'s
encoding is a proper prefix of {a: 1, b: 2}'s and a prefix match would claim a
document is present when it is not.
Mutation-checked, all three red: unique=false on id_index; exclude=id_key on
insert; eql -> cmp_prefix in lookup_exact.
This commit is contained in:
22
PLAN.md
22
PLAN.md
@@ -301,10 +301,24 @@ in real MongoDB. Making `_id_` a `unique` index is therefore a
|
||||
of `src/index.zig` anticipated when it fenced off the old map fast path.
|
||||
|
||||
Migration hazard, accepted deliberately: a database written by the current
|
||||
code may legitimately hold two such documents, and replay evicts by
|
||||
canonical key, so one is lost on reopen. Mitigation is a loud replay warning
|
||||
naming the namespace and both `_id` values, plus a documented one-way
|
||||
migration. Agreed with the human rather than assumed.
|
||||
code may legitimately hold two such documents. Agreed with the human rather
|
||||
than assumed.
|
||||
|
||||
*Corrected while implementing:* the hazard does **not** bite when `_id_`
|
||||
becomes unique, and the reason matters for sequencing. Replay does not
|
||||
maintain `_id_` entries — `apply_record` evicts through the docs map, keyed on
|
||||
`serialize_value`, and the tree is bulk-built afterwards by
|
||||
`build_all_indexes`. That build calls `finish_bulk` with enforcement off,
|
||||
which tolerates duplicate keys and already warns ("unique index '_id_' has
|
||||
duplicate keys in existing data"), honouring the "database must always open"
|
||||
rule. So such a database reopens with both documents intact and a loud
|
||||
warning, and new colliding inserts are rejected from then on.
|
||||
|
||||
The document *loss* arrives only with the commit that drops the docs hashmap,
|
||||
because eviction then goes through the canonical tree and the second document
|
||||
takes the first one's place. That commit is where this needs handling — a
|
||||
pre-flight scan for compare-equal `_id`s, refusing to drop the map silently
|
||||
while any exist — not here.
|
||||
|
||||
---
|
||||
|
||||
|
||||
Reference in New Issue
Block a user