From a6bd767d38ca772b626afd7f90301a9701dda85f Mon Sep 17 00:00:00 2001 From: Andrii Dobroshynski Date: Thu, 17 Jun 2021 13:54:21 +0000 Subject: SERVER-57300 Fix logic for detecting numeric path component to avoid executing with SBE --- jstests/core/match_numeric_components.js | 84 +++++++++++++++++++++++++++++- src/mongo/db/matcher/expression_parser.cpp | 22 ++++---- 2 files changed, 96 insertions(+), 10 deletions(-) diff --git a/jstests/core/match_numeric_components.js b/jstests/core/match_numeric_components.js index fd00929e822..07a58507b8e 100644 --- a/jstests/core/match_numeric_components.js +++ b/jstests/core/match_numeric_components.js @@ -172,6 +172,81 @@ assert.commandWorked(coll.insert(kDocs)); assert.sameMembers(res, expected); } +// Using $exists with true and false. +{ + let res = coll.find({"a.0": {$exists: false}}).toArray(); + let expected = [{_id: 0, "a": 42}]; + + assert.sameMembers(res, expected); + + res = coll.find({"a.0": {$exists: true}}).toArray(); + expected = [ + {_id: 1, "a": [42]}, + {_id: 2, "a": {"0": 42}}, + {_id: 3, "a": [[42]]}, + {_id: 4, "a": [{"0": 42}]}, + {_id: 5, "a": {"0": [42]}}, + {_id: 6, "a": {"0": {"0": 42}}}, + {_id: 7, "a": [[[42]]]}, + {_id: 8, "a": [[{"0": 42}]]}, + {_id: 9, "a": [{"0": [42]}]}, + {_id: 10, "a": [{"0": {"0": 42}}]}, + {_id: 11, "a": {"0": [[42]]}}, + {_id: 12, "a": {"0": [{"0": 42}]}}, + {_id: 13, "a": {"0": {"0": [42]}}}, + {_id: 14, "a": {"0": {"0": {"0": 42}}}}, + {_id: 15, "a": [[[[42]]]]}, + {_id: 16, "a": [[[{"0": 42}]]]}, + {_id: 17, "a": [[{"0": [42]}]]}, + {_id: 18, "a": [[{"0": {"0": 42}}]]}, + {_id: 19, "a": [{"0": [[42]]}]}, + {_id: 20, "a": [{"0": [{"0": 42}]}]}, + {_id: 21, "a": [{"0": {"0": [42]}}]}, + {_id: 22, "a": [{"0": {"0": {"0": 42}}}]}, + {_id: 23, "a": {"0": [[[42]]]}}, + {_id: 24, "a": {"0": [[{"0": 42}]]}}, + {_id: 25, "a": {"0": [{"0": [42]}]}}, + {_id: 26, "a": {"0": [{"0": {"0": 42}}]}}, + {_id: 27, "a": {"0": {"0": [[42]]}}}, + {_id: 28, "a": {"0": {"0": [{"0": 42}]}}}, + {_id: 29, "a": {"0": {"0": {"0": [42]}}}}, + {_id: 30, "a": {"0": {"0": {"0": {"0": 42}}}}}, + ]; + + assert.sameMembers(res, expected); +} + +// Using $elemMatch. +{ + const res = coll.find({a: {$elemMatch: {"0.0": {$eq: 42}}}}).toArray(); + const expected = [ + {_id: 7, "a": [[[42]]]}, + {_id: 8, "a": [[{"0": 42}]]}, + {_id: 9, "a": [{"0": [42]}]}, + {_id: 10, "a": [{"0": {"0": 42}}]}, + {_id: 16, "a": [[[{"0": 42}]]]}, + {_id: 17, "a": [[{"0": [42]}]]}, + {_id: 20, "a": [{"0": [{"0": 42}]}]}, + {_id: 21, "a": [{"0": {"0": [42]}}]}, + ]; + + assert.sameMembers(res, expected); +} + +// Using top-level $and. +{ + const res = coll.find({_id: {$lt: 15}, "a.0": {$gt: 41}}).toArray(); + const expected = [ + {_id: 1, "a": [42]}, + {_id: 2, "a": {"0": 42}}, + {_id: 4, "a": [{"0": 42}]}, + {_id: 5, "a": {"0": [42]}}, + {_id: 9, "a": [{"0": [42]}]}, + ]; + + assert.sameMembers(res, expected); +} + // $all with equality { const res = coll.find({"a.0": {$all: [42]}}).toArray(); @@ -262,10 +337,17 @@ assert.commandWorked(coll2.insert(kRegexDocs)); assert.sameMembers(res, expected); } -// $all with regexes +// $all with regexes. { const res = coll2.find({"b.0": {$all: [/^hello/]}}).toArray(); const expected = [{_id: 2, "b": {"0": "hello"}}, {_id: 3, "b": ["hello", "abc", "abc"]}]; assert.sameMembers(res, expected); } + +// $not with regex. +{ + const res = coll2.find({"b.0": {$not: /^h/}}).toArray(); + const expected = [{_id: 1, "b": "hello"}]; + assert.sameMembers(res, expected); +} })(); diff --git a/src/mongo/db/matcher/expression_parser.cpp b/src/mongo/db/matcher/expression_parser.cpp index 798c5e59934..84802d9b5f8 100644 --- a/src/mongo/db/matcher/expression_parser.cpp +++ b/src/mongo/db/matcher/expression_parser.cpp @@ -94,16 +94,27 @@ bool hasNode(const MatchExpression* root, MatchExpression::MatchType type) { // TODO SERVER-49852: Currently SBE cannot handle match expressions with numeric path // components due to some of the complexity around how arrays are handled. void disableSBEForNumericPathComponent(const boost::intrusive_ptr& expCtx, - const FieldRef* fieldRef) { + const MatchExpression* node) { + auto fieldRef = node->fieldRef(); if (fieldRef && fieldRef->hasNumericPathComponents()) { expCtx->sbeCompatible = false; + return; + } + for (size_t i = 0; i < node->numChildren(); ++i) { + // For some match expressions trees, there could be a path associated with a node deeper in + // the tree. This is true in particular for negations. For example, {a: {$not: {$gt: 0}}} + // will be converted to a NOT => GT tree, but it is the GT node that carries the path, + // rather than the NOT node. + disableSBEForNumericPathComponent(expCtx, node->getChild(i)); + if (!expCtx->sbeCompatible) + return; } } void addExpressionToRoot(const boost::intrusive_ptr& expCtx, AndMatchExpression* root, std::unique_ptr newNode) { - disableSBEForNumericPathComponent(expCtx, newNode->fieldRef()); + disableSBEForNumericPathComponent(expCtx, newNode.get()); root->add(std::move(newNode)); } } // namespace @@ -1582,10 +1593,6 @@ StatusWithMatchExpression parseSubField(const BSONObj& context, expCtx, allowedFeatures); - // The NotMatchExpression below does not have a path, so 's' must be checked for a - // numeric path component instead. - disableSBEForNumericPathComponent(expCtx, s.getValue()->fieldRef()); - return {std::make_unique( s.getValue().release(), doc_validation_error::createAnnotation(expCtx, AnnotationMode::kIgnoreButDescend))}; @@ -1630,9 +1637,6 @@ StatusWithMatchExpression parseSubField(const BSONObj& context, return parseStatus; } - // The NotMatchExpression below does not have a path, so 's' must be checked for a - // numeric path component instead. - disableSBEForNumericPathComponent(expCtx, temp->fieldRef()); return {std::make_unique( temp.release(), doc_validation_error::createAnnotation(expCtx, AnnotationMode::kIgnoreButDescend))}; -- cgit v1.2.1