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

commands: refuse partialFilterExpression instead of ignoring it

Browse Source 
`createIndex({a: 1}, {partialFilterExpression: ...})` answered success, built
the index over every document, and left the option out of `listIndexes`.

An over-inclusive index still answers reads correctly -- it holds a superset,
never a subset -- so this was not the array-destroying class of bug. `unique`
is where it stopped being harmless. Measured on mongod 8.3.7:

  createIndex({a: 1}, {unique: true, partialFilterExpression: {t: true}})
  insertMany([{a: 1, t: false}, {a: 1, t: false}])
    mongod: accepted -- neither document is in the index, so neither collides
    ours:   E11000 duplicate key error, dup key: {a: 1}

A legal insert refused. Unique-within-a-subset -- unique email among active
accounts, unique external id among synced rows -- is the whole point of the
option, so every use of it hit this.

Refused at creation, which is the same judgement `cmd_update` already makes
about an update spec's `sort`: "ignoring the field would be the worst of the
three possible answers." The two alternatives here were to keep enforcing
`unique` over the wrong set, or to echo the option back from `listIndexes`
while not honouring it, which is a larger lie than saying no.

CannotCreateIndex (67), joining the five existing rows of the same test with
the mutation that reddens them written beside it. The implementation is the
rest of M3's last row and comes next, against a corpus that does not exist
yet -- nothing in this repository covered the row above, in any suite.

249/249 unit tests, pinned crud corpus unmoved at 228/63/196.
This commit was merged in pull request #10.
This commit is contained in:
A.Shakhmatov
2026-08-10 22:12:08 +03:00
parent ce4ff1fd63
commit 21002528be
2 changed files with 76 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
PLAN.md
View File

@@ -1123,6 +1123,32 @@ has to be its own commit with its own re-recorded scorecard.
agrees. Worth one commit, and it needs its own measurements first: the
padding case (`y.3.b` past the end) is a *legal* creation on mongod, so
the fix is not "refuse a non-document element".
- **`partialFilterExpression` was accepted and ignored**, and that made
`unique` mean the wrong thing. An index built over every document instead of
the filtered subset still answers reads correctly -- it holds a superset,
never a subset -- but a *unique* one then rejects inserts mongod accepts:
```js
createIndex({a: 1}, {unique: true, partialFilterExpression: {t: true}})
insertMany([{a: 1, t: false}, {a: 1, t: false}])
// mongod: accepted, neither document is in the index
// ours: E11000 duplicate key error
```
Unique-within-a-subset is the whole point of the option, so every use of it
was a legal insert refused. Refused at creation now, on the same judgement
`cmd_update` already makes about an update spec's `sort`. Nothing in the
repository covered this row: the pinned suite is crud and aggregate, and
neither `e2e5.js` nor `e2e6.js` writes a partial or hashed spec. Hashed, by
contrast, was honestly missing -- refused, with the wrong code (2 where
mongod says 67) but the right answer.
`docs/M3_INDEX_TYPES_DESIGN_REVIEW.md` measures both features' rules and
argues the planner rule that decides the design: a partial index may only
answer a query whose predicates *imply* its filter, so until that test
exists the safe rule is to maintain the index and never read from it. Too
few documents is the one failure worse than no index at all.
- **M3's second corpus is `tests/spec/operators/`.** The eight operators PLAN
§3 names all answered `bad update` with code 2, one message for every
question — and `$push`'s `$slice`, `$position` and `$sort` were parsed,

50
src/commands.zig
View File

@@ -934,6 +934,35 @@ fn cmd_create_indexes(ctx: *Context, msg: *wire.Message, reply: *wire.Reply) !vo
if (name == .string and std.mem.eql(u8, name.string, "_id_")) {
return bad_value(reply, "cannot create index with name '_id_'");
}
// Accepted and ignored until this commit, which is the worst of the
// three possible answers -- the same judgement `cmd_update` already
// makes about an update spec's `sort`.
//
// An index built over every document instead of the filtered subset
// still answers reads correctly: it holds a superset, never a subset.
// `unique` is where that stops being true. Measured on mongod 8.3.7:
//
// createIndex({a: 1}, {unique: true, partialFilterExpression: {t: true}})
// insertMany([{a: 1, t: false}, {a: 1, t: false}])
//
// mongod accepts both -- neither document is in the index, so neither
// collides -- and this server answered E11000. Unique-within-a-subset
// is the whole point of the option, so every use of it was a legal
// insert refused. The other two answers available were to keep doing
// that, or to echo the option back from `listIndexes` while not
// honouring it, which is a larger lie than saying no.
//
// See docs/M3_INDEX_TYPES_DESIGN_REVIEW.md. The implementation is the
// rest of M3's last row; this is what stands in until then.
if (bson.get_pair(spec, "partialFilterExpression") != null) {
return reply.put_error(
@intFromEnum(ErrorCode.cannot_create_index),
"CannotCreateIndex",
"partialFilterExpression is not implemented by this server: it would be " ++
"accepted and ignored, and a unique index would then be enforced over " ++
"documents the filter excludes",
);
}
const spec_doc = bson.Document{ .arena = undefined, .pairs = spec };
_ = ctx.engine.create_index(db_name, coll_name, &spec_doc) catch |err| switch (err) {
@@ -5655,6 +5684,23 @@ test "TTL index round-trips through createIndexes/listIndexes; bad specs give 67
.{ .key = "key", .value = .{ .doc = &.{.{ .key = "_id", .value = .{ .int32 = 1 } }} } },
.{ .key = "expireAfterSeconds", .value = .{ .int32 = 60 } },
} } },
// Same rule, different option: a partial filter accepted and ignored
// would enforce `unique` over documents the filter excludes.
.{ .code = 67, .spec = .{ .doc = &.{
.{ .key = "key", .value = .{ .doc = &.{.{ .key = "a", .value = .{ .int32 = 1 } }} } },
.{ .key = "partialFilterExpression", .value = .{ .doc = &.{
.{ .key = "t", .value = .{ .bool = true } },
} } },
} } },
// And with `unique`, which is the combination that made it a wrong
// answer rather than only a missing one.
.{ .code = 67, .spec = .{ .doc = &.{
.{ .key = "key", .value = .{ .doc = &.{.{ .key = "b", .value = .{ .int32 = 1 } }} } },
.{ .key = "unique", .value = .{ .bool = true } },
.{ .key = "partialFilterExpression", .value = .{ .doc = &.{
.{ .key = "t", .value = .{ .bool = true } },
} } },
} } },
};
for (bad) |case| {
var ctx = tdb.ctx(io);
@@ -5668,6 +5714,10 @@ test "TTL index round-trips through createIndexes/listIndexes; bad specs give 67
try dispatch(&ctx, &msg, &reply);
try testing.expectEqual(case.code, bson.get_pair(reply.pairs.items, "code").?.int32);
}
// Mutation check for the two `partialFilterExpression` rows: delete the
// guard in `cmd_create_indexes` and both go green on the code -- and the
// second one's index then refuses `{a: 1, t: false}` twice, which mongod
// accepts because neither document is in the index at all.
// Nothing partial was registered by the rejected specs.
try testing.expectEqual(@as(usize, 1), tdb.engine.get_collection("test", "sessions").?.indexes.items.len);
}