src/commands.zig

@@ -187,6 +187,7 @@ const command_table = [_]Command{
         .handler = cmd_get_more,
     },
     .{ .name = "count", .kind = .read, .locks = .{ .catalog = .shared, .coll = .shared }, .handler = cmd_count },
+    .{ .name = "distinct", .kind = .read, .locks = .{ .catalog = .shared, .coll = .shared }, .handler = cmd_distinct },
     .{ .name = "aggregate", .kind = .read, .locks = .{ .catalog = .shared, .coll = .shared }, .handler = cmd_aggregate },
     .{ .name = "listDatabases", .kind = .read, .locks = .{ .catalog = .shared }, .handler = cmd_list_databases },
     .{ .name = "listCollections", .kind = .read, .locks = .{ .catalog = .shared }, .handler = cmd_list_collections },
@@ -2141,6 +2142,119 @@ fn cmd_count(ctx: *Context, msg: *wire.Message, reply: *wire.Reply) !void {
     try reply.put_ok();
 }
 
+/// `distinct` is the one read command whose answer is a *set*, and both halves
+/// of what that means were measured against mongod 8.3.7 rather than recalled:
+///
+///   - the values come back **sorted in canonical BSON order**, not in the
+///     order they were met. `{s: "a"}, {s: "b"}, {s: null}` answers
+///     `[null, "a", "b"]` -- null ahead of the strings, because that is where
+///     its type ranks.
+///   - deduping uses the same comparator, so an int32 `1` and a double `1.0`
+///     collapse into one value while `null` and `"1"` stay distinct.
+///
+/// Both fall out of `bson.compare`, the comparator `$sort` and `$min` already
+/// use, and that is not a coincidence: mongod accumulates into a
+/// `BSONElementSet` ordered by the same `woCompare`. Sorting was the part
+/// worth measuring -- insertion order is the obvious guess and it is wrong.
+///
+/// Unbounded in memory, like `$group` and `$sort`: every value at the key is
+/// held before the answer is deduped. mongod caps the reply at 16 MB instead;
+/// recorded in PLAN §6 with the other two rather than solved here.
+fn cmd_distinct(ctx: *Context, msg: *wire.Message, reply: *wire.Reply) !void {
+    const db_name = msg.db_name() orelse return invalid_arg(reply, "distinct requires $db");
+    // A non-string collection name never reaches here: dispatch refuses it
+    // with BadValue while resolving the lock target, where mongod answers
+    // InvalidNamespace (73). Left alone deliberately -- that answer is
+    // dispatch's for every command, and changing it is its own measurement.
+    const coll_name = str_arg(msg.body.get("distinct")) orelse
+        return bad_value(reply, "distinct requires a collection name");
+
+    const key_value = msg.body.get("key") orelse return reply.put_error(
+        @intFromEnum(ErrorCode.idl_failed_to_parse),
+        "IDLFailedToParse",
+        "BSON field 'distinctCommandRequest.key' is missing but a required field",
+    );
+    const key = str_arg(key_value) orelse return distinct_wrong_type(reply, "key", key_value, "string");
+
+    // `query` absent, and `query: null`, are both an empty filter -- measured,
+    // and the second is not guessable from the first. Anything else that is
+    // not a document is a TypeMismatch, which is stricter than `count` is
+    // about its own `query`, because mongod parses this one through its IDL.
+    const query_value: bson.Value = msg.body.get("query") orelse .null;
+    const filter: []const bson.Pair = switch (query_value) {
+        .doc => |pairs| pairs,
+        .null => &.{},
+        else => return distinct_wrong_type(reply, "query", query_value, "object"),
+    };
+
+    // An absent collection -- or database -- is an empty set, not an error.
+    // Measured, and the same answer `aggregate` gives.
+    const coll = ctx.engine.get_collection(db_name, coll_name) orelse {
+        try reply.put("values", .{ .array = &.{} });
+        return reply.put_ok();
+    };
+    var offs: std.ArrayListUnmanaged(u64) = .empty;
+    defer offs.deinit(ctx.gpa);
+    _ = try scan_matching(ctx, db_name, coll_name, filter, 0, &offs);
+
+    // The values outlive this frame inside the reply, so they are built in its
+    // arena; the collection lock dispatch is holding is what keeps the slab
+    // bytes they were read from mapped for the duration.
+    const arena = reply.arena_alloc();
+    var values: std.ArrayListUnmanaged(bson.Value) = .empty;
+    var at_key: std.ArrayListUnmanaged(bson.Value) = .empty;
+    for (offs.items) |off| {
+        at_key.clearRetainingCapacity();
+        // Byte-walked, and the same traversal the matcher and the index use:
+        // a path through an array of subdocuments collects from each of them.
+        try query.collect_values_bytes(arena, coll.doc_bytes(off), key, &at_key, 0);
+        for (at_key.items) |v| switch (v) {
+            // A terminal array contributes its elements rather than itself,
+            // and exactly one level deep: `[[7, 8], 9]` answers `[7, 8]` and
+            // `9`, never 7 and 8. The interior of the path is already unwound
+            // by `collect_values_bytes`, so this only ever sees the last
+            // segment's value.
+            .array => |items| try values.appendSlice(arena, items),
+            else => try values.append(arena, v),
+        };
+    }
+
+    // Sort, then collapse equal neighbours -- see the note above for why both
+    // steps use `bson.compare`. `std.mem.sort` is stable, so when several
+    // documents spell one value differently (`1` and `1.0`) the representation
+    // that survives is the one the scan met first.
+    std.mem.sort(bson.Value, values.items, {}, value_less);
+    var n: usize = 0;
+    for (values.items) |v| {
+        if (n > 0 and bson.compare(values.items[n - 1], v) == .eq) continue;
+        values.items[n] = v;
+        n += 1;
+    }
+    try reply.put("values", .{ .array = values.items[0..n] });
+    try reply.put_ok();
+}
+
+fn value_less(_: void, a: bson.Value, b: bson.Value) bool {
+    return bson.compare(a, b) == .lt;
+}
+
+/// The shape mongod's IDL parser reports a wrong-typed command field in. A
+/// driver that matches on the text is matching on this, so it is reproduced
+/// rather than paraphrased.
+fn distinct_wrong_type(
+    reply: *wire.Reply,
+    field: []const u8,
+    got: bson.Value,
+    want: []const u8,
+) !void {
+    const text = try std.fmt.allocPrint(
+        reply.arena_alloc(),
+        "BSON field 'distinctCommandRequest.{s}' is the wrong type '{s}', expected type '{s}'",
+        .{ field, got.type_name(), want },
+    );
+    return reply.put_error(@intFromEnum(ErrorCode.type_mismatch), "TypeMismatch", text);
+}
+
 fn cmd_aggregate(ctx: *Context, msg: *wire.Message, reply: *wire.Reply) !void {
     const db_name = msg.db_name() orelse return invalid_arg(reply, "aggregate requires $db");
     const coll_name = str_arg(msg.body.get("aggregate")) orelse return bad_value(reply, "aggregate requires a collection name");
@@ -5106,6 +5220,280 @@ fn clear_id_list(gpa: std.mem.Allocator, list: *std.ArrayListUnmanaged([]u8)) vo
     list.clearRetainingCapacity();
 }
 
