dev/MultiforaDB
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

update: replacement-style writes

Browse Source 
`replaceOne`, `findOneAndReplace` and `bulkWrite`'s `replaceOne` all failed
with "bad update". `update.apply` rejected any update document whose first key
was not `$`-prefixed, so a replacement document -- which by definition has no
operators -- could not get through at all.

MongoDB decides on the first field and nothing else: `$`-prefixed means
operators, anything else means the document *is* the new content. An empty
document is a replacement too, and a legal one. `is_replacement` says which,
`apply_replacement` does the work, and because all three call sites already go
through `apply`, that one branch covers the update command, findAndModify and
the upsert builder.

What a replacement means, precisely:

  - every field is replaced except `_id`, which is immutable and keeps its
    position at the front, where it is stored and where the `_id_` index
    descends on it;
  - a replacement may restate the same `_id` but not a different one -- that
    is `ImmutableId`, because otherwise a rewrite would silently change a
    document's identity while the index entry kept the old key;
  - when the target has no `_id` yet, the replacement supplies it. That is the
    upsert path: `build_upsert_doc` seeds a document from the filter's
    equalities, so `replaceOne({_id: 99}, {u: 1}, {upsert: true})` inserts
    `{_id: 99, u: 1}` and not a generated ObjectId;
  - a mixed document is refused from either side, rather than guessed at.

Two options on update specs are refused rather than ignored:

  - `multi` with a replacement (FailedToParse). A replacement describes one
    document; applying it to many would leave every match identical apart from
    its `_id`.
  - `sort`, a MongoDB 8.0 addition this server does not implement. Ignoring it
    is the worst of the three answers -- `sort` chooses *which* match to write,
    so the client would silently get a different document than it asked for.

  spec scorecard   131 pass / 161 fail  ->  161 pass / 131 fail
  e2e.js           35 checks -> 45

Sixteen spec files improved and none regressed. The two `-sort` files briefly
did: they had been passing on their "server-side error" case, which our
"bad update" failure satisfied by accident, and passing for the wrong reason is
how a gap survives a scorecard.

Five mutations, each verified red: seeding the replacement from the old pairs,
dropping the `_id` comparison, dropping the `_id` a replacement supplies, and
removing the mixed-document guard from either loop.

Note `nModified` is still wrong for a write that changes nothing -- MongoDB
counts a document as modified only if applying the update altered it. That is
the remaining bulkWrite failure and is fixed next, separately.
This commit is contained in:
Alexey Shakhmatov
2026-08-03 23:52:22 +03:00
parent 504179acd1
commit 53f88e6d3b
4 changed files with 304 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
src/commands.zig
View File

