update: $addToSet, $pop and $pullAll

`$addToSet`'s identity is `bson.compare` equality, which is already exactly
mongod's: an int32 `2` and a double `2.0` are one value, and `{a: 1, b: 2}`
and `{b: 2, a: 1}` are two, because `compare_docs` walks the pairs
positionally and tie-breaks on the key. Candidates are checked against the
array as it grows, so a `$each` holding the same value twice adds it once.

`$pullAll` is `$pull`'s neighbour and its opposite: `$pull` takes a predicate,
`$pullAll` takes values compared whole. `{$pull: {t: {a: 1}}}` removes
elements *having* `a: 1`; `{$pullAll: {t: [{a: 1}]}}` removes elements that
*are* `{a: 1}`. Sharing the comparison would have been the natural mistake and
is what the test mutates to check.

`$pop` is the small one, and its refusals are the measured part: an empty
array and an absent field are no-ops, an argument that is not 1 or -1 is
FailedToParse (9), and a non-array field is TypeMismatch (14) -- where the
identical mistake under `$addToSet` and `$pullAll` is BadValue (2). Three
codes for one shape, none of them derivable from the others. `$each` that is
not an array is 14 under `$addToSet` and 2 under `$push`, measured on both.

array-ops.json 2/26 -> 26/26. 234/234 unit tests.
This commit is contained in:
A.Shakhmatov
2026-08-10 21:11:33 +03:00
parent c2f682717b
commit 066b32617e
2 changed files with 322 additions and 0 deletions

View File