+/// Runs `distinct` and hands back its `values`. The caller owns `reply`,
+/// because the values are allocated in the reply's arena.
+fn distinct_values(
+    tdb: *TestDb,
+    io: std.Io,
+    reply: *wire.Reply,
+    coll: []const u8,
+    extra: []const bson.Pair,
+) ![]const bson.Value {
+    var ctx = tdb.ctx(io);
+    var msg = try parse_fake_msg("distinct", .{ .string = coll }, extra);
+    defer msg.deinit();
+    try dispatch(&ctx, &msg, reply);
+    return switch (bson.get_pair(reply.pairs.items, "values") orelse return error.TestUnexpectedResult) {
+        .array => |a| a,
+        else => error.TestUnexpectedResult,
+    };
+}
+
+test "distinct answers a sorted set, not the order it met the values" {
+    // The load-bearing property, and the one that is not guessable: mongod
+    // sorts the answer in canonical BSON order. Insertion order is the
+    // obvious implementation and it is wrong -- so the documents here are
+    // seeded in an order that tells the two apart, and `null` is included
+    // because its type ranks below strings and would otherwise trail them.
+    //
+    // Mutation check: delete the `std.mem.sort` in cmd_distinct and this
+    // reads ["b", "a", null].
+    var threaded: std.Io.Threaded = .init_single_threaded;
+    defer threaded.deinit();
+    const io = threaded.io();
+    var tdb = try TestDb.init(io);
+    defer tdb.deinit();
+
+    try dispatch_insert(&tdb, io, "d", &.{
+        .{ .doc = &.{ .{ .key = "_id", .value = .{ .int32 = 1 } }, .{ .key = "s", .value = .{ .string = "b" } } } },
+        .{ .doc = &.{ .{ .key = "_id", .value = .{ .int32 = 2 } }, .{ .key = "s", .value = .{ .string = "a" } } } },
+        .{ .doc = &.{ .{ .key = "_id", .value = .{ .int32 = 3 } }, .{ .key = "s", .value = .null } } },
+        // No `s` at all: contributes nothing, where an explicit null contributes
+        // null. Measured -- the two are not the same absence.
+        .{ .doc = &.{.{ .key = "_id", .value = .{ .int32 = 4 } }} },
+        .{ .doc = &.{ .{ .key = "_id", .value = .{ .int32 = 5 } }, .{ .key = "s", .value = .{ .string = "a" } } } },
+    });
+
+    var reply = wire.Reply.init(testing.allocator);
+    defer reply.deinit();
+    const values = try distinct_values(&tdb, io, &reply, "d", &.{
+        .{ .key = "key", .value = .{ .string = "s" } },
+    });
+    try testing.expectEqual(@as(usize, 3), values.len);
+    try testing.expect(values[0] == .null);
+    try testing.expectEqualStrings("a", values[1].string);
+    try testing.expectEqualStrings("b", values[2].string);
+}
+
+test "distinct unwinds a terminal array exactly one level" {
+    // `[[7, 8], 9]` answers `[7, 8]` and `9` -- the inner array is a value,
+    // not something to descend into. Both halves are measured, and both
+    // mutations are visible here: no unwinding at all makes `[1, 2, 2]` a
+    // value, and recursive unwinding turns `[7, 8]` into 7 and 8.
+    var threaded: std.Io.Threaded = .init_single_threaded;
+    defer threaded.deinit();
+    const io = threaded.io();
+    var tdb = try TestDb.init(io);
+    defer tdb.deinit();
+
+    const inner = [_]bson.Value{ .{ .int32 = 7 }, .{ .int32 = 8 } };
+    try dispatch_insert(&tdb, io, "d", &.{
+        .{ .doc = &.{
+            .{ .key = "_id", .value = .{ .int32 = 1 } },
+            .{ .key = "arr", .value = .{ .array = &.{ .{ .int32 = 1 }, .{ .int32 = 2 }, .{ .int32 = 2 } } } },
+        } },
+        // An empty array contributes nothing, the way a missing field does.
+        .{ .doc = &.{
+            .{ .key = "_id", .value = .{ .int32 = 2 } },
+            .{ .key = "arr", .value = .{ .array = &.{} } },
+        } },
+        // A non-array at the key is itself one value.
+        .{ .doc = &.{
+            .{ .key = "_id", .value = .{ .int32 = 3 } },
+            .{ .key = "arr", .value = .{ .string = "not an array" } },
+        } },
+        .{ .doc = &.{
+            .{ .key = "_id", .value = .{ .int32 = 4 } },
+            .{ .key = "arr", .value = .{ .array = &.{ .{ .array = &inner }, .{ .int32 = 9 } } } },
+        } },
+    });
+
+    var reply = wire.Reply.init(testing.allocator);
+    defer reply.deinit();
+    const values = try distinct_values(&tdb, io, &reply, "d", &.{
+        .{ .key = "key", .value = .{ .string = "arr" } },
+    });
+    // Canonical order again: the numbers, then the string, then the array.
+    try testing.expectEqual(@as(usize, 5), values.len);
+    try testing.expectEqual(@as(i32, 1), values[0].int32);
+    try testing.expectEqual(@as(i32, 2), values[1].int32);
+    try testing.expectEqual(@as(i32, 9), values[2].int32);
+    try testing.expectEqualStrings("not an array", values[3].string);
+    try testing.expectEqual(@as(usize, 2), values[4].array.len);
+    try testing.expectEqual(@as(i32, 7), values[4].array[0].int32);
+}
+
+test "distinct dedupes by value, so an int 1 and a double 1.0 are one" {
+    // Deduping and sorting are the same comparator, which is why this falls
+    // out for free -- and why `null` and `"1"` survive alongside it. The
+    // surviving spelling is the first the scan met, because the sort is
+    // stable; a switch to an unstable sort would make this arbitrary.
+    var threaded: std.Io.Threaded = .init_single_threaded;
+    defer threaded.deinit();
+    const io = threaded.io();
+    var tdb = try TestDb.init(io);
+    defer tdb.deinit();
+
+    try dispatch_insert(&tdb, io, "d", &.{
+        .{ .doc = &.{ .{ .key = "_id", .value = .{ .int32 = 1 } }, .{ .key = "m", .value = .{ .int32 = 1 } } } },
+        .{ .doc = &.{ .{ .key = "_id", .value = .{ .int32 = 2 } }, .{ .key = "m", .value = .{ .double = 1.0 } } } },
+        .{ .doc = &.{ .{ .key = "_id", .value = .{ .int32 = 3 } }, .{ .key = "m", .value = .{ .string = "1" } } } },
+        .{ .doc = &.{ .{ .key = "_id", .value = .{ .int32 = 4 } }, .{ .key = "m", .value = .null } } },
+    });
+
+    var reply = wire.Reply.init(testing.allocator);
+    defer reply.deinit();
+    const values = try distinct_values(&tdb, io, &reply, "d", &.{
+        .{ .key = "key", .value = .{ .string = "m" } },
+    });
+    try testing.expectEqual(@as(usize, 3), values.len);
+    try testing.expect(values[0] == .null);
+    try testing.expectEqual(@as(i32, 1), values[1].int32);
+    try testing.expectEqualStrings("1", values[2].string);
+}
+
+test "distinct traverses a path through an array of subdocuments" {
+    var threaded: std.Io.Threaded = .init_single_threaded;
+    defer threaded.deinit();
+    const io = threaded.io();
+    var tdb = try TestDb.init(io);
+    defer tdb.deinit();
+
+    try dispatch_insert(&tdb, io, "d", &.{
+        .{ .doc = &.{
+            .{ .key = "_id", .value = .{ .int32 = 1 } },
+            .{ .key = "n", .value = .{ .doc = &.{.{ .key = "d", .value = .{ .int32 = 5 } }} } },
+        } },
+        // Multikey: both subdocuments contribute, which is the same traversal
+        // the matcher and the index generator use.
+        .{ .doc = &.{
+            .{ .key = "_id", .value = .{ .int32 = 2 } },
+            .{ .key = "n", .value = .{ .array = &.{
+                .{ .doc = &.{.{ .key = "d", .value = .{ .int32 = 1 } }} },
+                .{ .doc = &.{.{ .key = "d", .value = .{ .int32 = 2 } }} },
+            } } },
+        } },
+        .{ .doc = &.{
+            .{ .key = "_id", .value = .{ .int32 = 3 } },
+            .{ .key = "n", .value = .{ .doc = &.{.{ .key = "d", .value = .{ .int32 = 5 } }} } },
+        } },
+    });
+
+    var reply = wire.Reply.init(testing.allocator);
+    defer reply.deinit();
+    const values = try distinct_values(&tdb, io, &reply, "d", &.{
+        .{ .key = "key", .value = .{ .string = "n.d" } },
+    });
+    try testing.expectEqual(@as(usize, 3), values.len);
+    try testing.expectEqual(@as(i32, 1), values[0].int32);
+    try testing.expectEqual(@as(i32, 2), values[1].int32);
+    try testing.expectEqual(@as(i32, 5), values[2].int32);
+}
+
+test "distinct answers the empty set where it has nothing, rather than erroring" {
+    // Three separate ways of having nothing, all of them `ok: 1` with an empty
+    // array rather than an error. The empty key in particular reads like a
+    // malformed request and is not one.
+    var threaded: std.Io.Threaded = .init_single_threaded;
+    defer threaded.deinit();
+    const io = threaded.io();
+    var tdb = try TestDb.init(io);
+    defer tdb.deinit();
+
+    try dispatch_insert(&tdb, io, "d", &.{
+        .{ .doc = &.{ .{ .key = "_id", .value = .{ .int32 = 1 } }, .{ .key = "x", .value = .{ .int32 = 11 } } } },
+    });
+
+    const cases = [_]struct { coll: []const u8, key: []const u8 }{
+        .{ .coll = "no_such_collection", .key = "x" },
+        .{ .coll = "d", .key = "nothing_has_this" },
+        .{ .coll = "d", .key = "" },
+    };
+    for (cases) |c| {
+        var reply = wire.Reply.init(testing.allocator);
+        defer reply.deinit();
+        const values = try distinct_values(&tdb, io, &reply, c.coll, &.{
+            .{ .key = "key", .value = .{ .string = c.key } },
+        });
+        try testing.expectEqual(@as(usize, 0), values.len);
+        try testing.expectEqual(@as(f64, 1.0), bson.get_pair(reply.pairs.items, "ok").?.double);
+    }
+}
+
+test "distinct refuses a malformed request with mongod's own codes" {
+    // Measured against mongod 8.3.7, not recalled: the missing `key` is an IDL
+    // parse failure (40414) while a wrong-typed one is a TypeMismatch (14),
+    // and a *null* query is an empty filter rather than a wrong type.
+    var threaded: std.Io.Threaded = .init_single_threaded;
+    defer threaded.deinit();
+    const io = threaded.io();
+    var tdb = try TestDb.init(io);
+    defer tdb.deinit();
+    var ctx = tdb.ctx(io);
+
+    const key_x = bson.Pair{ .key = "key", .value = .{ .string = "x" } };
+    try testing.expectEqual(@as(?i32, 40414), try run_for_code(&ctx, "distinct", .{ .string = "d" }, &.{}));
+    try testing.expectEqual(@as(?i32, 14), try run_for_code(&ctx, "distinct", .{ .string = "d" }, &.{
+        .{ .key = "key", .value = .{ .int32 = 7 } },
+    }));
+    try testing.expectEqual(@as(?i32, 14), try run_for_code(&ctx, "distinct", .{ .string = "d" }, &.{
+        .{ .key = "key", .value = .{ .doc = &.{} } },
+    }));
+    try testing.expectEqual(@as(?i32, 14), try run_for_code(&ctx, "distinct", .{ .string = "d" }, &.{
+        key_x, .{ .key = "query", .value = .{ .int32 = 7 } },
+    }));
+    try testing.expectEqual(@as(?i32, null), try run_for_code(&ctx, "distinct", .{ .string = "d" }, &.{
+        key_x, .{ .key = "query", .value = .null },
+    }));
+    // An unknown top-level field is tolerated. mongod's IDL refuses it with
+    // 40415, but this server tolerates unknown fields on every command, and
+    // `comment` and `rawData` -- which the CRUD corpus requires be ignored --
+    // arrive through exactly this door.
+    try testing.expectEqual(@as(?i32, null), try run_for_code(&ctx, "distinct", .{ .string = "d" }, &.{
+        key_x,
+        .{ .key = "comment", .value = .{ .string = "c" } },
+        .{ .key = "rawData", .value = .{ .bool = true } },
+    }));
+
+    // The message text is mongod's, reproduced rather than paraphrased,
+    // because a driver that matches on it is matching on this.
+    var reply = wire.Reply.init(testing.allocator);
+    defer reply.deinit();
+    var msg = try parse_fake_msg("distinct", .{ .string = "d" }, &.{});
+    defer msg.deinit();
+    try dispatch(&ctx, &msg, &reply);
+    try testing.expectEqualStrings(
+        "BSON field 'distinctCommandRequest.key' is missing but a required field",
+        bson.get_pair(reply.pairs.items, "errmsg").?.string,
+    );
+}
+
+test "distinct applies its filter before collecting" {
+    var threaded: std.Io.Threaded = .init_single_threaded;
+    defer threaded.deinit();
+    const io = threaded.io();
+    var tdb = try TestDb.init(io);
+    defer tdb.deinit();
+
+    try dispatch_insert(&tdb, io, "d", &.{
+        .{ .doc = &.{ .{ .key = "_id", .value = .{ .int32 = 1 } }, .{ .key = "x", .value = .{ .int32 = 11 } } } },
+        .{ .doc = &.{ .{ .key = "_id", .value = .{ .int32 = 2 } }, .{ .key = "x", .value = .{ .int32 = 22 } } } },
+        .{ .doc = &.{ .{ .key = "_id", .value = .{ .int32 = 3 } }, .{ .key = "x", .value = .{ .int32 = 33 } } } },
+    });
+
+    var reply = wire.Reply.init(testing.allocator);
+    defer reply.deinit();
+    const values = try distinct_values(&tdb, io, &reply, "d", &.{
+        .{ .key = "key", .value = .{ .string = "x" } },
+        .{ .key = "query", .value = .{ .doc = &.{
+            .{ .key = "_id", .value = .{ .doc = &.{.{ .key = "$gt", .value = .{ .int32 = 1 } }} } },
+        } } },
+    });
+    try testing.expectEqual(@as(usize, 2), values.len);
+    try testing.expectEqual(@as(i32, 22), values[0].int32);
+    try testing.expectEqual(@as(i32, 33), values[1].int32);
+}
+
 test "aggregate $sort without a preceding $group sorts and frees correctly" {
     // Regression test for a remote, client-triggerable invalid free: the
     // $sort stage materialized its document list from the reply arena and

tests/spec/scorecard.txt

@@ -18,7 +18,7 @@
 # hasServerConnectionId, and maxTimeMS in an expected command (CSOT rewrites
 # it -- the only assertion this runner declines to make).
 
-total	201 pass	90 fail	196 skip	175 files	0 errored
+total	204 pass	87 fail	196 skip	175 files	0 errored
 
 # per-file: name	pass	fail	skip
 aggregate-allowdiskuse.json	3	0	0
@@ -110,8 +110,8 @@ deleteOne.json	3	0	0
 distinct-collation.json	0	1	0
 distinct-comment.json	1	1	1
 distinct-hint.json	0	0	2
-distinct-rawdata.json	0	1	1
+distinct-rawdata.json	1	0	1
-distinct.json	0	2	0
+distinct.json	2	0	0
 estimatedDocumentCount-comment.json	1	1	1
 estimatedDocumentCount-rawdata.json	1	0	1
 estimatedDocumentCount.json	2	1	3
@@ -318,14 +318,11 @@ deleteOne-hint-unacknowledged.json	SKIP	Unacknowledged deleteOne with hint docum
 deleteOne-let.json	SKIP	deleteOne with let option	needs server >= 5.0
 deleteOne-let.json	FAIL	deleteOne with let option unsupported (server-side error)	deleteOne: expected an error, the operation succeeded
 deleteOne-rawdata.json	SKIP	deleteOne with rawData option	needs server >= 8.2.0
-distinct-collation.json	FAIL	Distinct with a collation	MongoServerError: no such command: 'distinct'
+distinct-collation.json	FAIL	Distinct with a collation	distinct: expected 1 elements, got 2
 distinct-comment.json	SKIP	distinct with document comment	needs server >= 4.4.14
-distinct-comment.json	FAIL	distinct with string comment	MongoServerError: no such command: 'distinct'
+distinct-comment.json	FAIL	distinct with document comment - pre 4.4, server error	distinct: expected an error, the operation succeeded
 distinct-hint.json	SKIP	*	needs server >= 7.1.0
 distinct-rawdata.json	SKIP	distinct with rawData option	needs server >= 8.2.0
-distinct-rawdata.json	FAIL	distinct with rawData option on less than 8.2.0 - ignore argument	MongoServerError: no such command: 'distinct'
-distinct.json	FAIL	Distinct without a filter	MongoServerError: no such command: 'distinct'
-distinct.json	FAIL	Distinct with a filter	MongoServerError: no such command: 'distinct'
 estimatedDocumentCount-comment.json	SKIP	estimatedDocumentCount with document comment	needs server >= 4.4.14
 estimatedDocumentCount-comment.json	FAIL	estimatedDocumentCount with document comment - pre 4.4.14, server error	estimatedDocumentCount: expected an error, the operation succeeded
 estimatedDocumentCount-rawdata.json	SKIP	Estimated document count with rawData option	needs server >= 8.2.0