src/commands.zig

@@ -4253,6 +4253,25 @@ fn update_refusal(reply: *wire.Reply, err: anyerror, diag: update.Diagnostic) !v
                 "name, found '{s}' and '{s}'",
             .{ diag.segment, diag.other },
         )),
+        error.NotNumericField => return reply.put_error(
+            @intFromEnum(ErrorCode.type_mismatch),
+            "TypeMismatch",
+            try std.fmt.allocPrint(
+                arena,
+                "Cannot apply {s} to a value of non-numeric type. The field '{s}' is of " ++
+                    "non-numeric type {s}",
+                .{ diag.segment, diag.path, diag.other },
+            ),
+        ),
+        error.NotNumericOperand => return reply.put_error(
+            @intFromEnum(ErrorCode.type_mismatch),
+            "TypeMismatch",
+            try std.fmt.allocPrint(
+                arena,
+                "Cannot {s} with non-numeric argument at field '{s}'",
+                .{ if (std.mem.eql(u8, diag.segment, "$inc")) "increment" else "multiply", diag.path },
+            ),
+        ),
         error.PathNotViable => return reply.put_error(
             @intFromEnum(ErrorCode.path_not_viable),
             "PathNotViable",

src/update.zig

@@ -16,6 +16,12 @@ const query = @import("query.zig");
 pub const UpdateError = error{
     ImmutableId,
     InvalidUpdate,
+    /// `$inc` or `$mul` against a stored field that is not a number, and by an
+    /// operand that is not one. Both are mongod's `TypeMismatch` (14) rather
+    /// than the `BadValue` most bad updates answer, and each has its own
+    /// sentence, so they are two errors.
+    NotNumericField,
+    NotNumericOperand,
     /// A path segment that is not a field to create: a non-numeric name
     /// applied to an array, or any name applied to a scalar element a
     /// positional segment selected. mongod's `PathNotViable`.
@@ -615,7 +621,10 @@ fn apply_operator(
     const ops = doc_pairs(value) orelse return error.InvalidUpdate;
     if (std.mem.eql(u8, op, "$set")) return op_set(arena, pairs, ops, opts);
     if (std.mem.eql(u8, op, "$unset")) return op_unset(arena, pairs, ops, opts);
-    if (std.mem.eql(u8, op, "$inc")) return op_inc(arena, pairs, ops, opts);
+    if (std.mem.eql(u8, op, "$inc")) return op_arith(arena, pairs, ops, opts, .add);
+    if (std.mem.eql(u8, op, "$mul")) return op_arith(arena, pairs, ops, opts, .mul);
+    if (std.mem.eql(u8, op, "$min")) return op_extremum(arena, pairs, ops, opts, .lt);
+    if (std.mem.eql(u8, op, "$max")) return op_extremum(arena, pairs, ops, opts, .gt);
     if (std.mem.eql(u8, op, "$push")) return op_push(arena, pairs, ops, opts);
     if (std.mem.eql(u8, op, "$pull")) return op_pull(arena, pairs, ops, opts);
     if (std.mem.eql(u8, op, "$rename")) return op_rename(arena, pairs, ops, opts);
@@ -649,18 +658,58 @@ fn op_unset(
     }
 }
 
-fn op_inc(
+/// `$inc` and `$mul`, which differ only in the operation and in what an absent
+/// field starts from: `$inc` from 0 because adding leaves the operand, `$mul`
+/// from 0 because multiplying does too -- so `{$mul: {gone: 5}}` writes 0, not
+/// 5. Measured; the natural guess is the other one.
+fn op_arith(
     arena: std.mem.Allocator,
     pairs: *std.ArrayListUnmanaged(bson.Pair),
     ops: []const bson.Pair,
     opts: Options,
+    comptime kind: enum { add, mul },
+) UpdateError!void {
+    for (ops) |p| {
+        // The operand is checked before the paths are resolved, so an operand
+        // that is not a number is one answer for the whole update rather than
+        // one per element a positional segment reached.
+        const op_name = if (kind == .add) "$inc" else "$mul";
+        if (!p.value.is_number()) {
+            note(opts.diag, p.key, op_name);
+            return error.NotNumericOperand;
+        }
+        for (try resolve(arena, pairs.items, p.key, opts)) |segs| {
+            const current = get_value(pairs.items, segs) orelse bson.Value{ .int32 = 0 };
+            if (!current.is_number()) {
+                if (opts.diag) |d| d.* = .{ .path = p.key, .segment = op_name, .other = current.type_name() };
+                return error.NotNumericField;
+            }
+            const result = switch (kind) {
+                .add => try numeric_add(current, p.value),
+                .mul => try numeric_mul(current, p.value),
+            };
+            try set_path(arena, pairs, segs, result, p.key, opts.diag);
+        }
+    }
+}
+
+/// `$min` and `$max`, which are not numeric operators at all: they compare in
+/// BSON canonical order, so `{$min: {s: 5}}` on `s: "b"` writes 5 because a
+/// number ranks below a string. An absent field is always written -- there is
+/// nothing to be smaller or larger than.
+fn op_extremum(
+    arena: std.mem.Allocator,
+    pairs: *std.ArrayListUnmanaged(bson.Pair),
+    ops: []const bson.Pair,
+    opts: Options,
+    comptime want: std.math.Order,
 ) UpdateError!void {
     for (ops) |p| {
         for (try resolve(arena, pairs.items, p.key, opts)) |segs| {
-            const current = get_value(pairs.items, segs) orelse bson.Value{ .int32 = 0 };
+            if (get_value(pairs.items, segs)) |current| {
-            if (!current.is_number() or !p.value.is_number()) return error.InvalidUpdate;
+                if (bson.compare(p.value, current) != want) continue;
-            const sum = try numeric_add(current, p.value);
+            }
-            try set_path(arena, pairs, segs, sum, p.key, opts.diag);
+            try set_path(arena, pairs, segs, try bson.copy_value(arena, p.value), p.key, opts.diag);
         }
     }
 }
@@ -970,6 +1019,35 @@ fn numeric_add(a: bson.Value, b: bson.Value) UpdateError!bson.Value {
     return .{ .int64 = sum };
 }
 
+/// The same widening ladder as `numeric_add`: a double anywhere makes the
+/// answer a double, two int32s stay int32 unless the product does not fit,
+/// and an int64 overflowing is a refusal rather than a wrap.
+fn numeric_mul(a: bson.Value, b: bson.Value) UpdateError!bson.Value {
+    if (a == .double or b == .double) {
+        const product: f64 = @floatCast(a.as_f128() * b.as_f128());
+        return .{ .double = product };
+    }
+    if (a == .int64 or b == .int64) {
+        const av: i64 = as_int64(a);
+        const bv: i64 = as_int64(b);
+        const product = std.math.mul(i64, av, bv) catch return error.InvalidUpdate;
+        return .{ .int64 = product };
+    }
+    const product: i64 = @as(i64, a.int32) * b.int32;
+    if (product >= std.math.minInt(i32) and product <= std.math.maxInt(i32)) {
+        return .{ .int32 = @intCast(product) };
+    }
+    return .{ .int64 = product };
+}
+
+fn as_int64(v: bson.Value) i64 {
+    return switch (v) {
+        .int32 => |i| i,
+        .int64 => |i| i,
+        else => unreachable,
+    };
+}
+
 // ---------------------------------------------------------------------------
 // Tests
 // ---------------------------------------------------------------------------
@@ -1671,3 +1749,118 @@ test "a replacement is not a path, so it is not refused" {
     try testing.expect(doc.get("y") == null);
     try testing.expectEqual(@as(i32, 1), doc.get("z").?.int32);
 }
+
+test "$mul multiplies, and starts a missing field from zero" {
+    // The measured surprise: `{$mul: {gone: 5}}` writes 0, not 5. Mutation
+    // check: start `op_arith` from `.{ .int32 = 1 }` for `.mul` and the second
+    // half goes red -- which is the reading anyone would reach for.
+    var doc = try doc_with(testing.allocator, &.{
+        .{ .key = "a", .value = .{ .int32 = 5 } },
+        .{ .key = "d", .value = .{ .double = 2.5 } },
+    });
+    defer doc.arena.deinit();
+    try apply(&doc, &doc_of(&.{
+        .{ .key = "$mul", .value = .{ .doc = &.{
+            .{ .key = "a", .value = .{ .int32 = 2 } },
+            .{ .key = "d", .value = .{ .int32 = 2 } },
+            .{ .key = "gone", .value = .{ .int32 = 5 } },
+        } } },
+    }), .{});
+    try testing.expectEqual(@as(i32, 10), doc.get("a").?.int32);
+    try testing.expectEqual(@as(f64, 5.0), doc.get("d").?.double);
+    try testing.expectEqual(@as(i32, 0), doc.get("gone").?.int32);
+}
+
+test "$mul widens an int32 product that does not fit" {
+    var doc = try doc_with(testing.allocator, &.{
+        .{ .key = "a", .value = .{ .int32 = 2000000000 } },
+    });
+    defer doc.arena.deinit();
+    try apply(&doc, &doc_of(&.{
+        .{ .key = "$mul", .value = .{ .doc = &.{.{ .key = "a", .value = .{ .int32 = 2 } }} } },
+    }), .{});
+    try testing.expectEqual(@as(i64, 4000000000), doc.get("a").?.int64);
+}
+
+test "$inc and $mul refuse a non-number, on either side" {
+    // TypeMismatch, not the BadValue the rest of a bad update answers, and the
+    // field and the operand are different sentences on mongod -- so they are
+    // different errors here.
+    var doc = try doc_with(testing.allocator, &.{
+        .{ .key = "a", .value = .{ .int32 = 5 } },
+        .{ .key = "s", .value = .{ .string = "b" } },
+    });
+    defer doc.arena.deinit();
+    var diag: Diagnostic = .{};
+    for ([_][]const u8{ "$inc", "$mul" }) |op| {
+        try testing.expectError(error.NotNumericField, apply(&doc, &doc_of(&.{
+            .{ .key = op, .value = .{ .doc = &.{.{ .key = "s", .value = .{ .int32 = 2 } }} } },
+        }), .{ .diag = &diag }));
+        try testing.expectEqualStrings("string", diag.other);
+        try testing.expectError(error.NotNumericOperand, apply(&doc, &doc_of(&.{
+            .{ .key = op, .value = .{ .doc = &.{.{ .key = "a", .value = .{ .string = "x" } }} } },
+        }), .{ .diag = &diag }));
+        try testing.expectEqualStrings(op, diag.segment);
+    }
+    // Neither refusal wrote anything.
+    try testing.expectEqual(@as(i32, 5), doc.get("a").?.int32);
+    try testing.expectEqualStrings("b", doc.get("s").?.string);
+}
+
+test "$min and $max compare in BSON order, not numerically" {
+    // The load-bearing case: `s` holds a string and the operand is a number,
+    // and there is still a defined answer because a number ranks below a
+    // string. Mutation check: make `op_extremum` require both to be numbers
+    // and the two `s` rows go red.
+    var doc = try doc_with(testing.allocator, &.{
+        .{ .key = "a", .value = .{ .int32 = 5 } },
+        .{ .key = "s", .value = .{ .string = "b" } },
+        .{ .key = "n", .value = .null },
+    });
+    defer doc.arena.deinit();
+    try apply(&doc, &doc_of(&.{
+        .{ .key = "$min", .value = .{ .doc = &.{
+            .{ .key = "a", .value = .{ .int32 = 7 } }, // higher: not written
+            .{ .key = "s", .value = .{ .int32 = 5 } }, // a number is below a string
+            .{ .key = "gone", .value = .{ .int32 = 7 } }, // absent: always written
+        } } },
+    }), .{});
+    try testing.expectEqual(@as(i32, 5), doc.get("a").?.int32);
+    try testing.expectEqual(@as(i32, 5), doc.get("s").?.int32);
+    try testing.expectEqual(@as(i32, 7), doc.get("gone").?.int32);
+
+    try apply(&doc, &doc_of(&.{
+        .{ .key = "$max", .value = .{ .doc = &.{
+            .{ .key = "a", .value = .{ .int32 = 3 } }, // lower: not written
+            .{ .key = "n", .value = .{ .int32 = 1 } }, // a number is above null
+        } } },
+    }), .{});
+    try testing.expectEqual(@as(i32, 5), doc.get("a").?.int32);
+    try testing.expectEqual(@as(i32, 1), doc.get("n").?.int32);
+}
+
+test "$min treats an int and an equal double as the same value" {
+    // Equal is not less, so nothing is written and the stored type survives.
+    var doc = try doc_with(testing.allocator, &.{.{ .key = "a", .value = .{ .int32 = 5 } }});
+    defer doc.arena.deinit();
+    try apply(&doc, &doc_of(&.{
+        .{ .key = "$min", .value = .{ .doc = &.{.{ .key = "a", .value = .{ .double = 5.0 } }} } },
+    }), .{});
+    try testing.expect(doc.get("a").? == .int32);
+}
+
+test "$mul reaches every element a positional segment names" {
+    var doc = try doc_with(testing.allocator, &.{
+        .{ .key = "t", .value = .{ .array = &.{
+            .{ .doc = &.{.{ .key = "a", .value = .{ .int32 = 2 } }} },
+            .{ .doc = &.{.{ .key = "a", .value = .{ .int32 = 3 } }} },
+        } } },
+    });
+    defer doc.arena.deinit();
+    try apply(&doc, &doc_of(&.{
+        .{ .key = "$mul", .value = .{ .doc = &.{.{ .key = "t.$[].a", .value = .{ .int32 = 10 } }} } },
+    }), .{});
+    const t = doc.get("t").?.array;
+    try testing.expectEqual(@as(i32, 20), t[0].doc[0].value.int32);
+    try testing.expectEqual(@as(i32, 30), t[1].doc[0].value.int32);
+}