diff --git a/src/query.zig b/src/query.zig index 18f799b..c91e11b 100644 --- a/src/query.zig +++ b/src/query.zig @@ -182,12 +182,42 @@ fn apply_expected( return false; } // Bare equality — matches if any candidate equals the expected value. - for (candidates) |actual| { + for (equality_candidates(candidates)) |actual| { if (bson.compare(actual, expected) == .eq) return true; } return false; } +/// A path that yields nothing is null to the equality family, and to it +/// alone. +/// +/// `{a: null}` matches a document with no `a` at all, as well as one holding +/// an explicit null. `{a: {$lt: 5}}` does not, even though null sorts below +/// 5; `{a: {$exists: false}}` has to keep seeing that there is nothing there; +/// `{a: {$type: "null"}}` stays false. So this substitutes a single null +/// rather than collecting one, and only where equality can see it. +/// +/// The other two layers have always agreed with this and only the matcher +/// did not: `index.build_entries` stores a missing field as null under a +/// non-sparse index, and the planner's sparse guard exists to stop exactly +/// this query reading an index that skipped those documents. Without this the +/// guard was defending a behaviour that did not exist. +const missing_as_null = [_]bson.Value{.null}; + +fn equality_candidates(candidates: []const bson.Value) []const bson.Value { + return if (candidates.len == 0) &missing_as_null else candidates; +} + +/// Whether `op` compares for equality, which is what decides that a missing +/// field is null. `$ne` and `$nin` are in it because they are the negations: +/// `{a: {$ne: null}}` has to *exclude* a document with no `a`. +fn equality_family(op: Op) bool { + return switch (op) { + .eq, .ne, .in, .nin => true, + else => false, + }; +} + /// Whether a stored document (canonical BSON bytes) matches `filter` — the /// byte-matcher counterpart of `matches`, used by scans. Same semantics, /// different collection: fields the filter does not name are skipped by @@ -398,9 +428,10 @@ fn match_operator( gpa: std.mem.Allocator, op: Op, value: bson.Value, - actuals: []const bson.Value, + actuals_at_path: []const bson.Value, regex_options: []const u8, ) QueryError!bool { + const actuals = if (equality_family(op)) equality_candidates(actuals_at_path) else actuals_at_path; if (op == .eq) { for (actuals) |a| if (bson.compare(a, value) == .eq) return true; return false; @@ -1348,6 +1379,95 @@ test "array index dot path and bare regex value" { try testing.expect(!try matches(testing.allocator, &doc_of(&.{.{ .key = "name", .value = .{ .regex = .{ .pattern = "^z", .options = "" } } }}), &d)); } +test "a missing field is null to equality and to nothing else" { + const gpa = testing.allocator; + // No `a` at all. The index layer has always stored this document under + // null in a non-sparse index; the matcher used to disagree, so + // `find({a: null})` answered nothing here and two documents on mongod. + const d = doc_of(&.{ .{ .key = "_id", .value = .{ .int32 = 1 } }, .{ .key = "z", .value = .{ .int32 = 1 } } }); + + // A table of static filters: `&.{...}` inside a `const` initializer is a + // comptime constant, where the same thing returned from a helper would be + // a pointer into that helper's dead frame. + const Case = struct { what: []const u8, filter: []const bson.Pair, want: bool }; + const cases = [_]Case{ + // The equality family sees a null... + .{ .what = "{a: null}", .want = true, .filter = &.{ + .{ .key = "a", .value = .null }, + } }, + .{ .what = "$eq null", .want = true, .filter = &.{ + .{ .key = "a", .value = .{ .doc = &.{.{ .key = "$eq", .value = .null }} } }, + } }, + .{ .what = "$in [null, 1]", .want = true, .filter = &.{ + .{ .key = "a", .value = .{ .doc = &.{.{ .key = "$in", .value = .{ .array = &.{ + .null, + .{ .int32 = 1 }, + } } }} } }, + } }, + // ...including through a dotted path that stops short... + .{ .what = "{a.b: null}", .want = true, .filter = &.{ + .{ .key = "a.b", .value = .null }, + } }, + // ...and the negations exclude it, which is the half that a bare + // "true when there are no candidates" would get backwards. + .{ .what = "$ne null", .want = false, .filter = &.{ + .{ .key = "a", .value = .{ .doc = &.{.{ .key = "$ne", .value = .null }} } }, + } }, + .{ .what = "$nin [null]", .want = false, .filter = &.{ + .{ .key = "a", .value = .{ .doc = &.{.{ .key = "$nin", .value = .{ .array = &.{.null} } }} } }, + } }, + // A non-null equality still finds nothing. + .{ .what = "{a: 1}", .want = false, .filter = &.{ + .{ .key = "a", .value = .{ .int32 = 1 } }, + } }, + .{ .what = "$ne 1", .want = true, .filter = &.{ + .{ .key = "a", .value = .{ .doc = &.{.{ .key = "$ne", .value = .{ .int32 = 1 } }} } }, + } }, + // Nothing outside the family sees it. Mutation check: widen + // `equality_family` to every operator, or collect the null in + // `field_matches` instead of substituting it here, and these flip. + .{ .what = "$lt 5", .want = false, .filter = &.{ + .{ .key = "a", .value = .{ .doc = &.{.{ .key = "$lt", .value = .{ .int32 = 5 } }} } }, + } }, + .{ .what = "$gte null", .want = false, .filter = &.{ + .{ .key = "a", .value = .{ .doc = &.{.{ .key = "$gte", .value = .null }} } }, + } }, + .{ .what = "$type null", .want = false, .filter = &.{ + .{ .key = "a", .value = .{ .doc = &.{.{ .key = "$type", .value = .{ .string = "null" } }} } }, + } }, + .{ .what = "$size 0", .want = false, .filter = &.{ + .{ .key = "a", .value = .{ .doc = &.{.{ .key = "$size", .value = .{ .int32 = 0 } }} } }, + } }, + .{ .what = "$exists false", .want = true, .filter = &.{ + .{ .key = "a", .value = .{ .doc = &.{.{ .key = "$exists", .value = .{ .bool = false } }} } }, + } }, + }; + + // The byte matcher answers scans and the tree matcher answers everything + // else, so both are run over every case: a rule in one and not the other + // would show up as a filter that changes its mind after a checkpoint. + var bytes: std.ArrayListUnmanaged(u8) = .empty; + defer bytes.deinit(gpa); + try bson.write_doc(d.pairs, gpa, &bytes); + for (cases) |c| { + errdefer std.debug.print("case: {s}\n", .{c.what}); + try testing.expectEqual(c.want, try matches(gpa, &doc_of(c.filter), &d)); + try testing.expectEqual(c.want, try matches_bytes(gpa, c.filter, bytes.items)); + } + + // An explicit null is unaffected, and so is a value. + const eq_null: []const bson.Pair = &.{.{ .key = "a", .value = .null }}; + const ne_null: []const bson.Pair = &.{ + .{ .key = "a", .value = .{ .doc = &.{.{ .key = "$ne", .value = .null }} } }, + }; + const explicit = doc_of(&.{.{ .key = "a", .value = .null }}); + try testing.expect(try matches(gpa, &doc_of(eq_null), &explicit)); + try testing.expect(!try matches(gpa, &doc_of(ne_null), &explicit)); + const valued = doc_of(&.{.{ .key = "a", .value = .{ .int32 = 1 } }}); + try testing.expect(!try matches(gpa, &doc_of(eq_null), &valued)); + try testing.expect(try matches(gpa, &doc_of(ne_null), &valued)); +} + test "sort compares by BSON order" { const a = doc_of(&.{ .{ .key = "n", .value = .{ .int32 = 2 } }, .{ .key = "x", .value = .{ .string = "a" } } }); const b = doc_of(&.{ .{ .key = "n", .value = .{ .int32 = 10 } }, .{ .key = "x", .value = .{ .string = "b" } } });