diff --git a/src/commands.zig b/src/commands.zig index 0571523..b9bda5c 100644 --- a/src/commands.zig +++ b/src/commands.zig @@ -88,6 +88,12 @@ pub const ErrorCode = enum(i32) { index_key_specs_conflict = 86, cannot_create_index = 67, invalid_index_specification_option = 197, + /// The three hashed-index codes, measured on mongod 8.3.7. They are bare + /// location numbers with no name in `error_codes.yml`, so the reply's + /// `codeName` is the generic one -- which is what mongod itself sends. + hashed_unique = 16764, + hashed_array_value = 16766, + hashed_two_components = 31303, // Cursor codes. Taken from MongoDB's own error_codes.js rather than // recalled -- CursorInUse in particular is 143, not the 12051 that turns up // in older notes. @@ -951,6 +957,21 @@ fn cmd_create_indexes(ctx: *Context, msg: *wire.Message, reply: *wire.Reply) !vo "CannotCreateIndex", "unsupported expression in partialFilterExpression", ), + error.UnknownIndexPlugin => return reply.put_error( + @intFromEnum(ErrorCode.cannot_create_index), + "CannotCreateIndex", + "Unknown index plugin", + ), + error.TwoHashedComponents => return reply.put_error( + @intFromEnum(ErrorCode.hashed_two_components), + "Location31303", + "A maximum of one index field is allowed to be hashed", + ), + error.UniqueHashed => return reply.put_error( + @intFromEnum(ErrorCode.hashed_unique), + "Location16764", + "Currently hashed indexes cannot guarantee uniqueness. Use a regular index.", + ), error.PartialFilterNotDocument => return reply.put_error( @intFromEnum(ErrorCode.type_mismatch), "TypeMismatch", @@ -986,6 +1007,9 @@ fn cmd_create_indexes(ctx: *Context, msg: *wire.Message, reply: *wire.Reply) !vo return reply.put_error(@intFromEnum(ErrorCode.duplicate_key), "DuplicateKey", msg_text); }, error.ParallelArrays => return bad_value(reply, "cannot index parallel arrays"), + // Reachable here as well as on insert: a collection can already + // hold the array the new index cannot hash. + error.HashedArray => return hashed_array_error(reply), else => return err, }; } @@ -1137,6 +1161,12 @@ fn cmd_insert(ctx: *Context, msg: *wire.Message, reply: *wire.Reply) !void { e[2] = .{ .key = "errmsg", .value = .{ .string = try duplicate_key_message(ctx, reply, db_name, coll_name, doc) } }; try write_errors.append(reply.arena_alloc(), .{ .doc = e }); }, + // Measured: a per-document writeError with `ok: 1`, not a + // command error -- the rest of the batch still goes in. + error.HashedArray => try write_errors.append( + reply.arena_alloc(), + try hashed_array_write_error(reply, i), + ), else => return err, } } @@ -2052,6 +2082,7 @@ fn cmd_update(ctx: *Context, msg: *wire.Message, reply: *wire.Reply) !void { const new_doc = (try build_upsert_doc(reply, ctx, q, u_doc, u_pipeline, opts, &diag)) orelse return; ctx.engine.insert(db_name, coll_name, new_doc, ctx.oid_gen) catch |err| switch (err) { error.DuplicateKey, error.DuplicateKeyIndex => return duplicate_key_error(ctx, reply, db_name, coll_name, new_doc), + error.HashedArray => return hashed_array_error(reply), else => return err, }; const id = new_doc.get("_id") orelse bson.Value.null; @@ -2087,6 +2118,13 @@ fn cmd_update(ctx: *Context, msg: *wire.Message, reply: *wire.Reply) !void { try write_errors.append(reply.arena_alloc(), .{ .doc = e }); continue; }, + error.HashedArray => { + try write_errors.append( + reply.arena_alloc(), + try hashed_array_write_error(reply, si), + ); + continue; + }, else => return err, }; // `n` counts matches, `nModified` counts documents the update @@ -2208,6 +2246,7 @@ fn cmd_find_and_modify(ctx: *Context, msg: *wire.Message, reply: *wire.Reply) !v const new_doc = (try build_upsert_doc(reply, ctx, q, u_doc, u_pipeline, opts, &diag)) orelse return; ctx.engine.insert(db_name, coll_name, new_doc, ctx.oid_gen) catch |err| switch (err) { error.DuplicateKey, error.DuplicateKeyIndex => return duplicate_key_error(ctx, reply, db_name, coll_name, new_doc), + error.HashedArray => return hashed_array_error(reply), else => return err, }; n = 1; @@ -2230,7 +2269,12 @@ fn cmd_find_and_modify(ctx: *Context, msg: *wire.Message, reply: *wire.Reply) !v }; // findAndModify reports `n` (matched) and `updatedExisting`, neither of // which distinguishes a no-op, so whether it wrote is not needed here. - _ = try ctx.engine.replace(db_name, coll_name, copy, ctx.oid_gen); + // It has no writeErrors array either, so a document a hashed index + // cannot take is a command error here rather than a per-write one. + _ = ctx.engine.replace(db_name, coll_name, copy, ctx.oid_gen) catch |err| switch (err) { + error.HashedArray => return hashed_array_error(reply), + else => return err, + }; n = 1; updated_existing = true; value = if (ret_new) try project_doc(reply, copy, proj_pairs) else .{ .doc = before }; @@ -4781,6 +4825,31 @@ fn bad_value(reply: *wire.Reply, msg: []const u8) !void { return reply.put_error(@intFromEnum(ErrorCode.bad_value), "BadValue", msg); } +/// What a hashed index answers for a document holding an array at its path. +/// Measured on mongod 8.3.7: refused when the *document* arrives rather than +/// when the index is created, because there is no single hash for an array. +const hashed_array_message = "hashed indexes do not currently support array values"; + +/// The createIndexes shape: a command error, because the index does not come +/// into existence at all. Measured, `codeName` included. +fn hashed_array_error(reply: *wire.Reply) !void { + return reply.put_error( + @intFromEnum(ErrorCode.hashed_array_value), + "Location16766", + hashed_array_message, + ); +} + +/// The write shape: one entry in `writeErrors` beside `ok: 1`, so the rest of +/// a batch still lands. A writeError carries no `codeName`, measured. +fn hashed_array_write_error(reply: *wire.Reply, i: usize) !bson.Value { + const e = try reply.arena_alloc().alloc(bson.Pair, 3); + e[0] = .{ .key = "index", .value = .{ .int32 = @intCast(i) } }; + e[1] = .{ .key = "code", .value = .{ .int32 = @intFromEnum(ErrorCode.hashed_array_value) } }; + e[2] = .{ .key = "errmsg", .value = .{ .string = hashed_array_message } }; + return .{ .doc = e }; +} + fn failed_to_parse(reply: *wire.Reply, msg: []const u8) !void { return reply.put_error(@intFromEnum(ErrorCode.failed_to_parse), "FailedToParse", msg); } @@ -5706,6 +5775,24 @@ test "TTL index round-trips through createIndexes/listIndexes; bad specs give 67 .{ .key = "t", .value = .{ .bool = true } }, } } }, } } }, + // The three hashed refusals. Each code is a bare location number + // measured on mongod 8.3.7, and each is a different kind of no: two + // hashed components order nothing, a unique hashed index cannot tell + // a duplicate from a collision, and an unrecognised string names an + // index type this server does not have. + .{ .code = 31303, .spec = .{ .doc = &.{ + .{ .key = "key", .value = .{ .doc = &.{ + .{ .key = "a", .value = .{ .string = "hashed" } }, + .{ .key = "b", .value = .{ .string = "hashed" } }, + } } }, + } } }, + .{ .code = 16764, .spec = .{ .doc = &.{ + .{ .key = "key", .value = .{ .doc = &.{.{ .key = "a", .value = .{ .string = "hashed" } }} } }, + .{ .key = "unique", .value = .{ .bool = true } }, + } } }, + .{ .code = 67, .spec = .{ .doc = &.{ + .{ .key = "key", .value = .{ .doc = &.{.{ .key = "a", .value = .{ .string = "bogus" } }} } }, + } } }, }; for (bad) |case| { var ctx = tdb.ctx(io); @@ -5777,6 +5864,65 @@ test "unique index constraint returns 11000 through insert and update" { try testing.expectEqual(@as(i64, 11000), bson.get_pair(up_errs[0].doc, "code").?.int32); } +test "an array under a hashed index fails its own document and no other" { + // Measured on mongod 8.3.7: `ok: 1` with one writeError, not a command + // error -- so the rest of the batch still lands. Getting this wrong the + // other way would turn one bad document into a whole failed insert. + 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_create_index(&tdb, io, "evt", .{ .doc = &.{ + .{ .key = "key", .value = .{ .doc = &.{.{ .key = "a", .value = .{ .string = "hashed" } }} } }, + } }); + + const docs = [_]bson.Value{ + .{ .doc = &.{ .{ .key = "_id", .value = .{ .int32 = 1 } }, .{ .key = "a", .value = .{ .int32 = 7 } } } }, + .{ .doc = &.{ .{ .key = "_id", .value = .{ .int32 = 2 } }, .{ .key = "a", .value = .{ .array = &.{ + .{ .int32 = 1 }, + .{ .int32 = 2 }, + } } } } }, + .{ .doc = &.{ .{ .key = "_id", .value = .{ .int32 = 3 } }, .{ .key = "a", .value = .{ .int32 = 8 } } } }, + }; + var ctx = tdb.ctx(io); + var msg = try parse_fake_msg("insert", .{ .string = "evt" }, &.{ + .{ .key = "documents", .value = .{ .array = &docs } }, + }); + defer msg.deinit(); + var reply = wire.Reply.init(testing.allocator); + defer reply.deinit(); + try dispatch(&ctx, &msg, &reply); + try testing.expectEqual(@as(f64, 1.0), bson.get_pair(reply.pairs.items, "ok").?.double); + try testing.expectEqual(@as(i64, 2), bson.get_pair(reply.pairs.items, "n").?.int32); + const errors = bson.get_pair(reply.pairs.items, "writeErrors").?.array; + try testing.expectEqual(@as(usize, 1), errors.len); + try testing.expectEqual(@as(i64, 1), bson.get_pair(errors[0].doc, "index").?.int32); + try testing.expectEqual(@as(i64, 16766), bson.get_pair(errors[0].doc, "code").?.int32); + + // The same value arriving through an update is the same answer, and the + // document is left as it was. Mutation: return the error instead of + // appending a writeError and `ok` goes to 0 here. + const updates = [_]bson.Value{.{ .doc = &.{ + .{ .key = "q", .value = .{ .doc = &.{.{ .key = "_id", .value = .{ .int32 = 3 } }} } }, + .{ .key = "u", .value = .{ .doc = &.{.{ .key = "$set", .value = .{ .doc = &.{ + .{ .key = "a", .value = .{ .array = &.{.{ .int32 = 1 }} } }, + } } }} } }, + } }}; + var msg2 = try parse_fake_msg("update", .{ .string = "evt" }, &.{ + .{ .key = "updates", .value = .{ .array = &updates } }, + }); + defer msg2.deinit(); + var reply2 = wire.Reply.init(testing.allocator); + defer reply2.deinit(); + try dispatch(&ctx, &msg2, &reply2); + try testing.expectEqual(@as(f64, 1.0), bson.get_pair(reply2.pairs.items, "ok").?.double); + try testing.expectEqual(@as(i64, 0), bson.get_pair(reply2.pairs.items, "nModified").?.int32); + const up_errs = bson.get_pair(reply2.pairs.items, "writeErrors").?.array; + try testing.expectEqual(@as(i64, 16766), bson.get_pair(up_errs[0].doc, "code").?.int32); +} + /// Free a list of serialized ids (each element is gpa-owned). fn free_id_list(gpa: std.mem.Allocator, list: *std.ArrayListUnmanaged([]u8)) void { for (list.items) |id| gpa.free(id); diff --git a/src/db.zig b/src/db.zig index 546c166..58f585a 100644 --- a/src/db.zig +++ b/src/db.zig @@ -242,7 +242,7 @@ pub const Collection = struct { .id_index = undefined, .layout_epoch = layout_epoch, }; - const keys = [_]index.IndexKey{.{ .path = "_id", .descending = false }}; + const keys = [_]index.IndexKey{.{ .path = "_id" }}; // unique: the tree, not the docs map, is what enforces _id uniqueness // now (PLAN A3/A4). It is keyed on bson.encode_key, which is canonical // where serialize_value is not, so int32 1 / int64 1 / double 1.0 @@ -2247,7 +2247,12 @@ pub const Engine = struct { ix.reset_tree(self.gpa) catch |err| return err; for (moved) |m| { ix.append_doc_entries(self.gpa, doc_bytes_in(self.pager, m.off), m.off) catch |err| switch (err) { - error.ParallelArrays => continue, + // Neither is reachable: an insert and a createIndex both + // refuse the document that would produce it. Skipped rather + // than propagated for the reason `rebuild_index` states -- + // a maintenance task must not be able to take the database + // down over one document it cannot index. + error.ParallelArrays, error.HashedArray => continue, else => return err, }; } @@ -2350,7 +2355,7 @@ pub const Engine = struct { var doc_it = coll.id_index.iter(); while (doc_it.next()) |entry| { ix.append_doc_entries(self.gpa, coll.doc_bytes(entry.off), entry.off) catch |err| switch (err) { - error.ParallelArrays => { + error.ParallelArrays, error.HashedArray => { std.debug.print( "multiforadb: WARNING: index '{s}' cannot index an existing " ++ "document; entry skipped\n", @@ -2495,7 +2500,13 @@ pub const Engine = struct { try put_u32(gpa, out, @intCast(ix.keys.len)); for (ix.keys) |k| { try put_bytes(gpa, out, k.path); - try out.append(gpa, @intFromBool(k.descending)); + // The per-component byte that used to be a bare `descending` + // flag. It has only ever held 0 or 1, so a third value costs no + // format change: an older file reads back exactly as it did, and + // `catalog_version` stays 1. That is also why hashed is *not* a + // sixth bit in the flags byte below -- hashed belongs to a + // component, and a compound index may hold one beside range ones. + try out.append(gpa, @intFromEnum(k.kind)); } var flags: u8 = 0; if (ix.unique) flags |= 1; @@ -2626,11 +2637,22 @@ pub const Engine = struct { for (keys[0..built]) |k| gpa.free(k.path); gpa.free(keys); } + var hashed: usize = 0; while (built < nkeys) : (built += 1) { const path = try r.read_bytes(); - keys[built] = .{ .path = try gpa.dupe(u8, path), .descending = (try r.read_byte()) != 0 }; + const kind_byte = try r.read_byte(); + if (kind_byte > @intFromEnum(index.KeyKind.hashed)) return error.CorruptCatalog; + const kind: index.KeyKind = @enumFromInt(kind_byte); + if (kind == .hashed) hashed += 1; + keys[built] = .{ .path = try gpa.dupe(u8, path), .kind = kind }; } + // The two rules `parse_spec` enforces, restated against the file: + // `Index.init` asserts both, and this reader swaps its keys in behind + // init's back, so a file claiming otherwise has to be rejected here or + // it would trip an assertion in a build that has them on. + if (hashed > 1) return error.CorruptCatalog; const flags = try r.read_byte(); + if (hashed > 0 and flags & 1 != 0) return error.CorruptCatalog; const ttl_raw: i64 = @bitCast(try r.read_u64()); // Read in the same order it was written, and *before* the identity // swap below: a corrupt filter has to fail with the index untouched. @@ -6092,3 +6114,61 @@ test "a unique partial index constrains only the documents its filter selects" { try engine.insert("app", "acct", &outside, &env.gen); } } + +test "a hashed key survives a checkpoint as a hash, not as a direction" { + // The per-component byte the catalog has always written held 0 or 1 and + // now holds 2. Reading it back as a bool would leave an index whose + // entries are hashes and whose lookups are not -- a tree that answers + // nothing and reports no error. + // + // Mutation check: write `@intFromBool(k.kind == .descending)` in + // `write_index_catalog` and the lookup after the reopen returns 0 rows. + var threaded: std.Io.Threaded = .init_single_threaded; + defer threaded.deinit(); + var env = test_env(&threaded); + const io = env.io; + const gpa = testing.allocator; + + var tmp = try TmpLog.init(gpa); + defer tmp.deinit(gpa); + const spec = bson.Document{ .arena = undefined, .pairs = &.{ + .{ .key = "key", .value = .{ .doc = &.{.{ .key = "a", .value = .{ .string = "hashed" } }} } }, + } }; + { + var engine = try Engine.open(gpa, io, tmp.path); + defer engine.deinit(); + try engine.lock(); + defer engine.unlock(); + const ix = try engine.create_index("app", "acct", &spec); + try testing.expectEqualStrings("a_hashed", ix.name); + + for ([_]i32{ 1, 2, 3 }) |id| { + var doc = try doc_at(gpa, id, id * 10, true); + defer doc.deinit(); + try engine.insert("app", "acct", &doc, &env.gen); + } + try engine.checkpoint(); + } + + { + var engine = try Engine.open(gpa, io, tmp.path); + defer engine.deinit(); + const coll = engine.get_collection("app", "acct").?; + const ix = coll.indexes.items[0]; + try testing.expectEqual(index.KeyKind.hashed, ix.keys[0].kind); + try testing.expectEqual(@as(usize, 3), ix.count()); + + // Found through the reopened tree, and through a numeric type the + // document was not stored with -- the two halves of the encoding + // agreeing across a restart. + var offs: std.ArrayListUnmanaged(u64) = .empty; + defer offs.deinit(gpa); + try ix.lookup_eq(gpa, &.{.{ .double = 20.0 }}, &offs); + try testing.expectEqual(@as(usize, 1), offs.items.len); + + // And uniqueness is still off, which is what makes a collision + // harmless: `unique` was refused at creation, so nothing here depends + // on two distinct values having distinct hashes. + try testing.expect(!ix.unique); + } +} diff --git a/src/fuzz_split.zig b/src/fuzz_split.zig index 9179e9c..4003aae 100644 --- a/src/fuzz_split.zig +++ b/src/fuzz_split.zig @@ -59,7 +59,7 @@ fn run(seed: u64, ops: usize, max_len: usize) !void { pg.deinit(); gpa.destroy(pg); } - var ix = try index.Index.init(gpa, pg, "s_1", &.{.{ .path = "s", .descending = false }}, false, false, null); + var ix = try index.Index.init(gpa, pg, "s_1", &.{.{ .path = "s" }}, false, false, null); defer ix.deinit(gpa); var docs: std.ArrayListUnmanaged(Doc) = .empty; diff --git a/src/index.zig b/src/index.zig index 084ab72..dbc9e19 100644 --- a/src/index.zig +++ b/src/index.zig @@ -56,10 +56,31 @@ pub const max_index_keys: usize = 32; /// planner falls back to a scan. const max_combos: u64 = 100; +/// How one key component is stored. +/// +/// The enum values *are* the on-disk encoding: `write_index_catalog` has +/// always written one byte per component, and that byte has only ever held 0 +/// or 1. A third value costs no format change and no `catalog_version` bump, +/// which is why hashed lives here rather than in the index-wide flags byte +/// the design review guessed at -- hashed is a property of a component, and +/// a compound index may hold one hashed component beside range ones. +pub const KeyKind = enum(u8) { + ascending = 0, + descending = 1, + /// The tree holds a hash of the value rather than the value. Ordered by + /// that hash, which is to say not usefully ordered at all: such a + /// component answers equality and nothing else. + hashed = 2, +}; + /// One key in an index spec. `path` is owned by the Index. pub const IndexKey = struct { path: []const u8, - descending: bool, + kind: KeyKind = .ascending, + + pub fn descending(self: IndexKey) bool { + return self.kind == .descending; + } }; /// One index entry: the gpa-owned encoded form of a document's values for the @@ -183,6 +204,74 @@ pub const EntryRef = struct { /// unchanged by the switch away from id bytes. const off_len: u32 = @sizeOf(u64); +/// The stored form of a hashed component: a tag byte and a 64-bit hash, big +/// endian for the same reason datetimes are -- every other fixed-width column +/// in an encoded key reads that way, even where the order is meaningless. +/// +/// The hash is taken over the value's *ordinary* encoded bytes, not over the +/// value, and that is what makes `{a: 5}` and `{a: 5.0}` land on the same +/// entry for free: `bson.encode_key` already normalizes every numeric type +/// through f128, because two values that compare `.eq` have to encode +/// identically for the tree to be a memcmp. +/// +/// Collisions are harmless. This file's governing invariant is that an index +/// only generates candidates and the full filter is re-applied to every one, +/// so two values sharing a hash cost a rejected candidate and nothing else. +/// The single place that would *not* survive a collision is uniqueness, which +/// is why `unique` on a hashed index is refused outright. +const hashed_tag: u8 = 0xff; +const hashed_width: usize = 1 + @sizeOf(u64); + +/// Whether an array appears anywhere along `path` in the document `bytes` -- +/// at the end of the path or stepped through on the way to it. +/// +/// The one question a hashed component asks of a document, and it has its own +/// walker rather than a flag on `query.collect_values_bytes` for two reasons: +/// that function is on every filter's hot path, and an array *through* a path +/// is invisible in its output anyway. `{a: [{b: 1}]}` and `{a: {b: 1}}` both +/// yield exactly one value for `a.b`, and mongod refuses the first -- measured +/// on 8.3.7, which reports "Found array at path: a" for a one-element array +/// just as it does for a longer one. +fn array_on_path(bytes: []const u8, path: []const u8) bool { + var it = std.mem.splitScalar(u8, path, '.'); + const first = it.next() orelse return false; + const rest = it.rest(); + + var idx: usize = 4; // skip the document length prefix + while (idx + 1 < bytes.len and bytes[idx] != 0) { + const tag = bytes[idx]; + idx += 1; + const key = bson.element_key(bytes, &idx) orelse return false; + if (!std.mem.eql(u8, key, first)) { + bson.skip_value(bytes, &idx, tag) catch return false; + continue; + } + if (tag == 0x04) return true; // array + if (rest.len == 0) return false; + if (tag != 0x03) return false; // not a document: the path stops here + return array_on_path(bytes[idx..], rest); + } + return false; +} + +fn hashed_components(keys: []const IndexKey) usize { + var n: usize = 0; + for (keys) |k| { + if (k.kind == .hashed) n += 1; + } + return n; +} + +fn encode_hashed(v: bson.Value, gpa: std.mem.Allocator, out: *std.ArrayListUnmanaged(u8)) !void { + var plain: std.ArrayListUnmanaged(u8) = .empty; + defer plain.deinit(gpa); + try bson.encode_key(v, gpa, &plain); + var buf: [hashed_width]u8 = undefined; + buf[0] = hashed_tag; + std.mem.writeInt(u64, buf[1..][0..8], std.hash.XxHash3.hash(0, plain.items), .big); + try out.appendSlice(gpa, &buf); +} + pub const Index = struct { name: []const u8, keys: []const IndexKey, @@ -305,9 +394,17 @@ pub const Index = struct { gpa.free(self.name); } while (n < keys.len) : (n += 1) { - owned_keys[n] = .{ .path = try gpa.dupe(u8, keys[n].path), .descending = keys[n].descending }; + owned_keys[n] = .{ .path = try gpa.dupe(u8, keys[n].path), .kind = keys[n].kind }; } self.keys = owned_keys; + assert_msg(hashed_components(owned_keys) <= 1, "an index has at most one hashed component"); + // A hash is not evidence that two values differ, so a hashed tree + // cannot tell a duplicate from a collision. mongod refuses the + // combination for the same reason (16764) and `parse_spec` reports it. + assert_msg( + !(unique and hashed_components(owned_keys) > 0), + "a hashed index cannot enforce uniqueness", + ); // Slot 0 is a dummy (0 is the null node id); the root is one empty // leaf, so a fresh index is always a valid tree. try self.node_pages.ensureUnusedCapacity(gpa, 2); @@ -353,6 +450,23 @@ pub const Index = struct { // -- entry generation --------------------------------------------------- + /// Encode component `i` of a key the way *this* index stores it. + /// + /// The single definition, shared by entry generation and by both lookup + /// paths. A hashed component that were hashed on the way in and not on + /// the way out would simply never find anything, which is the kind of + /// bug a search silently returns zero rows for. + fn encode_component( + self: *const Index, + i: usize, + v: bson.Value, + gpa: std.mem.Allocator, + out: *std.ArrayListUnmanaged(u8), + ) !void { + if (self.keys[i].kind == .hashed) return encode_hashed(v, gpa, out); + try bson.encode_key(v, gpa, out); + } + /// Build the entries one document contributes. Mirrors field_matches /// (src/query.zig): the value at each path plus the elements of any /// array there, so both `{tags: "a"}` element queries and whole-array @@ -392,6 +506,12 @@ pub const Index = struct { try query.collect_values_bytes(a, doc, k.path, &values, 0); // Index the array itself and each element, like field_matches. const direct = values.items.len; + // Except under a hashed component, where mongod refuses the + // document instead -- at *insert* time, not at creation (16766). + // A hash of an array and a hash of its elements are unrelated + // values, so there is no reading of "multikey" a hashed tree + // could answer equality from. + if (k.kind == .hashed and array_on_path(doc, k.path)) return error.HashedArray; var i: usize = 0; while (i < direct) : (i += 1) { if (values.items[i] == .array) { @@ -425,7 +545,7 @@ pub const Index = struct { var enc: std.ArrayListUnmanaged(u8) = .empty; while (true) { enc.clearRetainingCapacity(); - for (0..nkeys) |ci| try bson.encode_key(per_path.items[ci].items[choice[ci]], a, &enc); + for (0..nkeys) |ci| try self.encode_component(ci, per_path.items[ci].items[choice[ci]], a, &enc); const key = try gpa.dupe(u8, enc.items); errdefer gpa.free(key); try out.append(gpa, .{ .key = key }); @@ -761,7 +881,7 @@ pub const Index = struct { ) !void { var enc: std.ArrayListUnmanaged(u8) = .empty; defer enc.deinit(gpa); - for (key) |v| try bson.encode_key(v, gpa, &enc); + for (key, 0..) |v, i| try self.encode_component(i, v, gpa, &enc); var it = self.seek(enc.items); while (it.next()) |e| { if (cmp_prefix(enc.items, e.key) != .eq) break; @@ -791,11 +911,19 @@ pub const Index = struct { var enc_b: std.ArrayListUnmanaged(u8) = .empty; defer enc_a.deinit(gpa); defer enc_b.deinit(gpa); - for (prefix) |v| try bson.encode_key(v, gpa, &enc_a); + // The bounds land on the component after the prefix, and that + // component is never hashed: a hash orders nothing, so `evaluate_index` + // refuses to put a range on one. Asserted rather than handled -- + // encoding a bound as a hash would silently return the wrong rows. + assert_msg( + (lo == null and hi == null) or self.keys[prefix.len].kind != .hashed, + "a range bound on a hashed component", + ); + for (prefix, 0..) |v, i| try self.encode_component(i, v, gpa, &enc_a); const prefix_len = enc_a.items.len; if (lo) |l| try bson.encode_key(l, gpa, &enc_a); if (hi) |h| { - for (prefix) |v| try bson.encode_key(v, gpa, &enc_b); + for (prefix, 0..) |v, i| try self.encode_component(i, v, gpa, &enc_b); try bson.encode_key(h, gpa, &enc_b); } const prefix_key = enc_a.items[0..prefix_len]; @@ -1184,7 +1312,13 @@ pub const Index = struct { try out.append(arena, .{ .key = "v", .value = .{ .int32 = 2 } }); const key_pairs = try arena.alloc(bson.Pair, self.keys.len); for (self.keys, 0..) |k, i| { - key_pairs[i] = .{ .key = k.path, .value = if (k.descending) .{ .int32 = -1 } else .{ .int32 = 1 } }; + key_pairs[i] = .{ .key = k.path, .value = switch (k.kind) { + .ascending => .{ .int32 = 1 }, + .descending => .{ .int32 = -1 }, + // A client is told the plugin's name, never the hash -- which + // is what leaves the encoding a private choice. + .hashed => .{ .string = "hashed" }, + } }; } try out.append(arena, .{ .key = "key", .value = .{ .doc = key_pairs } }); try out.append(arena, .{ .key = "name", .value = .{ .string = self.name } }); @@ -1211,7 +1345,7 @@ pub const Index = struct { if (a.keys.len != b.keys.len) return false; for (a.keys, b.keys) |ka, kb| { if (!std.mem.eql(u8, ka.path, kb.path)) return false; - if (ka.descending != kb.descending) return false; + if (ka.kind != kb.kind) return false; } return true; } @@ -2060,7 +2194,14 @@ pub fn find_by_key_pattern(indexes: []const *Index, key_pairs: []const bson.Pair if (ix.keys.len != key_pairs.len) continue; var match = true; for (ix.keys, key_pairs) |k, kp| { - if (!std.mem.eql(u8, k.path, kp.key) or k.descending != descending(kp.value)) { + // A pattern component this server cannot name at all matches + // nothing, which is what an unknown plugin should do here: the + // caller is naming an index it could not have created. + const kind = key_kind(kp.value) catch { + match = false; + break; + }; + if (!std.mem.eql(u8, k.path, kp.key) or k.kind != kind) { match = false; break; } @@ -2084,20 +2225,18 @@ pub fn parse_spec(gpa: std.mem.Allocator, pager: *pgr.Pager, spec: *const bson.D }; if (key_pairs.len == 0 or key_pairs.len > max_index_keys) return error.InvalidIndexSpec; var keys: [max_index_keys]IndexKey = undefined; - for (key_pairs, 0..) |p, i| { - const ok_mag = switch (p.value) { - .int32 => |n| n == 1 or n == -1, - .int64 => |n| n == 1 or n == -1, - .double => |n| n == 1.0 or n == -1.0, - else => false, - }; - if (!ok_mag) return error.InvalidIndexSpec; - keys[i] = .{ .path = p.key, .descending = descending(p.value) }; - } + for (key_pairs, 0..) |p, i| keys[i] = .{ .path = p.key, .kind = try key_kind(p.value) }; + // Measured: 31303. Two hashed components would each need their own + // equality predicate to be usable at all, and their product orders + // nothing, so mongod caps it at one and so does this. + if (hashed_components(keys[0..key_pairs.len]) > 1) return error.TwoHashedComponents; var unique = false; var sparse = false; if (bson.get_pair(spec.pairs, "unique")) |v| unique = query.truthy(v); if (bson.get_pair(spec.pairs, "sparse")) |v| sparse = query.truthy(v); + // Measured: 16764. A hash is not evidence that two values differ, so the + // tree cannot tell a duplicate from a collision. + if (unique and hashed_components(keys[0..key_pairs.len]) > 0) return error.UniqueHashed; var ttl: ?i64 = null; if (bson.get_pair(spec.pairs, "expireAfterSeconds")) |v| { ttl = try expire_after_seconds(v); @@ -2203,19 +2342,30 @@ fn expire_after_seconds(v: bson.Value) error{InvalidExpireAfterSeconds}!i64 { return secs; } -/// Whether a key-pattern direction value means descending. The single -/// definition of what -1 means in a key pattern, shared with dropIndexes' -/// key-pattern matching. -pub fn descending(v: bson.Value) bool { - return switch (v) { - .int32 => |n| n < 0, - .int64 => |n| n < 0, - .double => |n| n < 0, - else => false, +/// What one key-pattern value asks for. The single definition of what 1, -1 +/// and `"hashed"` mean in a key pattern, shared with dropIndexes' key-pattern +/// matching. +/// +/// A string names an index type, so an unrecognised one is `UnknownIndexPlugin` +/// (67) rather than a malformed spec -- measured, and the distinction matters +/// to a client that is probing for a capability. +pub fn key_kind(v: bson.Value) error{ InvalidIndexSpec, UnknownIndexPlugin }!KeyKind { + const magnitude: f64 = switch (v) { + .int32 => |n| @floatFromInt(n), + .int64 => |n| @floatFromInt(n), + .double => |n| n, + .string => |s| { + if (std.mem.eql(u8, s, "hashed")) return .hashed; + return error.UnknownIndexPlugin; + }, + else => return error.InvalidIndexSpec, }; + if (magnitude == 1.0) return .ascending; + if (magnitude == -1.0) return .descending; + return error.InvalidIndexSpec; } -/// MongoDB's default index name: a_1_b_-1. +/// MongoDB's default index name: a_1_b_-1, and a_hashed for the plugin. fn default_name(gpa: std.mem.Allocator, key_pairs: []const bson.Pair) ![]u8 { var out: std.ArrayListUnmanaged(u8) = .empty; errdefer out.deinit(gpa); @@ -2223,10 +2373,11 @@ fn default_name(gpa: std.mem.Allocator, key_pairs: []const bson.Pair) ![]u8 { if (i > 0) try out.append(gpa, '_'); try out.appendSlice(gpa, p.key); try out.append(gpa, '_'); - if (descending(p.value)) { - try out.append(gpa, '-'); + switch (try key_kind(p.value)) { + .ascending => try out.append(gpa, '1'), + .descending => try out.appendSlice(gpa, "-1"), + .hashed => try out.appendSlice(gpa, "hashed"), } - try out.append(gpa, '1'); } return out.toOwnedSlice(gpa); } @@ -2586,11 +2737,18 @@ fn index_provides_sort(ix: *const Index, run: usize, sort: []const query.SortKey if (sort.len == 0 or ix.multikey) return null; if (run + sort.len > ix.keys.len) return null; - const backward = sort[0].descending != ix.keys[run].descending; + // A hashed component orders by hash, so reading the leaves in order is + // not reading the values in order. Never providing a sort from one is + // exactly why `find({}, {sort: {a: 1}})` over a hashed index still comes + // back correct. + for (0..sort.len) |i| { + if (ix.keys[run + i].kind == .hashed) return null; + } + const backward = sort[0].descending != ix.keys[run].descending(); for (sort, 0..) |sk, i| { const k = ix.keys[run + i]; if (!std.mem.eql(u8, sk.path, k.path)) return null; - if ((sk.descending != k.descending) != backward) return null; + if ((sk.descending != k.descending()) != backward) return null; } return backward; } @@ -2618,7 +2776,11 @@ fn evaluate_index( var lo_incl = false; var hi: ?bson.Value = null; var hi_incl = false; - if (run < n) { + // A hashed component is ordered by its hash, so a range over it selects + // an arbitrary set of values -- correct answers are not a contiguous band + // of leaves. Equality survives the hash and nothing else does, which is + // the whole of what a hashed index can be read for. + if (run < n and ix.keys[run].kind != .hashed) { lo = infos[run].lo; lo_incl = infos[run].lo_incl; hi = infos[run].hi; @@ -2755,7 +2917,7 @@ fn simple_index( sparse: bool, ) !Index { var keys: [max_index_keys]IndexKey = undefined; - for (paths, 0..) |p, i| keys[i] = .{ .path = p, .descending = false }; + for (paths, 0..) |p, i| keys[i] = .{ .path = p }; return Index.init(gpa, pager, "test", keys[0..paths.len], unique, sparse, null); } @@ -3682,6 +3844,179 @@ test "TTL spec round-trips through write_spec and compares in spec_equal" { try testing.expect(plain_doc.get("expireAfterSeconds") == null); } +test "a hashed spec names the plugin and refuses what it cannot honour" { + const gpa = testing.allocator; + const spec = doc_of(&.{ + .{ .key = "key", .value = .{ .doc = &.{.{ .key = "a", .value = .{ .string = "hashed" } }} } }, + }); + var ix = try parse_spec(gpa, test_pager(), &spec); + defer ix.deinit(gpa); + try testing.expectEqualStrings("a_hashed", ix.name); + try testing.expectEqual(KeyKind.hashed, ix.keys[0].kind); + + // What a client is told: the plugin's name, never the hash. That is what + // leaves the encoding a private choice, so it round-trips through the log. + var bytes: std.ArrayListUnmanaged(u8) = .empty; + defer bytes.deinit(gpa); + try ix.write_spec(gpa, &bytes); + var reparsed = try bson.Document.parse(gpa, bytes.items); + defer reparsed.deinit(); + try testing.expectEqualStrings("hashed", reparsed.get("key").?.doc[0].value.string); + var ix2 = try parse_spec(gpa, test_pager(), &reparsed); + defer ix2.deinit(gpa); + try testing.expect(Index.spec_equal(&ix, &ix2)); + + // Same path, different kind: not the same index. mongod keeps both, and + // `spec_equal` returning true here is what would silently drop one. + const asc = doc_of(&.{ + .{ .key = "key", .value = .{ .doc = &.{.{ .key = "a", .value = .{ .int32 = 1 } }} } }, + .{ .key = "name", .value = .{ .string = "a_hashed" } }, + }); + var ix3 = try parse_spec(gpa, test_pager(), &asc); + defer ix3.deinit(gpa); + try testing.expect(!Index.spec_equal(&ix, &ix3)); + + // One hashed component beside a range one is allowed, and named for both. + const compound = doc_of(&.{ + .{ .key = "key", .value = .{ .doc = &.{ + .{ .key = "a", .value = .{ .string = "hashed" } }, + .{ .key = "s", .value = .{ .int32 = -1 } }, + } } }, + }); + var ix4 = try parse_spec(gpa, test_pager(), &compound); + defer ix4.deinit(gpa); + try testing.expectEqualStrings("a_hashed_s_-1", ix4.name); + + // The three refusals, each measured on mongod 8.3.7. + const two = doc_of(&.{.{ .key = "key", .value = .{ .doc = &.{ + .{ .key = "a", .value = .{ .string = "hashed" } }, + .{ .key = "s", .value = .{ .string = "hashed" } }, + } } }}); + try testing.expectError(error.TwoHashedComponents, parse_spec(gpa, test_pager(), &two)); + + const uniq = doc_of(&.{ + .{ .key = "key", .value = .{ .doc = &.{.{ .key = "a", .value = .{ .string = "hashed" } }} } }, + .{ .key = "unique", .value = .{ .bool = true } }, + }); + try testing.expectError(error.UniqueHashed, parse_spec(gpa, test_pager(), &uniq)); + + const bogus = doc_of(&.{.{ .key = "key", .value = .{ .doc = &.{ + .{ .key = "a", .value = .{ .string = "bogus" } }, + } } }}); + try testing.expectError(error.UnknownIndexPlugin, parse_spec(gpa, test_pager(), &bogus)); +} + +test "a hashed component answers equality across numeric types and nothing else" { + const gpa = testing.allocator; + const spec = doc_of(&.{ + .{ .key = "key", .value = .{ .doc = &.{.{ .key = "a", .value = .{ .string = "hashed" } }} } }, + }); + var ix = try parse_spec(gpa, test_pager(), &spec); + defer ix.deinit(gpa); + + const d5 = try bytes_of(gpa, &.{ + .{ .key = "_id", .value = .{ .int32 = 1 } }, + .{ .key = "a", .value = .{ .int32 = 5 } }, + }); + defer gpa.free(d5); + _ = try ix.add_doc(gpa, d5, 10, true); + // A document with no `a` at all: a non-sparse index stores it as null, + // and the hash of null is a value like any other. + const dnone = try bytes_of(gpa, &.{.{ .key = "_id", .value = .{ .int32 = 2 } }}); + defer gpa.free(dnone); + _ = try ix.add_doc(gpa, dnone, 20, true); + + // The load-bearing property: the hash is taken over `encode_key`'s + // output, which normalizes every numeric type through f128. Stored as an + // int32, found as a double. Mutation: hash the serialized value instead + // and this line finds nothing. + try expect_offs(gpa, &ix, &.{.{ .double = 5.0 }}, &.{10}); + try expect_offs(gpa, &ix, &.{.{ .int64 = 5 }}, &.{10}); + try expect_offs(gpa, &ix, &.{.null}, &.{20}); + try expect_offs(gpa, &ix, &.{.{ .int32 = 6 }}, &.{}); + + // Equality plans; a range and a sort do not. Both would read a band of + // leaves ordered by hash, which is an arbitrary set of values -- the one + // failure worse than not using an index. Mutation: drop either guard and + // the corresponding case here starts producing a plan. + { + const f = [_]bson.Pair{.{ .key = "a", .value = .{ .int32 = 5 } }}; + var p = (try plan(gpa, null, &.{&ix}, &f, &.{})).?; + defer p.deinit(gpa); + try testing.expectEqual(@as(usize, 1), p.key_len()); + try testing.expect(p.lo == null and p.hi == null); + } + { + const f = [_]bson.Pair{.{ .key = "a", .value = .{ .doc = &.{ + .{ .key = "$gte", .value = .{ .int32 = 5 } }, + } } }}; + try testing.expect((try plan(gpa, null, &.{&ix}, &f, &.{})) == null); + } + { + const sort = [_]query.SortKey{.{ .path = "a", .descending = false }}; + try testing.expect((try plan(gpa, null, &.{&ix}, &.{}, &sort)) == null); + } +} + +test "an array anywhere along a hashed path has no single hash" { + const gpa = testing.allocator; + const spec = doc_of(&.{ + .{ .key = "key", .value = .{ .doc = &.{.{ .key = "a.b", .value = .{ .string = "hashed" } }} } }, + }); + var ix = try parse_spec(gpa, test_pager(), &spec); + defer ix.deinit(gpa); + + // An array *through* the path, with a single element. Measured: mongod + // refuses this exactly as it refuses a longer one, and one element is the + // case a value count cannot tell apart from `{a: {b: 1}}` -- which is why + // `array_on_path` walks rather than counting. + const one = try bytes_of(gpa, &.{ + .{ .key = "_id", .value = .{ .int32 = 1 } }, + .{ .key = "a", .value = .{ .array = &.{.{ .doc = &.{ + .{ .key = "b", .value = .{ .int32 = 1 } }, + } }} } }, + }); + defer gpa.free(one); + try testing.expectError(error.HashedArray, ix.add_doc(gpa, one, 1, true)); + + // The same document without the array is indexed. + const plain = try bytes_of(gpa, &.{ + .{ .key = "_id", .value = .{ .int32 = 2 } }, + .{ .key = "a", .value = .{ .doc = &.{.{ .key = "b", .value = .{ .int32 = 1 } }} } }, + }); + defer gpa.free(plain); + _ = try ix.add_doc(gpa, plain, 2, true); + try testing.expectEqual(@as(usize, 1), ix.count()); + + // An array at the end of the path, including an empty one. + var end = try parse_spec(gpa, test_pager(), &doc_of(&.{ + .{ .key = "key", .value = .{ .doc = &.{.{ .key = "a", .value = .{ .string = "hashed" } }} } }, + })); + defer end.deinit(gpa); + const arr = try bytes_of(gpa, &.{ + .{ .key = "_id", .value = .{ .int32 = 3 } }, + .{ .key = "a", .value = .{ .array = &.{ .{ .int32 = 1 }, .{ .int32 = 2 } } } }, + }); + defer gpa.free(arr); + try testing.expectError(error.HashedArray, end.add_doc(gpa, arr, 3, true)); + const empty = try bytes_of(gpa, &.{ + .{ .key = "_id", .value = .{ .int32 = 4 } }, + .{ .key = "a", .value = .{ .array = &.{} } }, + }); + defer gpa.free(empty); + try testing.expectError(error.HashedArray, end.add_doc(gpa, empty, 4, true)); + + // An array *beside* the path is nothing to do with it. + const beside = try bytes_of(gpa, &.{ + .{ .key = "_id", .value = .{ .int32 = 5 } }, + .{ .key = "a", .value = .{ .int32 = 1 } }, + .{ .key = "z", .value = .{ .array = &.{.{ .int32 = 1 }} } }, + }); + defer gpa.free(beside); + _ = try end.add_doc(gpa, beside, 5, true); + try testing.expectEqual(@as(usize, 1), end.count()); +} + test "TTL spec rejects compound keys and bad expireAfterSeconds" { const gpa = testing.allocator; const compound = doc_of(&.{ diff --git a/src/spill.zig b/src/spill.zig index d34bd51..bc99e90 100644 --- a/src/spill.zig +++ b/src/spill.zig @@ -32,7 +32,7 @@ pub fn main() !void { var prng = std.Random.DefaultPrng.init(0x1234_5678); const rand = prng.random(); - var keys = [_]index.IndexKey{.{ .path = "tag", .descending = false }}; + var keys = [_]index.IndexKey{.{ .path = "tag" }}; var ix = try index.Index.init(gpa, try harness_pager(gpa, "spill"), "tag", &keys, false, false, null); // Keys straddling the spill threshold: inline, exactly at the limit, diff --git a/src/spill2.zig b/src/spill2.zig index cb2ad25..6b99758 100644 --- a/src/spill2.zig +++ b/src/spill2.zig @@ -30,7 +30,7 @@ fn harness_pager(gpa: std.mem.Allocator, name: []const u8) !*pgr.Pager { pub fn main() !void { const gpa = std.heap.page_allocator; - var keys = [_]index.IndexKey{.{ .path = "tag", .descending = false }}; + var keys = [_]index.IndexKey{.{ .path = "tag" }}; var ix = try index.Index.init(gpa, try harness_pager(gpa, "spill2"), "tag", &keys, false, false, null); // 5000 docs, each with a 2 KiB key: spills on every record, forcing diff --git a/src/stress.zig b/src/stress.zig index a62f06e..dc0a438 100644 --- a/src/stress.zig +++ b/src/stress.zig @@ -74,7 +74,7 @@ pub fn main() !void { var pairs: [2]bson.Pair = undefined; // 1. Bulk build an index over N docs. - var keys = [_]index.IndexKey{ .{ .path = "a", .descending = false }, .{ .path = "b", .descending = false } }; + var keys = [_]index.IndexKey{ .{ .path = "a" }, .{ .path = "b" } }; var ix = try index.Index.init(gpa, try harness_pager(gpa, "stress"), "ab", &keys, false, false, null); for (0..N) |i| { const id: u64 = @intCast(i + 1);