Fix six MongoDB query-semantics defects in the heuristics calculator - #1739
Closed
LautaroPetaccio wants to merge 7 commits into
Closed
Fix six MongoDB query-semantics defects in the heuristics calculator#1739LautaroPetaccio wants to merge 7 commits into
LautaroPetaccio wants to merge 7 commits into
Conversation
The truthness for {"f": {"$all": [...]}} was aggregated as an AND over the
elements of the array held by "f", where each element was scored by an OR
over the expected values. That asks "is every stored element one of the
queried values", which is the inverse of $all: the operator requires every
queried value to be present in the stored array, and is indifferent to extra
elements.
The direction was inverted in both ways:
{tags: {$all: ["a"]}} + {tags: ["a", "b"]} matches in MongoDB,
but was scored as false;
{tags: {$all: ["a","b","c"]}} + {tags: ["a"]} does not match,
but was scored as fully true, ie distance 0.
The second case is the damaging one: a condition no data can satisfy is
reported as already covered, so the search stops trying to reach it.
Aggregate as an AND over the expected values instead, scoring each one with
an OR over the elements of the array, which also restores a usable gradient:
a document missing fewer of the expected values now scores closer to true.
The existing tests only compared identical lists, where both directions agree,
so they did not catch this. The three added tests cover each failure direction
and the gradient.
computeHeuristicComparisonNonNullValues falls back to C_FALSE when no comparison
logic is defined for a pair of types, regardless of the operator. That is right for
equality and for the ordering operators, but wrong for $ne: MongoDB treats values of
different, mutually incomparable BSON types as different from each other, so an
inequality check on them holds.
{age: {$ne: "abc"}} against {age: 20} matches in MongoDB (verified on 7.0.40),
but was scored as definitely false, with no gradient that could ever reach true.
Return TRUE_C for NOT_EQUALS_TO in that fallback, and keep C_FALSE for the other
operators. Ordering comparisons across different BSON types do not match in MongoDB
either, so those were already correct; a test now pins that down.
$in checks each element when the queried field holds an array, but $nin passed the
whole array to the comparison instead. A List compared against a scalar has no
comparison logic defined, so it fell back to false, and the negation turned that into
a full match with distance 0.
{tags: {$nin: ["a"]}} against {tags: ["a", "b"]} does not match in MongoDB
(verified on 7.0.40), but was reported as satisfied.
$nin is the negation of $in, so both now share computeHeuristicForMembership and
differ only by that negation.
Two guards are needed by the shared path, and each fixes an IllegalArgumentException
thrown out of the empty aggregation that would otherwise be reachable:
an empty stored array holds none of the expected values, and an empty list of expected
values cannot be matched. In MongoDB {tags: {$in: []}} matches nothing and
{tags: {$nin: []}} matches everything, which is what these guards produce.
computeHeuristic for $not returned true whenever the queried field was absent from the
document. That is right only for inner operators that do not match an absent field.
Several do match one, and for those the negation must be false.
{a: {$not: {$ne: 5}}} against {} does not match in MongoDB (verified on
{a: {$not: {$nin: [1, 2]}}} 7.0.40), because the inner operator matches the
{a: {$not: {$exists: false}}} absent field. All three were reported as satisfied.
The generic path below the shortcut already produced the right answer in these cases,
since the inner operators treat an absent field as a null value, so the shortcut was
actively discarding a correct result. Removing it also keeps the complementary cases
($eq, $in, $exists:true, $gt, $size) true, as they were before.
… fixed The existing endpoints exercise these operators only in shapes that were already handled correctly: "ne" compares two numbers, "nin" is applied to a scalar field, and "not" wraps a $gt, which does not match an absent field. None of them reaches the paths that were wrong. Three endpoints are added for the shapes that were: neCrossType $ne between a string field and a number, ie two incomparable BSON types ninArrayField $nin on a field holding an array, which must be checked element-wise notExistsFalse $not wrapping an operator that does match a document lacking the field Note that "ninArrayField" is deliberately not satisfied by the two documents that saveData inserts, as both of them hold "a", so it can only be covered by data that EvoMaster generates itself.
$all, $in and $nin each decided on their own what it means for a field to hold a
value, and each got a different part of it wrong:
{f: {$all: ["a"]}} against {f: "a"} matches in MongoDB, but any
non-array field was scored false
{tags: {$in: [["a","b"]]}} against {tags: ["a","b"]} matches in MongoDB, but only
the elements were ever compared,
never the array itself
MongoDB has one rule for all of them: a field matches a value when its own value is
that value, or when it holds an array of which at least one element is. Both readings
apply at once, so {"tags": ["a","b"]} is matched both by ["a","b"] and by "a".
computeHeuristicForMatchedValue now implements that rule, and $all, $in and $nin are
expressed in terms of it: $all is an AND over the expected values, $in an OR, and
$nin the negation of $in. This drops the special cases for scalar and empty arrays,
which the rule covers on its own, and removes 11 lines overall.
Verified against MongoDB 7.0.40, including the cases where the two readings differ:
["b","a"] does not match ["a","b"], since arrays are equal only in the same order,
while an array holding ["a","b"] as an element does.
Also fixes an unrelated crash this uncovered, present since before this series:
comparing two empty arrays took the branch for lists of equal size, and aggregated
over their zero elements, throwing IllegalArgumentException instead of reporting the
two as equal. It is reachable from a plain {f: {$eq: []}}, and now from {f: {$in: [[]]}}.
$eq compared the field's value as a whole, so it missed the other half of MongoDB's
matching rule: a field holding an array is also matched by any one of its elements.
{f: {$eq: "a"}} against {f: ["a", "b"]} matches in MongoDB (verified on 7.0.40),
but was scored false.
$ne is the exact negation of $eq, and had the mirrored problem: it reported true for
the same document, where MongoDB does not match it.
Both now go through computeHeuristicForMatchedValue, which is what $all, $in and $nin
already use, so a single value is now interpreted the same way everywhere. This also
removes a live inconsistency: $in with one value already looked at array elements
while $eq with the same value did not, although MongoDB treats the two identically.
Note the whole-array reading already worked for $eq, since two lists were compared
element by element; only the reading that looks inside the array was missing. The
second added test pins the part that already held, so it passes either way.
Collaborator
Author
|
Closing this in favour of #1743, which carries the same findings as test cases only, with no change to the heuristics. Each defect is there as its own The implementation I had written here is still on |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Six independent defects in
MongoHeuristicsCalculator, each one a case where the computed truthness disagrees with what MongoDB actually does. Every claim below was checked by running the query against a real MongoDB 7.0.40 and comparing it with the calculator, rather than reasoning from the documentation.Supersedes #1737, which carried the
$allfix alone. These are folded together because they touch the same class and would otherwise need rebasing against each other; each commit stands on its own if you prefer to take them separately.$all{tags:{$all:["a","b","c"]}}vs["a"]$all{tags:{$all:["a"]}}vs["a","b"]$ne{age:{$ne:"abc"}}vs{age:20}$nin{tags:{$nin:["a"]}}vs["a","b"]$not{a:{$not:{$ne:5}}}vs{}$all{f:{$all:["a"]}}vs{f:"a"}$in{tags:{$in:[["a","b"]]}}vs["a","b"]$eq{f:{$eq:"a"}}vs{f:["a","b"]}$ne{f:{$ne:"a"}}vs{f:["a","b"]}The rows in bold are the damaging ones: a condition that no data can satisfy is reported as already covered, so the search stops generating data for it.
1.
$allwas quantified in the wrong directionThe score was an AND over the elements of the array held by the field, each scored by an OR over the expected values. That asks "is every stored element one of the queried values", which is the inverse of
$all:The two coincide exactly when the sets are equal, which is the only case
testAllcovered.v6.1.1had the correct direction; the quantifiers were transposed when the calculator moved toTruthness.Fixing the direction also restores a usable gradient. Averaging over the document's elements meant that adding an element not in the expected list lowered the score, so the search was rewarded for shrinking the array rather than adding the missing values. Measured after the fix, for
$all:[1,2,3]:1.0000 → 0.8425 → 0.6850 → 0.2923as more expected values go missing.2.
$nebetween incomparable BSON typescomputeHeuristicComparisonNonNullValuesfell back toC_FALSEwhenever no comparison logic was defined for a pair of types, regardless of operator. MongoDB treats values of different, incomparable BSON types as different from each other, so$neon them holds.NOT_EQUALS_TOnow returnsTRUE_Cin that fallback.Ordering operators were already right:
$gt/$gte/$lt/$ltegenuinely do not match across BSON types, which is now pinned by a test so it does not get "fixed" later.3.
$ninignored the elements of an array-valued field$inchecked each element;$ninpassed the whole array to the comparison, which has no defined comparison against a scalar, so it fell back to false and the negation turned that into a full match. Both now sharecomputeHeuristicForMembershipand differ only by the negation, which is the point: they are definitionally complementary.4.
$notshort-circuited on a missing field$notreturned true whenever the field was absent. That is right only for inner operators that do not match an absent field;$ne,$ninand$exists:falsedo match one, and negating them must then be false. The generic path below the shortcut already produced the correct answer in those cases, so the shortcut was discarding a correct result.5.
$all,$inand$nineach had their own idea of what "holds a value" meansMongoDB has a single rule: a field matches a value when its own value is that value, or
when it holds an array of which at least one element is. Both readings apply at once, so
{"tags": ["a","b"]}is matched both by["a","b"]and by"a".Each operator implemented a different half of it:
$allhandled only the array reading, so any non-array field scored false, even though{f:{$all:["a"]}}matches{f:"a"}.$in/$ninhandled only the element reading, so they never compared the array itself, eventhough
{tags:{$in:[["a","b"]]}}matches{tags:["a","b"]}.computeHeuristicForMatchedValuenow implements the rule once, and the three operators areexpressed in terms of it:
$allis an AND over the expected values,$inan OR, and$ninthenegation of
$in. That removes the special cases for scalar and empty arrays, which the rulecovers on its own, and is a net -11 lines.
Order sensitivity is covered too:
["b","a"]does not match["a","b"], since arrays are equalonly element-by-element in order, while an array holding
["a","b"]as an element does match.6.
$eqand$nedid not apply that rule either$eqcompared the field's value as a whole, so it missed the half of the rule that looks insidean array.
$ne, being its negation, had the mirrored problem. Both now go through the samehelper, so a single value means the same thing for every operator.
This also removes a live inconsistency that predates this PR:
$inwith one value alreadylooked at array elements while
$eqwith the same value did not, although MongoDB treats thetwo identically.
The whole-array reading already worked for
$eq, since two lists were compared element byelement; only the reading that looks inside the array was missing.
Two latent crashes fixed along the way
Routing
$ninthrough the shared membership path required guarding two cases that throw on currentmaster:Both escape
MongoHandler.computeFindDistance, which has notry/catch, and take the wholeExtraHeuristicsDtofor that action with them. In MongoDB$in:[]matches nothing and$nin:[]matches everything, which is what the guards now produce.A third one, older than this series and reachable from a plain
{f:{$eq:[]}}, is fixed in the last commit: comparing two empty arrays took the branch for lists of equal size and aggregated over their zero elements, throwing the same exception instead of reporting the two as equal.Tests
Unit — 21 tests added. Full Mongo suite on this branch: 293 tests, 0 failures, 0 skipped, including the Testcontainers-backed
MongoHandlerTestandMongoScriptRunnerTest.E2E — the existing endpoints exercise these operators only in shapes that were already correct:
necompares two numbers,ninis applied to a scalar field, andnotwraps a$gt, which does not match an absent field. Three endpoints are added for the shapes that were broken:neCrossType$nebetween a string field and a numberninArrayField$ninon a field holding an arraynotExistsFalse$notover an operator that matches an absent fieldninArrayFieldis deliberately not satisfied by either document thatsaveDatainserts, since both hold"a", so it can only be covered by data EvoMaster generates itself.$allalready has an endpoint and needed none.Notes
MongoHeuristicsCalculatorgoes from 934 to 987 lines, ie just under the 1000-line guideline infor_developers.md. Worth flagging given how quickly this class is growing.Point 6 is a behavioural change to the most widely used operator, since the implicit form
{f: "a"}parses to the same operation, so it is worth a closer look than the rest. It is thelast commit and can be dropped on its own if you would rather take it separately.