@@ -872,6 +872,21 @@ fn cmd_update(ctx: *Context, msg: *wire.Message, reply: *wire.Reply) !void {
const u_doc = doc_arg(spec.get("u")) orelse return bad_value(reply, "update spec requires u");
const multi = bool_arg(spec.get("multi")) orelse false;
const upsert = bool_arg(spec.get("upsert")) orelse false;
// `sort` on an update spec picks *which* match to write when the filter
// matches several -- a MongoDB 8.0 addition. This server advertises 4.4,
// and ignoring the field would be the worst of the three possible
// answers: the client asked for a specific document and would silently
// get a different one. So refuse it, which is also what a real 4.4 does
// with an unknown update-spec field.
if (spec.get("sort") != null) {
return failed_to_parse(reply, "Unknown option to update: sort");
}
// A replacement describes one document, so there is no coherent meaning
// for applying it to many: every match would end up identical apart from
// its `_id`. MongoDB rejects the combination rather than doing that.
if (multi and update.is_replacement(u_doc)) {
return failed_to_parse(reply, "multi update is not supported for replacement-style update");
}
var matched: std.ArrayListUnmanaged(u64) = .empty;
defer matched.deinit(ctx.gpa);
@@ -1605,6 +1620,10 @@ fn bad_value(reply: *wire.Reply, msg: []const u8) !void {
return reply.put_error(@intFromEnum(ErrorCode.bad_value), "BadValue", msg);
}
fn failed_to_parse(reply: *wire.Reply, msg: []const u8) !void {
return reply.put_error(@intFromEnum(ErrorCode.failed_to_parse), "FailedToParse", msg);
}
/// The E11000 text, shared by the top-level error reply and the per-document
/// `writeErrors` entries of a batch insert. Uses the collection's
/// dup_index (set by a rejected unique-index write) when the conflict came

195
src/update.zig
View File

@@ -10,17 +10,88 @@ pub const UpdateError = error{ ImmutableId, InvalidUpdate, OutOfMemory };
const max_path_segments = 16;
/// Apply an update document (whose fields are operator documents) to `doc`.
/// Whether an update document is a *replacement* rather than a set of
/// operators. MongoDB decides on the first field and nothing else: a
/// `$`-prefixed one means operators. An empty document is a replacement, and a
/// legal one -- it strips every field but `_id`.
///
/// This distinction is the whole of `replaceOne`, `findOneAndReplace` and
/// `bulkWrite`'s `replaceOne`. Without it `apply` rejected every update whose
/// first key was not `$`-prefixed, so all three failed with "bad update".
pub fn is_replacement(pairs: []const bson.Pair) bool {
if (pairs.len == 0) return true;
return !is_operator_key(pairs[0].key);
}
fn is_operator_key(key: []const u8) bool {
return key.len > 0 and key[0] == '$';
}
/// Apply an update document to `doc`: a replacement, or a set of operators.
pub fn apply(doc: *bson.Document, update: *const bson.Document) UpdateError!void {
if (is_replacement(update.pairs)) return apply_replacement(doc, update.pairs);
const arena = doc.arena.allocator();
var pairs = try copy_to_list(bson.Pair, arena, doc.pairs);
for (update.pairs) |op| {
if (op.key.len == 0 or op.key[0] != '$') return error.InvalidUpdate;
// The first field decided this is an operator update, so a field that
// is not one is a mixed document -- which MongoDB rejects rather than
// guessing at.
if (!is_operator_key(op.key)) return error.InvalidUpdate;
try apply_operator(arena, &pairs, op.key, op.value);
}
doc.pairs = try pairs.toOwnedSlice(arena);
}
/// Replace every field of `doc` with `replacement`'s, except `_id`.
///
/// `_id` is immutable, so it survives and keeps its position at the front (which
/// is also where MongoDB stores it, and what the `_id_` index descends on). A
/// replacement carrying an `_id` is allowed only when it is the *same* `_id`;
/// anything else is an attempt to change a document's identity by rewriting it.
///
/// `doc` may legitimately have no `_id` yet: that is the upsert path, where the
/// caller has seeded the document from the filter's equalities and `insert`
/// generates an ObjectId afterwards. Then the replacement's own `_id`, if it has
/// one, is what the new document gets.
fn apply_replacement(doc: *bson.Document, replacement: []const bson.Pair) UpdateError!void {
const arena = doc.arena.allocator();
const old_id = doc.get("_id");
var pairs: std.ArrayListUnmanaged(bson.Pair) = .empty;
errdefer pairs.deinit(arena);
try pairs.ensureTotalCapacity(arena, replacement.len + 1);
if (old_id) |id| {
try pairs.append(arena, .{
.key = try arena.dupe(u8, "_id"),
.value = try bson.copy_value(arena, id),
});
}
for (replacement) |p| {
// A replacement is data, not instructions. `{$set: {...}}` reaching here
// would mean the first field was not an operator and a later one was,
// i.e. a mixed document.
if (is_operator_key(p.key)) return error.InvalidUpdate;
if (std.mem.eql(u8, p.key, "_id")) {
if (old_id) |id| {
if (bson.compare(id, p.value) != .eq) return error.ImmutableId;
continue; // already at the front
}
// No stored `_id`: this replacement supplies it.
try pairs.insert(arena, 0, .{
.key = try arena.dupe(u8, "_id"),
.value = try bson.copy_value(arena, p.value),
});
continue;
}
try pairs.append(arena, .{
.key = try arena.dupe(u8, p.key),
.value = try bson.copy_value(arena, p.value),
});
}
doc.pairs = try pairs.toOwnedSlice(arena);
}
fn apply_operator(
arena: std.mem.Allocator,
pairs: *std.ArrayListUnmanaged(bson.Pair),
@@ -417,10 +488,128 @@ test "$set nested creation and _id protection" {
})));
}
test "non-operator update rejected" {
test "a replacement keeps _id and drops every other field" {
// Mutation check: seed `apply_replacement`'s list from `doc.pairs` (as the
// operator path does) instead of starting empty. Red -- `gone` survives, and
// a replacement that does not remove fields is not a replacement.
var doc = bson.Document{ .arena = std.heap.ArenaAllocator.init(testing.allocator), .pairs = &.{} };
defer doc.arena.deinit();
doc.pairs = try doc.arena.allocator().dupe(bson.Pair, &.{
.{ .key = "_id", .value = .{ .int32 = 7 } },
.{ .key = "gone", .value = .{ .int32 = 1 } },
.{ .key = "also_gone", .value = .{ .string = "x" } },
});
try apply(&doc, &doc_of(&.{
.{ .key = "fresh", .value = .{ .int32 = 42 } },
}));
try testing.expectEqual(@as(usize, 2), doc.pairs.len);
// _id survives, and stays at the front where it is stored.
try testing.expectEqualStrings("_id", doc.pairs[0].key);
try testing.expectEqual(@as(i64, 7), doc.pairs[0].value.int32);
try testing.expectEqual(@as(i64, 42), bson.get_pair(doc.pairs, "fresh").?.int32);
try testing.expect(bson.get_pair(doc.pairs, "gone") == null);
try testing.expect(bson.get_pair(doc.pairs, "also_gone") == null);
}
test "an empty replacement leaves a document holding only its _id" {
var doc = bson.Document{ .arena = std.heap.ArenaAllocator.init(testing.allocator), .pairs = &.{} };
defer doc.arena.deinit();
doc.pairs = try doc.arena.allocator().dupe(bson.Pair, &.{
.{ .key = "_id", .value = .{ .int32 = 7 } },
.{ .key = "a", .value = .{ .int32 = 1 } },
});
// Legal, and the reason `is_replacement` treats an empty document as one
// rather than as a no-op set of operators.
try apply(&doc, &doc_of(&.{}));
try testing.expectEqual(@as(usize, 1), doc.pairs.len);
try testing.expectEqualStrings("_id", doc.pairs[0].key);
}
test "a replacement may repeat the _id it is replacing, but not change it" {
// Mutation check: drop the `bson.compare` in `apply_replacement`. Red on the
// second half -- a replacement would silently rewrite a document's identity,
// and since the `_id_` index is keyed on it, the stored entry and the stored
// document would disagree.
var doc = bson.Document{ .arena = std.heap.ArenaAllocator.init(testing.allocator), .pairs = &.{} };
defer doc.arena.deinit();
const original = [_]bson.Pair{
.{ .key = "_id", .value = .{ .int32 = 7 } },
.{ .key = "a", .value = .{ .int32 = 1 } },
};
doc.pairs = try doc.arena.allocator().dupe(bson.Pair, &original);
// The same _id, restated: accepted.
try apply(&doc, &doc_of(&.{
.{ .key = "_id", .value = .{ .int32 = 7 } },
.{ .key = "b", .value = .{ .int32 = 2 } },
}));
try testing.expectEqual(@as(i64, 7), doc.pairs[0].value.int32);
try testing.expectEqual(@as(i64, 2), bson.get_pair(doc.pairs, "b").?.int32);
// And only once, not twice.
try testing.expectEqual(@as(usize, 2), doc.pairs.len);
// A different _id: refused.
try testing.expectError(error.ImmutableId, apply(&doc, &doc_of(&.{
.{ .key = "_id", .value = .{ .int32 = 8 } },
})));
// Equal across numeric types is the same _id, matching the canonical key
// encoding the _id_ index descends on.
try apply(&doc, &doc_of(&.{
.{ .key = "_id", .value = .{ .double = 7.0 } },
.{ .key = "c", .value = .{ .int32 = 3 } },
}));
try testing.expectEqual(@as(i64, 3), bson.get_pair(doc.pairs, "c").?.int32);
}
test "a replacement supplies the _id when the document has none" {
// The upsert path: the caller seeds a document from the filter's equalities
// and the replacement carries the _id.
//
// Mutation check: make the `old_id == null` arm `continue` without adding the
// pair. Red -- the upserted document loses the _id the client asked for and
// gets a generated ObjectId instead, so a retried upsert inserts twice.
var doc = bson.Document{ .arena = std.heap.ArenaAllocator.init(testing.allocator), .pairs = &.{} };
defer doc.arena.deinit();
try apply(&doc, &doc_of(&.{
.{ .key = "a", .value = .{ .int32 = 1 } },
.{ .key = "_id", .value = .{ .int32 = 99 } },
}));
try testing.expectEqual(@as(usize, 2), doc.pairs.len);
try testing.expectEqualStrings("_id", doc.pairs[0].key);
try testing.expectEqual(@as(i64, 99), doc.pairs[0].value.int32);
}
test "a mixed update document is refused from either side" {
// Mutation check: drop the `is_operator_key` guard in `apply`'s operator loop
// (first case) or in `apply_replacement`'s loop (second). Each leaves one
// half green and the other red.
var doc = bson.Document{ .arena = std.heap.ArenaAllocator.init(testing.allocator), .pairs = &.{} };
defer doc.arena.deinit();
// Starts with an operator, so operators are expected throughout.
try testing.expectError(error.InvalidUpdate, apply(&doc, &doc_of(&.{
.{ .key = "$set", .value = .{ .doc = &.{.{ .key = "a", .value = .{ .int32 = 1 } }} } },
.{ .key = "plain", .value = .{ .int32 = 1 } },
})));
// Starts with data, so it is a replacement and an operator has no meaning.
try testing.expectError(error.InvalidUpdate, apply(&doc, &doc_of(&.{
.{ .key = "plain", .value = .{ .int32 = 1 } },
.{ .key = "$set", .value = .{ .doc = &.{.{ .key = "a", .value = .{ .int32 = 1 } }} } },
})));
}
test "is_replacement decides on the first field only" {
try testing.expect(is_replacement(&.{}));
try testing.expect(is_replacement(&.{.{ .key = "a", .value = .{ .int32 = 1 } }}));
try testing.expect(!is_replacement(&.{.{ .key = "$set", .value = .{ .doc = &.{} } }}));
// An empty key cannot be an operator, so it reads as data -- and the
// replacement path then stores it, which is what MongoDB does with it.
try testing.expect(is_replacement(&.{.{ .key = "", .value = .{ .int32 = 1 } }}));
}