db/index: partial indexes hold only what their filter selects
`partialFilterExpression` is honoured rather than refused. The filter is
consulted in exactly one place -- `build_entries` -- which is what keeps the
insert and the remove path from ever disagreeing about which documents the
index holds. Filtering at each call site instead is how an index ends up with
entries pointing at documents that are gone.
That single choke point is also why `unique` comes out right for free, and
`unique` is the whole reason this was a wrong answer rather than a missing
feature: two documents sharing a value *outside* the filter are now accepted,
where before they were E11000. Unique-within-a-subset is what the option is
for.
The filter is part of the index, so it is persisted with it: a fifth bit in
`write_index_catalog`'s flags byte and the serialized document after the TTL.
A file written before this never sets the bit and reads back exactly as it
did, so `catalog_version` stays 1 -- the same argument the free list used.
**The planner declines to read from a partial index.** It holds a subset, so
answering a query from it is only correct when the query's predicates imply
its filter, and that implication test does not exist yet. Returning too few
documents is the one failure worse than having no index at all. So it is
maintained, it enforces `unique`, and reads scan. PLAN §6.
Which predicates a filter may hold is measured: `$eq`, `$gt`, `$gte`, `$lt`,
`$lte`, `$in`, `$exists`, `$type`, `$and`, `$or`. `$ne` and `$regex` are
refused -- including a regex sent as a BSON *value*, which is how a driver
spells `{$regex: "x"}` and which the corpus caught. `sparse` and
`partialFilterExpression` may not be combined: a sparse index is a partial one
whose filter is `{<path>: {$exists: true}}`, and a document satisfying one and
not the other has no defined answer.
Same name, different filter is IndexKeySpecsConflict (86) where a differing
*option* is IndexOptionsConflict (85) next door -- measured, and the split is
that a filter decides which documents the index is over rather than how it
behaves.
ReleaseSafe earned its keep: an assertion held that a non-sparse index covers
every document after an open, and a partial one is a third shape that does
not. Extended rather than relaxed -- multikey was already the second.
partial.json 3/24 -> **24/24**. 250/250 unit tests in ReleaseFast and
ReleaseSafe, 83/83 fuzz, everything else unmoved.
This commit is contained in:
@@ -81,6 +81,11 @@ pub const ErrorCode = enum(i32) {
|
||||
/// stage leaves the document with a different `_id` than it started with.
|
||||
immutable_field = 66,
|
||||
index_options_conflict = 85,
|
||||
/// `IndexKeySpecsConflict`, measured on mongod 8.3.7: the same index name
|
||||
/// over a different *set of documents*, which is what a differing
|
||||
/// `partialFilterExpression` is -- as against a differing option, which is
|
||||
/// 85 next door.
|
||||
index_key_specs_conflict = 86,
|
||||
cannot_create_index = 67,
|
||||
invalid_index_specification_option = 197,
|
||||
// Cursor codes. Taken from MongoDB's own error_codes.js rather than
|
||||
@@ -934,39 +939,30 @@ 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) {
|
||||
error.InvalidIndexSpec => return bad_value(reply, "invalid index spec"),
|
||||
// A filter mongod would not accept either. It restricts the
|
||||
// operators because every one of them narrows a set in a way
|
||||
// another predicate can be checked against -- which is what a
|
||||
// future implication test needs. `$ne` and `$regex` do not.
|
||||
error.PartialFilterUnsupported => return reply.put_error(
|
||||
@intFromEnum(ErrorCode.cannot_create_index),
|
||||
"CannotCreateIndex",
|
||||
"unsupported expression in partialFilterExpression",
|
||||
),
|
||||
error.PartialFilterNotDocument => return reply.put_error(
|
||||
@intFromEnum(ErrorCode.type_mismatch),
|
||||
"TypeMismatch",
|
||||
"partialFilterExpression must be a document",
|
||||
),
|
||||
error.PartialAndSparse => return reply.put_error(
|
||||
@intFromEnum(ErrorCode.cannot_create_index),
|
||||
"CannotCreateIndex",
|
||||
"cannot mix sparse and partialFilterExpression: a sparse index is a partial " ++
|
||||
"one whose filter is {<path>: {$exists: true}}, and a document satisfying " ++
|
||||
"one and not the other has no defined answer",
|
||||
),
|
||||
error.TtlOnCompoundIndex => return reply.put_error(
|
||||
@intFromEnum(ErrorCode.cannot_create_index),
|
||||
"CannotCreateIndex",
|
||||
@@ -979,6 +975,11 @@ fn cmd_create_indexes(ctx: *Context, msg: *wire.Message, reply: *wire.Reply) !vo
|
||||
"between 0 and 2147483647",
|
||||
),
|
||||
error.IndexOptionsConflict => return reply.put_error(@intFromEnum(ErrorCode.index_options_conflict), "IndexOptionsConflict", "index already exists with a different specification"),
|
||||
error.IndexKeySpecsConflict => return reply.put_error(
|
||||
@intFromEnum(ErrorCode.index_key_specs_conflict),
|
||||
"IndexKeySpecsConflict",
|
||||
"an index with the same name exists over a different set of documents",
|
||||
),
|
||||
error.DuplicateKeyIndex => {
|
||||
const ix_name = if (name == .string) name.string else "index";
|
||||
const msg_text = try e11000_message(reply, db_name, coll_name, ix_name, try render_spec_key(reply, key_pairs));
|
||||
@@ -5684,19 +5685,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.
|
||||
// A partial filter holding an operator that narrows nothing another
|
||||
// predicate could be checked against.
|
||||
.{ .code = 67, .spec = .{ .doc = &.{
|
||||
.{ .key = "key", .value = .{ .doc = &.{.{ .key = "a", .value = .{ .int32 = 1 } }} } },
|
||||
.{ .key = "partialFilterExpression", .value = .{ .doc = &.{
|
||||
.{ .key = "t", .value = .{ .bool = true } },
|
||||
.{ .key = "t", .value = .{ .doc = &.{.{ .key = "$ne", .value = .{ .int32 = 1 } }} } },
|
||||
} } },
|
||||
} } },
|
||||
// And with `unique`, which is the combination that made it a wrong
|
||||
// answer rather than only a missing one.
|
||||
// A filter that is not a document at all.
|
||||
.{ .code = 14, .spec = .{ .doc = &.{
|
||||
.{ .key = "key", .value = .{ .doc = &.{.{ .key = "a", .value = .{ .int32 = 1 } }} } },
|
||||
.{ .key = "partialFilterExpression", .value = .{ .int32 = 1 } },
|
||||
} } },
|
||||
// Sparse and a partial filter overlap and may not be combined.
|
||||
.{ .code = 67, .spec = .{ .doc = &.{
|
||||
.{ .key = "key", .value = .{ .doc = &.{.{ .key = "b", .value = .{ .int32 = 1 } }} } },
|
||||
.{ .key = "unique", .value = .{ .bool = true } },
|
||||
.{ .key = "key", .value = .{ .doc = &.{.{ .key = "a", .value = .{ .int32 = 1 } }} } },
|
||||
.{ .key = "sparse", .value = .{ .bool = true } },
|
||||
.{ .key = "partialFilterExpression", .value = .{ .doc = &.{
|
||||
.{ .key = "t", .value = .{ .bool = true } },
|
||||
} } },
|
||||
@@ -5714,10 +5719,6 @@ 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);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user