M3: the implication test, and the row closes #14
33
PLAN.md
33
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**); 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 |
|
| 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**); the implication test that lets a partial index serve a read (**done**) — **the row is closed** | `tests/spec/positional/` 0 fail (51), `tests/spec/operators/` 0 fail (125) and `tests/spec/indexes/` 0 fail (48), 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,6 +1151,7 @@ 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.
|
||||||
|
(That step has since landed; see the implication entry below.)
|
||||||
|
|
||||||
**Hashed indexes landed**; `hashed.json` is 18/18, so `tests/spec/indexes/`
|
**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
|
is 42/42. A hashed component stores a tag byte and a 64-bit hash of the
|
||||||
@@ -1181,7 +1182,8 @@ has to be its own commit with its own re-recorded scorecard.
|
|||||||
cannot tell apart from a plain subdocument — so the check walks the path
|
cannot tell apart from a plain subdocument — so the check walks the path
|
||||||
rather than counting what it yields.
|
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
|
||||||
|
and since grown to 48 by the implication work.
|
||||||
A case there is a *sequence* -- create, insert, read, list -- because an
|
A case there is a *sequence* -- create, insert, read, list -- because an
|
||||||
index outlives a `deleteMany` and every case is about which indexes exist.
|
index outlives a `deleteMany` and every case is about which indexes exist.
|
||||||
Recording it corrected two of the review's own guesses: `$in` in a partial
|
Recording it corrected two of the review's own guesses: `$in` in a partial
|
||||||
@@ -1195,6 +1197,33 @@ 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.
|
||||||
|
|
||||||
|
- **The implication test closes M3's row, and the two gates see opposite
|
||||||
|
halves of it.** A partial index may answer a query that cannot match a
|
||||||
|
document its filter left out. Two routes: the query pins a value at the
|
||||||
|
filter's path and the *real matcher* is run against a stand-in document
|
||||||
|
holding it (sound because every operator the partial grammar admits is
|
||||||
|
existential, so more values at a path can only satisfy it more easily), or
|
||||||
|
the query's bounds are at least as tight as the filter's. An array value and
|
||||||
|
a null value are refused rather than approximated — `{a: [1, 2]}` matches
|
||||||
|
`{a: [[1, 2], 3]}`, whose values at `a` do not include 1 or 2, and
|
||||||
|
`{a: null}` matches a document with no `a` at all, which has no values there
|
||||||
|
rather than more of them.
|
||||||
|
|
||||||
|
Soundness is judged against **this server's** matcher, not mongod's:
|
||||||
|
`query.matches_bytes` decides the index's contents in `build_entries` and
|
||||||
|
re-filters every candidate. Where this server's comparison differs from
|
||||||
|
mongod's — the type-bracketing item below — both halves are wrong together,
|
||||||
|
which is a matching bug and not a lost document.
|
||||||
|
|
||||||
|
Measured with two mutations, and worth keeping because the answer is not
|
||||||
|
symmetric. Forcing the test to `true` takes `partial.json` to 23/7 and every
|
||||||
|
failure reads "expected N, got N-1" — the exact shape of the bug. Forcing it
|
||||||
|
to `false`, which is the behaviour it replaced, leaves the corpus **30/30**:
|
||||||
|
no client can observe *that* an index was used, only that an answer went
|
||||||
|
missing. So the corpus guards soundness and the unit test on `plan()` is the
|
||||||
|
only thing that can see the feature work at all. A read path with no
|
||||||
|
`explain` has no third option.
|
||||||
|
|
||||||
- **What the index corpus caught that was not about indexes: `{a: null}` did
|
- **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
|
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
|
no `a` as well as one holding an explicit null. This server matched only the
|
||||||
|
|||||||
@@ -154,7 +154,8 @@ list used for its own format change and it holds here for the same reason.
|
|||||||
|
|
||||||
## Outcome
|
## Outcome
|
||||||
|
|
||||||
Steps 1–4 landed in that order. `tests/spec/indexes/` is 42/42.
|
All five steps landed in that order, which closes M3's row.
|
||||||
|
`tests/spec/indexes/` is 48/48.
|
||||||
|
|
||||||
Three things this review got wrong, kept here because the point of writing it
|
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:
|
before the code was to find out which parts would not survive contact:
|
||||||
@@ -175,8 +176,22 @@ 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
|
`find({a: null})` did not match a document with no `a`, index or no index. See
|
||||||
PLAN §6.
|
PLAN §6.
|
||||||
|
|
||||||
Step 5, the implication test, is still open. Until it exists a partial index
|
And one thing §4 asserted without saying how: the implication test itself. It
|
||||||
is maintained, enforces `unique`, and is never read from.
|
turned out not to need a comparison of its own. Every operator
|
||||||
|
`check_partial_filter` admits is *existential* — "some value at this path
|
||||||
|
satisfies it" — so when the query pins a value, running the real matcher
|
||||||
|
against a stand-in document holding it settles `$eq`, `$in`, `$type`,
|
||||||
|
`$exists` and the bounds together. Only two shapes escape that argument (an
|
||||||
|
array value, whose elements a real document need not have; and null, which
|
||||||
|
also matches a document with no such field), and only ranges need reasoning
|
||||||
|
of their own.
|
||||||
|
|
||||||
|
The unnoticed half was the gates. A corpus recorded from mongod can see an
|
||||||
|
implication test that says *yes* too readily — the answer loses documents —
|
||||||
|
but not one that never says yes at all, because no client can observe which
|
||||||
|
index a read used. Measured: forcing the test true takes `partial.json` to
|
||||||
|
23/7; forcing it false leaves it 30/30. The unit test on `plan()` is the only
|
||||||
|
thing that sees the feature work.
|
||||||
|
|
||||||
## 6. Not covered
|
## 6. Not covered
|
||||||
|
|
||||||
|
|||||||
453
src/index.zig
453
src/index.zig
@@ -2659,6 +2659,264 @@ pub const Plan = struct {
|
|||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
// Partial-index implication
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
/// Whether every document this query matches is one the partial filter also
|
||||||
|
/// matches — the test that decides when a partial index may answer a read.
|
||||||
|
///
|
||||||
|
/// A partial index holds a subset of the collection, so reading from it is
|
||||||
|
/// only correct when the query cannot match a document the filter left out.
|
||||||
|
/// Too few documents is the one failure worse than having no index at all, so
|
||||||
|
/// every answer here is one-sided: a `false` costs a scan, a `true` has to be
|
||||||
|
/// right. Everything below is written to be wrong in that direction.
|
||||||
|
///
|
||||||
|
/// Soundness is judged against *this server's* matcher, not mongod's. Both
|
||||||
|
/// halves of the question are decided by the same code: `query.matches_bytes`
|
||||||
|
/// is what `build_entries` consults to decide the index's contents, and what
|
||||||
|
/// every candidate this plan yields is re-filtered through. Where this
|
||||||
|
/// server's comparison differs from mongod's — PLAN §6 records that the
|
||||||
|
/// comparison operators are not type-bracketed — both halves are wrong
|
||||||
|
/// together, which is a matching bug and not a lost document.
|
||||||
|
///
|
||||||
|
/// Only the operators `check_partial_filter` admits can appear on the filter
|
||||||
|
/// side, and every one of them is *existential*: "some value at this path
|
||||||
|
/// satisfies it". That is what the two routes below rest on.
|
||||||
|
fn query_implies_filter(
|
||||||
|
gpa: std.mem.Allocator,
|
||||||
|
filter: []const bson.Pair,
|
||||||
|
clauses: []const Clause,
|
||||||
|
) query.QueryError!bool {
|
||||||
|
for (filter) |f| {
|
||||||
|
if (!try conjunct_implied(gpa, f, clauses)) return false;
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Explicit rather than inferred: this and `query_implies_filter` call each
|
||||||
|
// other, which an inferred error set cannot resolve.
|
||||||
|
fn conjunct_implied(
|
||||||
|
gpa: std.mem.Allocator,
|
||||||
|
f: bson.Pair,
|
||||||
|
clauses: []const Clause,
|
||||||
|
) query.QueryError!bool {
|
||||||
|
if (f.key.len > 0 and f.key[0] == '$') {
|
||||||
|
const branches = switch (f.value) {
|
||||||
|
.array => |a| a,
|
||||||
|
else => return false,
|
||||||
|
};
|
||||||
|
if (std.mem.eql(u8, f.key, "$and")) {
|
||||||
|
for (branches) |b| {
|
||||||
|
const sub = switch (b) {
|
||||||
|
.doc => |d| d,
|
||||||
|
else => return false,
|
||||||
|
};
|
||||||
|
if (!try query_implies_filter(gpa, sub, clauses)) return false;
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
if (std.mem.eql(u8, f.key, "$or")) {
|
||||||
|
// One implied branch is enough. Implying none is not proof that
|
||||||
|
// the disjunction fails -- a query can imply `{$or: [A, B]}`
|
||||||
|
// without implying either -- so this is sufficient and not
|
||||||
|
// necessary, which is the side to be wrong on.
|
||||||
|
for (branches) |b| {
|
||||||
|
const sub = switch (b) {
|
||||||
|
.doc => |d| d,
|
||||||
|
else => continue,
|
||||||
|
};
|
||||||
|
if (try query_implies_filter(gpa, sub, clauses)) return true;
|
||||||
|
}
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
// The query's constraint on this path, accumulated exactly the way
|
||||||
|
// `evaluate_index` accumulates it. Whatever survives is a *necessary*
|
||||||
|
// condition of matching the query: an operator this does not understand
|
||||||
|
// clears the info rather than weakening it.
|
||||||
|
var qi = CompInfo{};
|
||||||
|
for (clauses) |cl| {
|
||||||
|
if (std.mem.eql(u8, cl.path, f.key)) analyze_clause(cl.value, &qi);
|
||||||
|
}
|
||||||
|
if (try pinned_values_imply(gpa, f.key, f.value, qi)) return true;
|
||||||
|
return bounds_imply(f.value, qi);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Route one: the query pins the path to a known value, or to one of a known
|
||||||
|
/// set, and each of them satisfies the filter.
|
||||||
|
fn pinned_values_imply(
|
||||||
|
gpa: std.mem.Allocator,
|
||||||
|
path: []const u8,
|
||||||
|
fv: bson.Value,
|
||||||
|
qi: CompInfo,
|
||||||
|
) query.QueryError!bool {
|
||||||
|
if (qi.eq) |v| return value_implies(gpa, path, v, fv);
|
||||||
|
if (qi.in_values) |list| {
|
||||||
|
// `{$in: []}` matches nothing and so implies everything, but an index
|
||||||
|
// read of a query with no answers is not worth a special case.
|
||||||
|
if (list.len == 0) return false;
|
||||||
|
for (list) |m| {
|
||||||
|
if (!try value_implies(gpa, path, m, fv)) return false;
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Whether a query pinning `v` at `path` implies the filter predicate `fv`
|
||||||
|
/// there.
|
||||||
|
///
|
||||||
|
/// Answered by running the real matcher against a stand-in document holding
|
||||||
|
/// exactly `v` at `path`, rather than by reimplementing eight operators
|
||||||
|
/// against a comparison that would then have two definitions. It is sound
|
||||||
|
/// because the filter's operators are existential, so a document with *more*
|
||||||
|
/// values at the path satisfies them at least as easily -- and every document
|
||||||
|
/// the query matches has `v` among its values there.
|
||||||
|
///
|
||||||
|
/// Two shapes break that argument and are refused rather than approximated:
|
||||||
|
///
|
||||||
|
/// - **`v` is an array.** `{a: [1, 2]}` matches `{a: [[1, 2], 3]}`, whose
|
||||||
|
/// values at `a` are the outer array, `[1, 2]` and `3` -- only one level
|
||||||
|
/// is expanded, so 1 and 2 are not among them and the real document's
|
||||||
|
/// value set is not a superset of the stand-in's.
|
||||||
|
/// - **`v` is null.** `{a: null}` also matches a document with no `a` at
|
||||||
|
/// all, which has *no* values at the path rather than more of them. The
|
||||||
|
/// stand-in alone would report `{a: {$exists: true}}` as implied, so an
|
||||||
|
/// empty document is tested too and both have to agree.
|
||||||
|
fn value_implies(
|
||||||
|
gpa: std.mem.Allocator,
|
||||||
|
path: []const u8,
|
||||||
|
v: bson.Value,
|
||||||
|
fv: bson.Value,
|
||||||
|
) query.QueryError!bool {
|
||||||
|
if (v == .array) return false;
|
||||||
|
var arena = std.heap.ArenaAllocator.init(gpa);
|
||||||
|
defer arena.deinit();
|
||||||
|
const a = arena.allocator();
|
||||||
|
const stand_in = (try nest_value(a, path, v)) orelse return false;
|
||||||
|
const pred = [_]bson.Pair{.{ .key = path, .value = fv }};
|
||||||
|
const f_doc = bson.Document{ .arena = undefined, .pairs = &pred };
|
||||||
|
if (!try query.matches(a, &f_doc, &stand_in)) return false;
|
||||||
|
if (v == .null) {
|
||||||
|
const empty = bson.Document{ .arena = undefined, .pairs = &.{} };
|
||||||
|
if (!try query.matches(a, &f_doc, &empty)) return false;
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The deepest path a stand-in document is built for. Past it the answer is
|
||||||
|
/// "cannot tell", which costs a scan.
|
||||||
|
const max_path_segments = 16;
|
||||||
|
|
||||||
|
/// A document holding `v` at a dotted `path`. Null when a segment is numeric:
|
||||||
|
/// `a.0` may address an array element, and a document with a literal "0"
|
||||||
|
/// field is a different question than the one being asked.
|
||||||
|
fn nest_value(
|
||||||
|
a: std.mem.Allocator,
|
||||||
|
path: []const u8,
|
||||||
|
v: bson.Value,
|
||||||
|
) std.mem.Allocator.Error!?bson.Document {
|
||||||
|
var segs: [max_path_segments][]const u8 = undefined;
|
||||||
|
var n: usize = 0;
|
||||||
|
var it = std.mem.splitScalar(u8, path, '.');
|
||||||
|
while (it.next()) |s| {
|
||||||
|
if (s.len == 0 or n == segs.len) return null;
|
||||||
|
if (std.fmt.parseInt(usize, s, 10)) |_| return null else |_| {}
|
||||||
|
segs[n] = s;
|
||||||
|
n += 1;
|
||||||
|
}
|
||||||
|
if (n == 0) return null;
|
||||||
|
var value = v;
|
||||||
|
var i = n;
|
||||||
|
while (i > 1) {
|
||||||
|
i -= 1;
|
||||||
|
const inner = try a.alloc(bson.Pair, 1);
|
||||||
|
inner[0] = .{ .key = segs[i], .value = value };
|
||||||
|
value = .{ .doc = inner };
|
||||||
|
}
|
||||||
|
const outer = try a.alloc(bson.Pair, 1);
|
||||||
|
outer[0] = .{ .key = segs[0], .value = value };
|
||||||
|
return .{ .arena = undefined, .pairs = outer };
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Route two: the query's bounds on a path are at least as tight as the
|
||||||
|
/// filter's. The only route that needs neither side to name a document --
|
||||||
|
/// `{a: {$gt: 5}}` implies `{a: {$gt: 0}}` with no value in common.
|
||||||
|
///
|
||||||
|
/// Restricted to the operators whose implication *is* an order comparison.
|
||||||
|
/// Anything else in the filter predicate leaves this to route one.
|
||||||
|
fn bounds_imply(fv: bson.Value, qi: CompInfo) bool {
|
||||||
|
const ops = switch (fv) {
|
||||||
|
.doc => |d| d,
|
||||||
|
else => return false,
|
||||||
|
};
|
||||||
|
if (!query.all_operator_keys(ops)) return false;
|
||||||
|
|
||||||
|
var lo: ?bson.Value = null;
|
||||||
|
var lo_incl = false;
|
||||||
|
var hi: ?bson.Value = null;
|
||||||
|
var hi_incl = false;
|
||||||
|
var wants_exists = false;
|
||||||
|
for (ops) |op| {
|
||||||
|
if (std.mem.eql(u8, op.key, "$gt")) {
|
||||||
|
lo = op.value;
|
||||||
|
lo_incl = false;
|
||||||
|
} else if (std.mem.eql(u8, op.key, "$gte")) {
|
||||||
|
lo = op.value;
|
||||||
|
lo_incl = true;
|
||||||
|
} else if (std.mem.eql(u8, op.key, "$lt")) {
|
||||||
|
hi = op.value;
|
||||||
|
hi_incl = false;
|
||||||
|
} else if (std.mem.eql(u8, op.key, "$lte")) {
|
||||||
|
hi = op.value;
|
||||||
|
hi_incl = true;
|
||||||
|
} else if (std.mem.eql(u8, op.key, "$exists")) {
|
||||||
|
// `$exists: false` is anti-monotone -- more values at a path make
|
||||||
|
// it *less* true -- so none of the reasoning here applies to it.
|
||||||
|
if (op.value != .bool or !op.value.bool) return false;
|
||||||
|
wants_exists = true;
|
||||||
|
} else {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (lo == null and hi == null and !wants_exists) return false;
|
||||||
|
|
||||||
|
if (lo) |f| {
|
||||||
|
const q = qi.lo orelse return false;
|
||||||
|
if (!bound_implies(q, qi.lo_incl, f, lo_incl, .gt)) return false;
|
||||||
|
}
|
||||||
|
if (hi) |f| {
|
||||||
|
const q = qi.hi orelse return false;
|
||||||
|
if (!bound_implies(q, qi.hi_incl, f, hi_incl, .lt)) return false;
|
||||||
|
}
|
||||||
|
// A bound predicate never matches a document with no value at the path:
|
||||||
|
// the comparison loop has nothing to run over. So any bound at all on the
|
||||||
|
// query's side settles `$exists: true`.
|
||||||
|
if (wants_exists and qi.lo == null and qi.hi == null) return false;
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Whether a query bound is at least as tight as a filter bound on the same
|
||||||
|
/// side. `tighter` is the direction that narrows: `.gt` for a lower bound,
|
||||||
|
/// `.lt` for an upper one.
|
||||||
|
fn bound_implies(
|
||||||
|
q: bson.Value,
|
||||||
|
q_incl: bool,
|
||||||
|
f: bson.Value,
|
||||||
|
f_incl: bool,
|
||||||
|
tighter: std.math.Order,
|
||||||
|
) bool {
|
||||||
|
const o = bson.compare(q, f);
|
||||||
|
if (o == tighter) return true;
|
||||||
|
if (o != .eq) return false;
|
||||||
|
// The same endpoint: the query implies the filter unless it admits that
|
||||||
|
// endpoint where the filter excludes it.
|
||||||
|
return f_incl or !q_incl;
|
||||||
|
}
|
||||||
|
|
||||||
/// Pick the index (if any) that can generate a superset of the matching
|
/// Pick the index (if any) that can generate a superset of the matching
|
||||||
/// documents: the one covering the longest leading run of equality/$in
|
/// documents: the one covering the longest leading run of equality/$in
|
||||||
/// predicates, optionally with a range on the next key. Returns null when
|
/// predicates, optionally with a range on the next key. Returns null when
|
||||||
@@ -2690,13 +2948,11 @@ pub fn plan(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
for (indexes) |ix| {
|
for (indexes) |ix| {
|
||||||
// A partial index holds a *subset* of the collection, so answering a
|
// A partial index holds a *subset* of the collection, so it may only
|
||||||
// query from it is only correct when the query's predicates imply its
|
// answer a query that cannot match a document its filter left out.
|
||||||
// filter. That implication test does not exist yet, and reading from
|
if (ix.partial) |f| {
|
||||||
// the index without it returns too few documents -- the one failure
|
if (!try query_implies_filter(gpa, f.pairs, clauses.items)) continue;
|
||||||
// worse than having no index at all. So it is maintained, it enforces
|
}
|
||||||
// `unique`, and reads scan. PLAN §6.
|
|
||||||
if (ix.partial != null) continue;
|
|
||||||
var cand = (try evaluate_index(gpa, ix, clauses.items, sort)) orelse continue;
|
var cand = (try evaluate_index(gpa, ix, clauses.items, sort)) orelse continue;
|
||||||
if (best) |b| {
|
if (best) |b| {
|
||||||
if (plan_better(&cand, &b)) {
|
if (plan_better(&cand, &b)) {
|
||||||
@@ -4248,6 +4504,189 @@ test "the _id index plan covers equality, ranges and _id sort order" {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
test "a partial index answers only a query that implies its filter" {
|
||||||
|
const gpa = testing.allocator;
|
||||||
|
|
||||||
|
// Every row is a (partial filter, query) pair and whether the query is
|
||||||
|
// allowed to read the index. The two halves this table is really about
|
||||||
|
// are opposite failures: a `true` that should be false loses documents
|
||||||
|
// silently, a `false` that should be true only costs a scan. So the rows
|
||||||
|
// are weighted towards the first.
|
||||||
|
const Row = struct {
|
||||||
|
filter: []const bson.Pair,
|
||||||
|
query: []const bson.Pair,
|
||||||
|
implied: bool,
|
||||||
|
why: []const u8,
|
||||||
|
};
|
||||||
|
const eq_true: []const bson.Pair = &.{.{ .key = "t", .value = .{ .bool = true } }};
|
||||||
|
const gt_zero: []const bson.Pair = &.{
|
||||||
|
.{ .key = "n", .value = .{ .doc = &.{.{ .key = "$gt", .value = .{ .int32 = 0 } }} } },
|
||||||
|
};
|
||||||
|
const exists: []const bson.Pair = &.{
|
||||||
|
.{ .key = "n", .value = .{ .doc = &.{.{ .key = "$exists", .value = .{ .bool = true } }} } },
|
||||||
|
};
|
||||||
|
|
||||||
|
const rows = [_]Row{
|
||||||
|
// -- route one: the query pins a value -----------------------------
|
||||||
|
.{ .why = "the same equality", .implied = true, .filter = eq_true, .query = &.{
|
||||||
|
.{ .key = "a", .value = .{ .int32 = 1 } },
|
||||||
|
.{ .key = "t", .value = .{ .bool = true } },
|
||||||
|
} },
|
||||||
|
.{ .why = "the opposite equality", .implied = false, .filter = eq_true, .query = &.{
|
||||||
|
.{ .key = "a", .value = .{ .int32 = 1 } },
|
||||||
|
.{ .key = "t", .value = .{ .bool = false } },
|
||||||
|
} },
|
||||||
|
.{ .why = "no predicate on the filter's path", .implied = false, .filter = eq_true, .query = &.{
|
||||||
|
.{ .key = "a", .value = .{ .int32 = 1 } },
|
||||||
|
} },
|
||||||
|
.{ .why = "an equality inside the filter's range", .implied = true, .filter = gt_zero, .query = &.{
|
||||||
|
.{ .key = "n", .value = .{ .int32 = 5 } },
|
||||||
|
} },
|
||||||
|
.{ .why = "an equality outside it", .implied = false, .filter = gt_zero, .query = &.{
|
||||||
|
.{ .key = "n", .value = .{ .int32 = -5 } },
|
||||||
|
} },
|
||||||
|
.{ .why = "an $in wholly inside", .implied = true, .filter = gt_zero, .query = &.{
|
||||||
|
.{ .key = "n", .value = .{ .doc = &.{.{ .key = "$in", .value = .{ .array = &.{
|
||||||
|
.{ .int32 = 5 },
|
||||||
|
.{ .int32 = 9 },
|
||||||
|
} } }} } },
|
||||||
|
} },
|
||||||
|
.{ .why = "an $in straddling it", .implied = false, .filter = gt_zero, .query = &.{
|
||||||
|
.{ .key = "n", .value = .{ .doc = &.{.{ .key = "$in", .value = .{ .array = &.{
|
||||||
|
.{ .int32 = 5 },
|
||||||
|
.{ .int32 = 0 },
|
||||||
|
} } }} } },
|
||||||
|
} },
|
||||||
|
.{ .why = "an equality implies $exists", .implied = true, .filter = exists, .query = &.{
|
||||||
|
.{ .key = "n", .value = .{ .int32 = 5 } },
|
||||||
|
} },
|
||||||
|
// `{n: null}` matches a document with no `n` at all, so it cannot
|
||||||
|
// imply that `n` exists. This is the row that makes the stand-in
|
||||||
|
// document insufficient on its own.
|
||||||
|
.{ .why = "equality to null does not imply $exists", .implied = false, .filter = exists, .query = &.{
|
||||||
|
.{ .key = "n", .value = .null },
|
||||||
|
} },
|
||||||
|
// ...but it does imply an equality to null, and the index does hold
|
||||||
|
// those documents: `build_entries` runs the same matcher.
|
||||||
|
.{ .why = "equality to null implies equality to null", .implied = true, .query = &.{
|
||||||
|
.{ .key = "n", .value = .null },
|
||||||
|
}, .filter = &.{.{ .key = "n", .value = .null }} },
|
||||||
|
// An array value cannot stand in for itself: `{a: [1, 2]}` matches
|
||||||
|
// `{a: [[1, 2], 3]}`, whose values at `a` do not include 1 or 2.
|
||||||
|
.{ .why = "an array equality is refused", .implied = false, .filter = gt_zero, .query = &.{
|
||||||
|
.{ .key = "n", .value = .{ .array = &.{ .{ .int32 = 5 }, .{ .int32 = 9 } } } },
|
||||||
|
} },
|
||||||
|
|
||||||
|
// -- route two: bounds ---------------------------------------------
|
||||||
|
.{ .why = "a tighter lower bound", .implied = true, .filter = gt_zero, .query = &.{
|
||||||
|
.{ .key = "n", .value = .{ .doc = &.{.{ .key = "$gt", .value = .{ .int32 = 5 } }} } },
|
||||||
|
} },
|
||||||
|
.{ .why = "the identical bound", .implied = true, .filter = gt_zero, .query = &.{
|
||||||
|
.{ .key = "n", .value = .{ .doc = &.{.{ .key = "$gt", .value = .{ .int32 = 0 } }} } },
|
||||||
|
} },
|
||||||
|
.{ .why = "a looser lower bound", .implied = false, .filter = gt_zero, .query = &.{
|
||||||
|
.{ .key = "n", .value = .{ .doc = &.{.{ .key = "$gt", .value = .{ .int32 = -1 } }} } },
|
||||||
|
} },
|
||||||
|
// $gte 0 admits 0 where the filter's $gt 0 excludes it -- the whole
|
||||||
|
// of what inclusivity decides.
|
||||||
|
.{ .why = "the same endpoint, inclusive against exclusive", .implied = false, .filter = gt_zero, .query = &.{
|
||||||
|
.{ .key = "n", .value = .{ .doc = &.{.{ .key = "$gte", .value = .{ .int32 = 0 } }} } },
|
||||||
|
} },
|
||||||
|
.{ .why = "a bound implies $exists", .implied = true, .filter = exists, .query = &.{
|
||||||
|
.{ .key = "n", .value = .{ .doc = &.{.{ .key = "$gt", .value = .{ .int32 = 5 } }} } },
|
||||||
|
} },
|
||||||
|
.{ .why = "an upper bound does not imply a lower one", .implied = false, .filter = gt_zero, .query = &.{
|
||||||
|
.{ .key = "n", .value = .{ .doc = &.{.{ .key = "$lt", .value = .{ .int32 = 5 } }} } },
|
||||||
|
} },
|
||||||
|
// An operator the analysis does not understand clears the info, so
|
||||||
|
// the tight bound beside it cannot be leaned on.
|
||||||
|
.{ .why = "an unusable operator beside a bound", .implied = false, .filter = gt_zero, .query = &.{
|
||||||
|
.{ .key = "n", .value = .{ .doc = &.{
|
||||||
|
.{ .key = "$gt", .value = .{ .int32 = 5 } },
|
||||||
|
.{ .key = "$bogus", .value = .{ .int32 = 1 } },
|
||||||
|
} } },
|
||||||
|
} },
|
||||||
|
|
||||||
|
// -- the filter's own shape ----------------------------------------
|
||||||
|
.{ .why = "both conjuncts implied", .implied = true, .query = &.{
|
||||||
|
.{ .key = "t", .value = .{ .bool = true } },
|
||||||
|
.{ .key = "n", .value = .{ .int32 = 5 } },
|
||||||
|
}, .filter = &.{
|
||||||
|
.{ .key = "t", .value = .{ .bool = true } },
|
||||||
|
.{ .key = "n", .value = .{ .doc = &.{.{ .key = "$gt", .value = .{ .int32 = 0 } }} } },
|
||||||
|
} },
|
||||||
|
.{ .why = "one conjunct implied is not enough", .implied = false, .query = &.{
|
||||||
|
.{ .key = "t", .value = .{ .bool = true } },
|
||||||
|
}, .filter = &.{
|
||||||
|
.{ .key = "t", .value = .{ .bool = true } },
|
||||||
|
.{ .key = "n", .value = .{ .doc = &.{.{ .key = "$gt", .value = .{ .int32 = 0 } }} } },
|
||||||
|
} },
|
||||||
|
.{ .why = "one branch of the filter's $or", .implied = true, .query = &.{
|
||||||
|
.{ .key = "t", .value = .{ .bool = true } },
|
||||||
|
}, .filter = &.{.{ .key = "$or", .value = .{ .array = &.{
|
||||||
|
.{ .doc = &.{.{ .key = "t", .value = .{ .bool = true } }} },
|
||||||
|
.{ .doc = &.{.{ .key = "n", .value = .{ .int32 = 9 } }} },
|
||||||
|
} } }} },
|
||||||
|
.{ .why = "no branch of the filter's $or", .implied = false, .query = &.{
|
||||||
|
.{ .key = "t", .value = .{ .bool = false } },
|
||||||
|
}, .filter = &.{.{ .key = "$or", .value = .{ .array = &.{
|
||||||
|
.{ .doc = &.{.{ .key = "t", .value = .{ .bool = true } }} },
|
||||||
|
.{ .doc = &.{.{ .key = "n", .value = .{ .int32 = 9 } }} },
|
||||||
|
} } }} },
|
||||||
|
// The query's own $or is skipped by `flatten_clauses`, so its members
|
||||||
|
// are not predicates the query is known to enforce.
|
||||||
|
.{ .why = "a query $or supplies nothing", .implied = false, .filter = eq_true, .query = &.{
|
||||||
|
.{ .key = "$or", .value = .{ .array = &.{
|
||||||
|
.{ .doc = &.{.{ .key = "t", .value = .{ .bool = true } }} },
|
||||||
|
} } },
|
||||||
|
} },
|
||||||
|
// ...but a query $and does: it is a conjunction like the top level.
|
||||||
|
.{ .why = "a query $and supplies its members", .implied = true, .filter = eq_true, .query = &.{
|
||||||
|
.{ .key = "$and", .value = .{ .array = &.{
|
||||||
|
.{ .doc = &.{.{ .key = "t", .value = .{ .bool = true } }} },
|
||||||
|
} } },
|
||||||
|
} },
|
||||||
|
// A dotted path is nested into the stand-in document...
|
||||||
|
.{ .why = "a dotted filter path", .implied = true, .query = &.{
|
||||||
|
.{ .key = "m.t", .value = .{ .bool = true } },
|
||||||
|
}, .filter = &.{.{ .key = "m.t", .value = .{ .bool = true } }} },
|
||||||
|
// ...unless a segment is numeric, where it might mean an array index.
|
||||||
|
.{ .why = "a numeric path segment is refused", .implied = false, .query = &.{
|
||||||
|
.{ .key = "m.0", .value = .{ .bool = true } },
|
||||||
|
}, .filter = &.{.{ .key = "m.0", .value = .{ .bool = true } }} },
|
||||||
|
};
|
||||||
|
|
||||||
|
for (rows) |r| {
|
||||||
|
errdefer std.debug.print("row: {s}\n", .{r.why});
|
||||||
|
var clauses: std.ArrayListUnmanaged(Clause) = .empty;
|
||||||
|
defer clauses.deinit(gpa);
|
||||||
|
try flatten_clauses(gpa, r.query, &clauses);
|
||||||
|
try testing.expectEqual(r.implied, try query_implies_filter(gpa, r.filter, clauses.items));
|
||||||
|
}
|
||||||
|
|
||||||
|
// And the whole point of the answer: a partial index the query implies is
|
||||||
|
// planned, one it does not is left alone. Mutation check: make
|
||||||
|
// `query_implies_filter` return true unconditionally and the second
|
||||||
|
// expectation here starts producing a plan -- which is the shape of a
|
||||||
|
// read that silently returns too few documents.
|
||||||
|
var ix = try simple_index(gpa, test_pager(), &.{"a"}, false, false);
|
||||||
|
defer ix.deinit(gpa);
|
||||||
|
try ix.set_partial(gpa, eq_true);
|
||||||
|
{
|
||||||
|
const f = [_]bson.Pair{
|
||||||
|
.{ .key = "a", .value = .{ .int32 = 1 } },
|
||||||
|
.{ .key = "t", .value = .{ .bool = true } },
|
||||||
|
};
|
||||||
|
var p = (try plan(gpa, null, &.{&ix}, &f, &.{})).?;
|
||||||
|
defer p.deinit(gpa);
|
||||||
|
try testing.expectEqual(@as(usize, 1), p.key_len());
|
||||||
|
}
|
||||||
|
{
|
||||||
|
const f = [_]bson.Pair{.{ .key = "a", .value = .{ .int32 = 1 } }};
|
||||||
|
try testing.expect((try plan(gpa, null, &.{&ix}, &f, &.{})) == null);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
test "planner picks eq run, ranges, and bails on sparse null" {
|
test "planner picks eq run, ranges, and bails on sparse null" {
|
||||||
const gpa = testing.allocator;
|
const gpa = testing.allocator;
|
||||||
var ix = try simple_index(gpa, test_pager(), &.{ "a", "b" }, false, false);
|
var ix = try simple_index(gpa, test_pager(), &.{ "a", "b" }, false, false);
|
||||||
|
|||||||
@@ -50,7 +50,7 @@ halves have since been driven green:
|
|||||||
|
|
||||||
```
|
```
|
||||||
hashed.json 18 pass 0 fail 0 skip
|
hashed.json 18 pass 0 fail 0 skip
|
||||||
partial.json 24 pass 0 fail 0 skip
|
partial.json 30 pass 0 fail 0 skip
|
||||||
```
|
```
|
||||||
|
|
||||||
The three that passed at the start were the reads a partial index does not
|
The three that passed at the start were the reads a partial index does not
|
||||||
@@ -63,6 +63,24 @@ 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
|
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.
|
the pinned crud+aggregate scorecard did not move when it was fixed.
|
||||||
|
|
||||||
|
## What this corpus cannot see
|
||||||
|
|
||||||
|
Six of `partial.json`'s cases exist for the implication test — the rule that
|
||||||
|
lets a partial index answer a read — and it is worth being exact about what
|
||||||
|
they guard, because it is only half of it.
|
||||||
|
|
||||||
|
A partial index holds a subset, so reading from one the query does not imply
|
||||||
|
returns **too few** documents, and that is an answer a result comparison
|
||||||
|
catches. It was measured: forcing the implication test to always say yes takes
|
||||||
|
this file to 23 pass / 7 fail, every failure reading "expected N, got N-1".
|
||||||
|
|
||||||
|
The other direction is invisible here. Forcing it to always say *no* — the
|
||||||
|
behaviour before the test existed, where a partial index was maintained and
|
||||||
|
never read — leaves this file at **30 pass / 0 fail**. No client can observe
|
||||||
|
which index a read used, and this server has no `explain`. So the corpus
|
||||||
|
guards soundness and a unit test on `plan()` is the only thing that sees the
|
||||||
|
feature work at all.
|
||||||
|
|
||||||
## 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
|
||||||
|
|||||||
File diff suppressed because one or more lines are too long
@@ -79,6 +79,85 @@
|
|||||||
{ "name": "updateOne", "arguments": { "filter": { "_id": 2 }, "update": { "$set": { "t": true } } } }
|
{ "name": "updateOne", "arguments": { "filter": { "_id": 2 }, "update": { "$set": { "t": true } } } }
|
||||||
]
|
]
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
"_comment": [
|
||||||
|
"The reads below exist for the implication test: a partial index may",
|
||||||
|
"only answer a query that cannot match a document its filter left",
|
||||||
|
"out. Each pairs a query that implies the filter with one that does",
|
||||||
|
"not and touches the same field, so an implication test that says yes",
|
||||||
|
"too readily loses the documents outside the filter -- which is a",
|
||||||
|
"wrong answer a result comparison can see, unlike the index being",
|
||||||
|
"used at all, which no client can observe."
|
||||||
|
],
|
||||||
|
"description": "a query that leaves the filter's field out still sees past it",
|
||||||
|
"documents": [
|
||||||
|
{ "_id": 1, "a": 1, "t": true },
|
||||||
|
{ "_id": 2, "a": 1, "t": false },
|
||||||
|
{ "_id": 3, "a": 2, "t": true }
|
||||||
|
],
|
||||||
|
"ops": [
|
||||||
|
{ "name": "createIndex", "arguments": { "keys": { "a": 1 }, "partialFilterExpression": { "t": true } } },
|
||||||
|
{ "name": "find", "arguments": { "filter": { "a": 1 }, "sort": { "_id": 1 } } },
|
||||||
|
{ "name": "find", "arguments": { "filter": { "a": 1, "t": true }, "sort": { "_id": 1 } } },
|
||||||
|
{ "name": "find", "arguments": { "filter": { "a": 1, "t": false }, "sort": { "_id": 1 } } }
|
||||||
|
]
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"description": "an $in straddling the filter still answers every match",
|
||||||
|
"ops": [
|
||||||
|
{ "name": "createIndex", "arguments": { "keys": { "a": 1 }, "partialFilterExpression": { "a": { "$gte": 5 } } } },
|
||||||
|
{ "name": "find", "arguments": { "filter": { "a": { "$in": [1, 5] } }, "sort": { "_id": 1 } } },
|
||||||
|
{ "name": "find", "arguments": { "filter": { "a": { "$in": [5, 9] } }, "sort": { "_id": 1 } } }
|
||||||
|
]
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"description": "a query for null does not belong to an index that requires the field",
|
||||||
|
"documents": [
|
||||||
|
{ "_id": 1, "a": 1 },
|
||||||
|
{ "_id": 2 },
|
||||||
|
{ "_id": 3, "a": null }
|
||||||
|
],
|
||||||
|
"ops": [
|
||||||
|
{ "name": "createIndex", "arguments": { "keys": { "a": 1 }, "partialFilterExpression": { "a": { "$exists": true } } } },
|
||||||
|
{ "name": "find", "arguments": { "filter": { "a": null }, "sort": { "_id": 1 } } },
|
||||||
|
{ "name": "find", "arguments": { "filter": { "a": 1 }, "sort": { "_id": 1 } } }
|
||||||
|
]
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"description": "an equality inside the filter's range, and one outside it",
|
||||||
|
"ops": [
|
||||||
|
{ "name": "createIndex", "arguments": { "keys": { "a": 1 }, "partialFilterExpression": { "a": { "$gt": 3 } } } },
|
||||||
|
{ "name": "find", "arguments": { "filter": { "a": 5 }, "sort": { "_id": 1 } } },
|
||||||
|
{ "name": "find", "arguments": { "filter": { "a": 1 }, "sort": { "_id": 1 } } },
|
||||||
|
{ "name": "find", "arguments": { "filter": { "a": { "$gt": 4 } }, "sort": { "_id": 1 } } },
|
||||||
|
{ "name": "find", "arguments": { "filter": { "a": { "$gt": 0 } }, "sort": { "_id": 1 } } }
|
||||||
|
]
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"description": "a sort a partial index could serve",
|
||||||
|
"documents": [
|
||||||
|
{ "_id": 1, "a": 3, "t": true },
|
||||||
|
{ "_id": 2, "a": 1, "t": false },
|
||||||
|
{ "_id": 3, "a": 2, "t": true }
|
||||||
|
],
|
||||||
|
"ops": [
|
||||||
|
{ "name": "createIndex", "arguments": { "keys": { "a": 1 }, "partialFilterExpression": { "t": true } } },
|
||||||
|
{ "name": "find", "arguments": { "filter": { "t": true }, "sort": { "a": 1 } } },
|
||||||
|
{ "name": "find", "arguments": { "filter": {}, "sort": { "a": 1 } } }
|
||||||
|
]
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"description": "a unique partial index answers the read it constrains",
|
||||||
|
"documents": [
|
||||||
|
{ "_id": 1, "a": 9, "t": true },
|
||||||
|
{ "_id": 2, "a": 9, "t": false }
|
||||||
|
],
|
||||||
|
"ops": [
|
||||||
|
{ "name": "createIndex", "arguments": { "keys": { "a": 1 }, "unique": true, "partialFilterExpression": { "t": true } } },
|
||||||
|
{ "name": "find", "arguments": { "filter": { "a": 9, "t": true }, "sort": { "_id": 1 } } },
|
||||||
|
{ "name": "find", "arguments": { "filter": { "a": 9 }, "sort": { "_id": 1 } } }
|
||||||
|
]
|
||||||
|
},
|
||||||
{
|
{
|
||||||
"description": "a filter on $exists",
|
"description": "a filter on $exists",
|
||||||
"ops": [
|
"ops": [
|
||||||
|
|||||||
Reference in New Issue
Block a user