@@ -4272,6 +4272,42 @@ fn update_refusal(reply: *wire.Reply, err: anyerror, diag: update.Diagnostic) !v
.{ if (std.mem.eql(u8, diag.segment, "$inc")) "increment" else "multiply", diag.path },
),
),
error.NotAnArrayField => return bad_value(reply, try std.fmt.allocPrint(
arena,
"Cannot apply {s} to non-array field. Field named '{s}' has non-array type",
.{ diag.segment, diag.path },
)),
error.NotAnArrayPathElement => return reply.put_error(
@intFromEnum(ErrorCode.type_mismatch),
"TypeMismatch",
try std.fmt.allocPrint(
arena,
"Path '{s}' contains an element of non-array type",
.{diag.path},
),
),
// mongod has two sentences here -- "$pop expects 1 or -1, found: 2"
// and "Expected a number in: t: \"x\"" -- both code 9, and both about
// an argument that is not one of the two values `$pop` takes. One
// sentence covering both says the same thing without rendering the
// operand, which no formatter here does.
error.BadPopArgument => return failed_to_parse(reply, try std.fmt.allocPrint(
arena,
"$pop expects 1 or -1, at field '{s}'",
.{diag.path},
)),
error.PullAllNeedsArray => return bad_value(reply, try std.fmt.allocPrint(
arena,
"$pullAll requires an array argument but was given a {s}",
.{diag.other},
)),
// `$push` and `$addToSet` disagree about the code for the identical
// mistake: 2 and 14. Measured on both, and not derivable from either.
error.BadEach => if (std.mem.eql(u8, diag.segment, "$addToSet")) return reply.put_error(
@intFromEnum(ErrorCode.type_mismatch),
"TypeMismatch",
"The argument to $each in $addToSet must be an array",
) else return bad_value(reply, "The argument to $each in $push must be an array"),
error.PathNotViable => return reply.put_error(
@intFromEnum(ErrorCode.path_not_viable),
"PathNotViable",

View File

@@ -22,6 +22,18 @@ pub const UpdateError = error{
/// sentence, so they are two errors.
NotNumericField,
NotNumericOperand,
/// `$addToSet` or `$pullAll` against a field that is not an array.
/// `BadValue` (2) on mongod, where `$pop` against one is `TypeMismatch`.
NotAnArrayField,
/// `$pop` against a field that is not an array. Same mistake as
/// `NotAnArrayField`, different code -- measured, not derived.
NotAnArrayPathElement,
/// `$pop` by anything that is not 1 or -1. `FailedToParse` (9).
BadPopArgument,
/// `$pullAll` by something that is not an array of values.
PullAllNeedsArray,
/// `$each` that is not an array.
BadEach,
/// 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`.
@@ -627,6 +639,9 @@ fn apply_operator(
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, "$addToSet")) return op_add_to_set(arena, pairs, ops, opts);
if (std.mem.eql(u8, op, "$pop")) return op_pop(arena, pairs, ops, opts);
if (std.mem.eql(u8, op, "$pullAll")) return op_pull_all(arena, pairs, ops, opts);
if (std.mem.eql(u8, op, "$rename")) return op_rename(arena, pairs, ops, opts);
return error.InvalidUpdate;
}
@@ -774,6 +789,139 @@ fn op_pull(
}
}
/// `$addToSet`: append what the array does not already hold.
///
/// "Already hold" is `bson.compare` equality, which is exactly mongod's: an
/// int32 `2` and a double `2.0` are one value, and two documents with the same
/// fields in a different order are two -- because `compare_docs` walks the
/// pairs positionally and tie-breaks on the key.
fn op_add_to_set(
arena: std.mem.Allocator,
pairs: *std.ArrayListUnmanaged(bson.Pair),
ops: []const bson.Pair,
opts: Options,
) UpdateError!void {
for (ops) |p| {
// `$each` is a modifier only here and under `$push`; anywhere else a
// document operand is the value being added.
const candidates: []const bson.Value = if (each_of(p.value)) |each| switch (each) {
.array => |a| a,
else => {
note(opts.diag, p.key, "$addToSet");
return error.BadEach;
},
} else &.{p.value};
for (try resolve(arena, pairs.items, p.key, opts)) |segs| {
var items: std.ArrayListUnmanaged(bson.Value) = .empty;
defer items.deinit(arena);
if (get_value(pairs.items, segs)) |current| switch (current) {
.array => |arr| try items.appendSlice(arena, arr),
.null => {},
else => {
note(opts.diag, p.key, "$addToSet");
return error.NotAnArrayField;
},
};
for (candidates) |c| {
if (holds(items.items, c)) continue;
try items.append(arena, try bson.copy_value(arena, c));
}
try set_path(arena, pairs, segs, .{ .array = try items.toOwnedSlice(arena) }, p.key, opts.diag);
}
}
}
fn holds(items: []const bson.Value, v: bson.Value) bool {
for (items) |item| if (bson.compare(item, v) == .eq) return true;
return false;
}
/// The `$each` of a modifier document, or null when the operand is a value.
///
/// Presence of `$each` is what makes an operand a modifier document at all --
/// measured: `{$push: {t: {$slice: 1}}}` pushes `{$slice: 1}` as a value.
fn each_of(v: bson.Value) ?bson.Value {
const doc = doc_pairs(v) orelse return null;
return bson.get_pair(doc, "$each");
}
/// `$pop`: remove one element from an end. `1` is the last, `-1` the first.
///
/// An empty array and an absent field are both no-ops rather than errors, so
/// the only refusals are the argument and the field's type.
fn op_pop(
arena: std.mem.Allocator,
pairs: *std.ArrayListUnmanaged(bson.Pair),
ops: []const bson.Pair,
opts: Options,
) UpdateError!void {
for (ops) |p| {
if (!p.value.is_number()) {
note(opts.diag, p.key, "");
return error.BadPopArgument;
}
const n = p.value.as_f128();
if (n != 1 and n != -1) {
note(opts.diag, p.key, "");
return error.BadPopArgument;
}
for (try resolve(arena, pairs.items, p.key, opts)) |segs| {
const current = get_value(pairs.items, segs) orelse continue;
const arr = switch (current) {
.array => |a| a,
else => {
note(opts.diag, p.key, "$pop");
return error.NotAnArrayPathElement;
},
};
if (arr.len == 0) continue;
const kept = if (n == 1) arr[0 .. arr.len - 1] else arr[1..];
try set_path(arena, pairs, segs, .{ .array = try arena.dupe(bson.Value, kept) }, p.key, opts.diag);
}
}
}
/// `$pullAll`: remove every element equal to any of the listed values.
///
/// The difference from `$pull` is the whole of it: `$pull` takes a *predicate*
/// and `$pullAll` takes values, compared whole. `{$pull: {t: {a: 1}}}` matches
/// elements having `a: 1`; `{$pullAll: {t: [{a: 1}]}}` matches elements that
/// *are* `{a: 1}`.
fn op_pull_all(
arena: std.mem.Allocator,
pairs: *std.ArrayListUnmanaged(bson.Pair),
ops: []const bson.Pair,
opts: Options,
) UpdateError!void {
for (ops) |p| {
const wanted = switch (p.value) {
.array => |a| a,
else => {
if (opts.diag) |d| d.* = .{ .path = p.key, .other = p.value.type_name() };
return error.PullAllNeedsArray;
},
};
for (try resolve(arena, pairs.items, p.key, opts)) |segs| {
const current = get_value(pairs.items, segs) orelse continue;
const arr = switch (current) {
.array => |a| a,
else => {
note(opts.diag, p.key, "$pullAll");
return error.NotAnArrayField;
},
};
var items: std.ArrayListUnmanaged(bson.Value) = .empty;
defer items.deinit(arena);
for (arr) |elem| {
if (holds(wanted, elem)) continue;
try items.append(arena, elem);
}
try set_path(arena, pairs, segs, .{ .array = try items.toOwnedSlice(arena) }, p.key, opts.diag);
}
}
}
/// `$rename` alone keeps the plain split: `validate` has already refused a
/// positional path on either end of it, which is what mongod does too.
fn op_rename(
@@ -1864,3 +2012,141 @@ test "$mul reaches every element a positional segment names" {
try testing.expectEqual(@as(i32, 20), t[0].doc[0].value.int32);
try testing.expectEqual(@as(i32, 30), t[1].doc[0].value.int32);
}
test "$addToSet appends only what the array does not hold" {
var doc = try doc_with(testing.allocator, &.{
.{ .key = "t", .value = .{ .array = &.{ .{ .int32 = 1 }, .{ .int32 = 2 } } } },
});
defer doc.arena.deinit();
try apply(&doc, &doc_of(&.{
.{ .key = "$addToSet", .value = .{ .doc = &.{
.{ .key = "t", .value = .{ .doc = &.{
.{ .key = "$each", .value = .{ .array = &.{ .{ .int32 = 2 }, .{ .int32 = 3 }, .{ .int32 = 3 } } } },
} } },
.{ .key = "gone", .value = .{ .int32 = 1 } },
} } },
}), .{});
// 2 is already there, and the two 3s in one `$each` are one value: the
// candidates are checked against the array *as it grows*.
const t = doc.get("t").?.array;
try testing.expectEqual(@as(usize, 3), t.len);
try testing.expectEqual(@as(i32, 3), t[2].int32);
// An absent field becomes a one-element array rather than an error.
try testing.expectEqual(@as(usize, 1), doc.get("gone").?.array.len);
}
test "$addToSet identity is BSON equality, field order included" {
// Two rows, opposite answers, one comparator. Mutation check: compare with
// anything that ignores key order and the second half goes red -- mongod
// stores both spellings of the same document.
var doc = try doc_with(testing.allocator, &.{
.{ .key = "n", .value = .{ .array = &.{.{ .int32 = 2 }} } },
.{ .key = "d", .value = .{ .array = &.{.{ .doc = &.{
.{ .key = "a", .value = .{ .int32 = 1 } },
.{ .key = "b", .value = .{ .int32 = 2 } },
} }} } },
});
defer doc.arena.deinit();
try apply(&doc, &doc_of(&.{
.{ .key = "$addToSet", .value = .{ .doc = &.{
// An int32 2 and a double 2.0 are one value.
.{ .key = "n", .value = .{ .double = 2.0 } },
// The same fields in the other order are two.
.{ .key = "d", .value = .{ .doc = &.{
.{ .key = "b", .value = .{ .int32 = 2 } },
.{ .key = "a", .value = .{ .int32 = 1 } },
} } },
} } },
}), .{});
try testing.expectEqual(@as(usize, 1), doc.get("n").?.array.len);
try testing.expectEqual(@as(usize, 2), doc.get("d").?.array.len);
}
test "$pop takes one element off an end, and is quiet when there is none" {
var doc = try doc_with(testing.allocator, &.{
.{ .key = "a", .value = .{ .array = &.{ .{ .int32 = 1 }, .{ .int32 = 2 }, .{ .int32 = 3 } } } },
.{ .key = "b", .value = .{ .array = &.{ .{ .int32 = 1 }, .{ .int32 = 2 } } } },
.{ .key = "e", .value = .{ .array = &.{} } },
});
defer doc.arena.deinit();
try apply(&doc, &doc_of(&.{
.{ .key = "$pop", .value = .{ .doc = &.{
.{ .key = "a", .value = .{ .int32 = 1 } }, // the last
.{ .key = "b", .value = .{ .double = -1.0 } }, // the first, and -1.0 is -1
.{ .key = "e", .value = .{ .int32 = 1 } }, // empty: no-op
.{ .key = "gone", .value = .{ .int32 = 1 } }, // absent: no-op
} } },
}), .{});
try testing.expectEqual(@as(usize, 2), doc.get("a").?.array.len);
try testing.expectEqual(@as(i32, 2), doc.get("a").?.array[1].int32);
try testing.expectEqual(@as(usize, 1), doc.get("b").?.array.len);
try testing.expectEqual(@as(i32, 2), doc.get("b").?.array[0].int32);
try testing.expectEqual(@as(usize, 0), doc.get("e").?.array.len);
try testing.expect(doc.get("gone") == null);
}
test "$pop refuses an argument that is not one of its two values" {
var doc = try doc_with(testing.allocator, &.{
.{ .key = "t", .value = .{ .array = &.{.{ .int32 = 1 }} } },
});
defer doc.arena.deinit();
for ([_]bson.Value{ .{ .int32 = 2 }, .{ .int32 = 0 }, .{ .string = "x" } }) |bad| {
try testing.expectError(error.BadPopArgument, apply(&doc, &doc_of(&.{
.{ .key = "$pop", .value = .{ .doc = &.{.{ .key = "t", .value = bad }} } },
}), .{}));
}
try testing.expectEqual(@as(usize, 1), doc.get("t").?.array.len);
}
test "$pullAll removes values, where $pull removes matches" {
// The whole difference between the two, in one document: `{a: 1}` as a
// `$pull` argument is a predicate and as a `$pullAll` element is a value.
// Mutation check: route `$pullAll` through `pull_matches` and the second
// half goes red -- `{a: 1, b: 2}` would be pulled by a predicate too.
var doc = try doc_with(testing.allocator, &.{
.{ .key = "n", .value = .{ .array = &.{
.{ .int32 = 1 }, .{ .int32 = 2 }, .{ .int32 = 3 }, .{ .int32 = 2 },
} } },
.{ .key = "d", .value = .{ .array = &.{
.{ .doc = &.{.{ .key = "a", .value = .{ .int32 = 1 } }} },
.{ .doc = &.{
.{ .key = "a", .value = .{ .int32 = 1 } },
.{ .key = "b", .value = .{ .int32 = 2 } },
} },
} } },
});
defer doc.arena.deinit();
try apply(&doc, &doc_of(&.{
.{ .key = "$pullAll", .value = .{ .doc = &.{
.{ .key = "n", .value = .{ .array = &.{.{ .int32 = 2 }} } },
.{ .key = "d", .value = .{ .array = &.{
.{ .doc = &.{.{ .key = "a", .value = .{ .int32 = 1 } }} },
} } },
.{ .key = "gone", .value = .{ .array = &.{.{ .int32 = 1 }} } },
} } },
}), .{});
try testing.expectEqual(@as(usize, 2), doc.get("n").?.array.len);
// Only the element that *is* `{a: 1}` went; the one that merely matches
// that predicate stayed.
try testing.expectEqual(@as(usize, 1), doc.get("d").?.array.len);
try testing.expectEqual(@as(usize, 2), doc.get("d").?.array[0].doc.len);
try testing.expect(doc.get("gone") == null);
}
test "the array operators refuse a field that is not an array" {
var doc = try doc_with(testing.allocator, &.{.{ .key = "t", .value = .{ .int32 = 5 } }});
defer doc.arena.deinit();
try testing.expectError(error.NotAnArrayField, apply(&doc, &doc_of(&.{
.{ .key = "$addToSet", .value = .{ .doc = &.{.{ .key = "t", .value = .{ .int32 = 1 } }} } },
}), .{}));
try testing.expectError(error.NotAnArrayPathElement, apply(&doc, &doc_of(&.{
.{ .key = "$pop", .value = .{ .doc = &.{.{ .key = "t", .value = .{ .int32 = 1 } }} } },
}), .{}));
try testing.expectError(error.NotAnArrayField, apply(&doc, &doc_of(&.{
.{ .key = "$pullAll", .value = .{ .doc = &.{.{ .key = "t", .value = .{ .array = &.{.{ .int32 = 1 }} } }} } },
}), .{}));
try testing.expectError(error.PullAllNeedsArray, apply(&doc, &doc_of(&.{
.{ .key = "$pullAll", .value = .{ .doc = &.{.{ .key = "t", .value = .{ .int32 = 1 } }} } },
}), .{}));
try testing.expectEqual(@as(i32, 5), doc.get("t").?.int32);
}