From 01c39d416435f930d249701ccb71744b519873b5 Mon Sep 17 00:00:00 2001 From: David Storch Date: Thu, 14 Dec 2017 14:49:07 -0500 Subject: SERVER-31760 Add InternalExprEqMatchExpression. --- src/mongo/db/matcher/SConscript | 2 + src/mongo/db/matcher/expression.h | 4 + src/mongo/db/matcher/expression_array.h | 11 +- .../db/matcher/expression_internal_expr_eq.cpp | 104 +++++++ src/mongo/db/matcher/expression_internal_expr_eq.h | 89 ++++++ .../matcher/expression_internal_expr_eq_test.cpp | 312 +++++++++++++++++++++ src/mongo/db/matcher/expression_leaf.h | 21 +- src/mongo/db/matcher/expression_parser.cpp | 7 + src/mongo/db/matcher/expression_parser.h | 1 + src/mongo/db/matcher/expression_parser_test.cpp | 29 +- src/mongo/db/matcher/expression_path.h | 21 +- .../db/matcher/expression_serialization_test.cpp | 15 + src/mongo/db/matcher/expression_type.h | 25 +- src/mongo/db/matcher/path.cpp | 27 +- src/mongo/db/matcher/path.h | 70 ++++- .../db/matcher/path_accepting_keyword_test.cpp | 3 + src/mongo/db/matcher/path_test.cpp | 108 ++++++- .../schema/expression_internal_schema_eq.cpp | 7 +- .../matcher/schema/expression_internal_schema_eq.h | 4 - .../expression_internal_schema_object_match.cpp | 8 +- .../expression_internal_schema_object_match.h | 4 - src/mongo/db/pipeline/document_source_match.cpp | 1 + .../db/pipeline/document_source_match_test.cpp | 4 + 23 files changed, 796 insertions(+), 81 deletions(-) create mode 100644 src/mongo/db/matcher/expression_internal_expr_eq.cpp create mode 100644 src/mongo/db/matcher/expression_internal_expr_eq.h create mode 100644 src/mongo/db/matcher/expression_internal_expr_eq_test.cpp (limited to 'src/mongo/db') diff --git a/src/mongo/db/matcher/SConscript b/src/mongo/db/matcher/SConscript index 4ccfaa219d0..2922ae05732 100644 --- a/src/mongo/db/matcher/SConscript +++ b/src/mongo/db/matcher/SConscript @@ -34,6 +34,7 @@ env.Library( 'expression_array.cpp', 'expression_expr.cpp', 'expression_geo.cpp', + 'expression_internal_expr_eq.cpp', 'expression_leaf.cpp', 'expression_parser.cpp', 'expression_text_base.cpp', @@ -87,6 +88,7 @@ env.CppUnitTest( 'expression_array_test.cpp', 'expression_expr_test.cpp', 'expression_geo_test.cpp', + 'expression_internal_expr_eq_test.cpp', 'expression_leaf_test.cpp', 'expression_optimize_test.cpp', 'expression_parser_geo_test.cpp', diff --git a/src/mongo/db/matcher/expression.h b/src/mongo/db/matcher/expression.h index d7cb4acbff7..6d588ef6dde 100644 --- a/src/mongo/db/matcher/expression.h +++ b/src/mongo/db/matcher/expression.h @@ -101,6 +101,10 @@ public: INTERNAL_2D_KEY_IN_REGION, INTERNAL_2D_POINT_IN_ANNULUS, + // Used to represent an expression language equality in a match expression tree, since $eq + // in the expression language has different semantics than the equality match expression. + INTERNAL_EXPR_EQ, + // JSON Schema expressions. INTERNAL_SCHEMA_ALLOWED_PROPERTIES, INTERNAL_SCHEMA_ALL_ELEM_MATCH_FROM_INDEX, diff --git a/src/mongo/db/matcher/expression_array.h b/src/mongo/db/matcher/expression_array.h index 4cf54f43677..454f6a4a335 100644 --- a/src/mongo/db/matcher/expression_array.h +++ b/src/mongo/db/matcher/expression_array.h @@ -46,9 +46,10 @@ namespace mongo { class ArrayMatchingMatchExpression : public PathMatchExpression { public: ArrayMatchingMatchExpression(MatchType matchType, StringData path) - : PathMatchExpression(matchType, path) { - setTraverseLeafArray(); - } + : PathMatchExpression(matchType, + path, + ElementPath::LeafArrayBehavior::kNoTraversal, + ElementPath::NonLeafArrayBehavior::kTraverse) {} virtual ~ArrayMatchingMatchExpression() {} @@ -63,10 +64,6 @@ public: bool equivalent(const MatchExpression* other) const override; - bool shouldExpandLeafArray() const final { - return false; - } - MatchCategory getCategory() const final { return MatchCategory::kArrayMatching; } diff --git a/src/mongo/db/matcher/expression_internal_expr_eq.cpp b/src/mongo/db/matcher/expression_internal_expr_eq.cpp new file mode 100644 index 00000000000..27d61db98a1 --- /dev/null +++ b/src/mongo/db/matcher/expression_internal_expr_eq.cpp @@ -0,0 +1,104 @@ +/** + * Copyright (C) 2017 MongoDB Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + * As a special exception, the copyright holders give permission to link the + * code of portions of this program with the OpenSSL library under certain + * conditions as described in each individual source file and distribute + * linked combinations including the program with the OpenSSL library. You + * must comply with the GNU Affero General Public License in all respects + * for all of the code used other than as permitted herein. If you modify + * file(s) with this exception, you may extend this exception to your + * version of the file(s), but you are not obligated to do so. If you do not + * wish to do so, delete this exception statement from your version. If you + * delete this exception statement from all source files in the program, + * then also delete it in the license file. + */ + +#include "mongo/platform/basic.h" + +#include "mongo/db/matcher/expression_internal_expr_eq.h" + +#include "mongo/bson/bsonobj.h" +#include "mongo/bson/bsonobjbuilder.h" + +namespace mongo { + +constexpr StringData InternalExprEqMatchExpression::kName; + +bool InternalExprEqMatchExpression::matchesSingleElement(const BSONElement& elem, + MatchDetails* details) const { + // We use NonLeafArrayBehavior::kMatchSubpath traversal in InternalExprEqMatchExpression. This + // means matchesSinglElement() will be called when an array is found anywhere along the patch we + // are matching against. When this occurs, we return 'true' and depend on the corresponding + // ExprMatchExpression node to filter properly. + if (elem.type() == BSONType::Array) { + return true; + } + + if (elem.canonicalType() != _rhsElem.canonicalType()) { + return false; + } + + auto comp = BSONElement::compareElements( + elem, _rhsElem, BSONElement::ComparisonRules::kConsiderFieldName, _collator); + return comp == 0; +} + +void InternalExprEqMatchExpression::debugString(StringBuilder& debug, int level) const { + _debugAddSpace(debug, level); + debug << path() << " " << kName << " " << _rhsElem.toString(false); + + auto td = getTag(); + if (td) { + debug << " "; + td->debugString(&debug); + } + + debug << "\n"; +} + +void InternalExprEqMatchExpression::serialize(BSONObjBuilder* builder) const { + BSONObjBuilder exprObj(builder->subobjStart(path())); + exprObj.appendAs(_rhsElem, kName); + exprObj.doneFast(); +} + +bool InternalExprEqMatchExpression::equivalent(const MatchExpression* other) const { + if (other->matchType() != matchType()) { + return false; + } + + const InternalExprEqMatchExpression* realOther = + static_cast(other); + + if (!CollatorInterface::collatorsMatch(_collator, realOther->_collator)) { + return false; + } + + constexpr StringData::ComparatorInterface* stringComparator = nullptr; + BSONElementComparator eltCmp(BSONElementComparator::FieldNamesMode::kIgnore, stringComparator); + return path() == realOther->path() && eltCmp.evaluate(_rhsElem == realOther->_rhsElem); +} + +std::unique_ptr InternalExprEqMatchExpression::shallowClone() const { + auto clone = stdx::make_unique(path(), _rhsElem); + clone->setCollator(_collator); + if (getTag()) { + clone->setTag(getTag()->clone()); + } + return std::move(clone); +} + +} // namespace mongo diff --git a/src/mongo/db/matcher/expression_internal_expr_eq.h b/src/mongo/db/matcher/expression_internal_expr_eq.h new file mode 100644 index 00000000000..c0747962a0a --- /dev/null +++ b/src/mongo/db/matcher/expression_internal_expr_eq.h @@ -0,0 +1,89 @@ +/** + * Copyright (C) 2017 MongoDB Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + * As a special exception, the copyright holders give permission to link the + * code of portions of this program with the OpenSSL library under certain + * conditions as described in each individual source file and distribute + * linked combinations including the program with the OpenSSL library. You + * must comply with the GNU Affero General Public License in all respects + * for all of the code used other than as permitted herein. If you modify + * file(s) with this exception, you may extend this exception to your + * version of the file(s), but you are not obligated to do so. If you do not + * wish to do so, delete this exception statement from your version. If you + * delete this exception statement from all source files in the program, + * then also delete it in the license file. + */ + +#pragma once + +#include "mongo/db/matcher/expression_leaf.h" + +namespace mongo { + +/** + * An InternalExprEqMatchExpression is an equality expression with similar semantics to the $eq + * expression. It differs from the regular equality match expression in the following ways: + * + * - The document will match if there is an array anywhere along the path. By always returning true + * in such cases, we match a superset of documents that the related aggregation expression would + * match. This sidesteps us having to implement field path expression evaluation as part of this + * match expression. + * + * - Equality to null matches literal nulls, but not documents in which the field path is missing or + * undefined. + * + * - Equality to undefined is legal, and matches either literal undefined, or documents in which the + * field path is missing. + */ +class InternalExprEqMatchExpression final : public LeafMatchExpression { +public: + static constexpr StringData kName = "$_internalExprEq"_sd; + + InternalExprEqMatchExpression(StringData path, BSONElement value) + : LeafMatchExpression(MatchType::INTERNAL_EXPR_EQ, + path, + ElementPath::LeafArrayBehavior::kNoTraversal, + ElementPath::NonLeafArrayBehavior::kMatchSubpath), + _rhsElem(value) {} + + bool matchesSingleElement(const BSONElement&, MatchDetails*) const final; + + void debugString(StringBuilder& debug, int level) const final; + + void serialize(BSONObjBuilder* out) const final; + + bool equivalent(const MatchExpression* other) const final; + + std::unique_ptr shallowClone() const final; + +protected: + /** + * 'collator' must outlive the InternalExprEqMatchExpression and any clones made of it. + */ + void _doSetCollator(const CollatorInterface* collator) final { + _collator = collator; + } + + // Collator used to compare elements. By default, simple binary comparison will be used. + const CollatorInterface* _collator = nullptr; + + BSONElement _rhsElem; + +private: + ExpressionOptimizerFunc getOptimizer() const final { + return [](std::unique_ptr expression) { return expression; }; + } +}; +} // namespace mongo diff --git a/src/mongo/db/matcher/expression_internal_expr_eq_test.cpp b/src/mongo/db/matcher/expression_internal_expr_eq_test.cpp new file mode 100644 index 00000000000..8846620b7bd --- /dev/null +++ b/src/mongo/db/matcher/expression_internal_expr_eq_test.cpp @@ -0,0 +1,312 @@ +/** + * Copyright (C) 2017 MongoDB Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + * As a special exception, the copyright holders give permission to link the + * code of portions of this program with the OpenSSL library under certain + * conditions as described in each individual source file and distribute + * linked combinations including the program with the OpenSSL library. You + * must comply with the GNU Affero General Public License in all respects + * for all of the code used other than as permitted herein. If you modify + * file(s) with this exception, you may extend this exception to your + * version of the file(s), but you are not obligated to do so. If you do not + * wish to do so, delete this exception statement from your version. If you + * delete this exception statement from all source files in the program, + * then also delete it in the license file. + */ + +#include "mongo/platform/basic.h" + +#include "mongo/bson/json.h" +#include "mongo/db/matcher/expression_internal_expr_eq.h" +#include "mongo/db/matcher/matcher.h" +#include "mongo/db/pipeline/expression_context_for_test.h" +#include "mongo/db/query/collation/collator_interface_mock.h" +#include "mongo/db/query/index_tag.h" +#include "mongo/unittest/unittest.h" + +namespace mongo { +namespace { + +const double kNaN = std::numeric_limits::quiet_NaN(); + +TEST(InternalExprEqMatchExpression, NodesWithDifferentCollationsAreNotEquivalent) { + auto operand = BSON("a" << 5); + CollatorInterfaceMock collator1(CollatorInterfaceMock::MockType::kReverseString); + InternalExprEqMatchExpression eq1(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + eq1.setCollator(&collator1); + CollatorInterfaceMock collator2(CollatorInterfaceMock::MockType::kAlwaysEqual); + InternalExprEqMatchExpression eq2(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + eq2.setCollator(&collator2); + ASSERT_FALSE(eq1.equivalent(&eq2)); +} + +TEST(InternalExprEqMatchExpression, ExprEqNotEquivalentToRegularEq) { + auto operand = BSON("a" << 5); + InternalExprEqMatchExpression exprEq(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + EqualityMatchExpression regularEq(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + ASSERT_FALSE(exprEq.equivalent(®ularEq)); +} + +TEST(InternalExprEqMatchExpression, NodesNotEquivalentWhenQueryDataDiffers) { + auto operand1 = BSON("a" << 5); + auto operand2 = BSON("a" << 6); + InternalExprEqMatchExpression eq1(operand1.firstElement().fieldNameStringData(), + operand1.firstElement()); + InternalExprEqMatchExpression eq2(operand2.firstElement().fieldNameStringData(), + operand2.firstElement()); + ASSERT_FALSE(eq1.equivalent(&eq2)); +} + +TEST(InternalExprEqMatchExpression, NodesNotEquivalentWhenQueryDataDiffersByFieldName) { + auto operand1 = BSON("a" << BSON("b" << 5)); + auto operand2 = BSON("a" << BSON("c" << 5)); + InternalExprEqMatchExpression eq1(operand1.firstElement().fieldNameStringData(), + operand1.firstElement()); + InternalExprEqMatchExpression eq2(operand2.firstElement().fieldNameStringData(), + operand2.firstElement()); + ASSERT_FALSE(eq1.equivalent(&eq2)); +} + +TEST(InternalExprEqMatchExpression, NodesAreEquivalentWhenTopLevelElementFieldNameDiffers) { + auto operand1 = BSON("b" << BSON("a" << 5)); + auto operand2 = BSON("c" << BSON("a" << 5)); + InternalExprEqMatchExpression eq1("path"_sd, operand1.firstElement()); + InternalExprEqMatchExpression eq2("path"_sd, operand2.firstElement()); + ASSERT_TRUE(eq1.equivalent(&eq2)); +} + +TEST(InternalExprEqMatchExpression, NodesNotEquivalentWhenPathDiffers) { + auto operand = BSON("a" << 5); + InternalExprEqMatchExpression eq1("path1"_sd, operand.firstElement()); + InternalExprEqMatchExpression eq2("path2"_sd, operand.firstElement()); + ASSERT_FALSE(eq1.equivalent(&eq2)); +} + +TEST(InternalExprEqMatchExpression, EquivalentNodesAreEquivalent) { + auto operand = BSON("a" << 5); + CollatorInterfaceMock collator1(CollatorInterfaceMock::MockType::kAlwaysEqual); + InternalExprEqMatchExpression eq1(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + eq1.setCollator(&collator1); + CollatorInterfaceMock collator2(CollatorInterfaceMock::MockType::kAlwaysEqual); + InternalExprEqMatchExpression eq2(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + eq2.setCollator(&collator2); + ASSERT_TRUE(eq1.equivalent(&eq2)); +} + +TEST(InternalExprEqMatchExpression, CorrectlyMatchesScalarElements) { + BSONObj operand1 = BSON("a" << 5); + + InternalExprEqMatchExpression eq1(operand1.firstElement().fieldNameStringData(), + operand1.firstElement()); + ASSERT_TRUE(eq1.matchesBSON(BSON("a" << 5.0))); + ASSERT_FALSE(eq1.matchesBSON(BSON("a" << 6))); + + BSONObj operand2 = BSON("a" + << "str"); + InternalExprEqMatchExpression eq2(operand2.firstElement().fieldNameStringData(), + operand2.firstElement()); + ASSERT_TRUE(eq2.matchesBSON(BSON("a" + << "str"))); + ASSERT_FALSE(eq2.matchesBSON(BSON("a" + << "string"))); +} + +TEST(InternalExprEqMatchExpression, StringMatchingRespectsCollation) { + BSONObj operand = BSON("a" + << "string"); + CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kAlwaysEqual); + InternalExprEqMatchExpression eq(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + eq.setCollator(&collator); + ASSERT_TRUE(eq.matchesBSON(BSON("a" + << "string2"))); +} + +TEST(InternalExprEqMatchExpression, ComparisonRespectsNewCollationAfterCallingSetCollator) { + BSONObj operand = BSON("a" + << "string1"); + + CollatorInterfaceMock collatorAlwaysEqual(CollatorInterfaceMock::MockType::kAlwaysEqual); + CollatorInterfaceMock collatorCompareLower(CollatorInterfaceMock::MockType::kToLowerString); + + InternalExprEqMatchExpression eq(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + eq.setCollator(&collatorAlwaysEqual); + ASSERT_TRUE(eq.matchesBSON(BSON("a" + << "string2"))); + + + eq.setCollator(&collatorCompareLower); + ASSERT_TRUE(eq.matchesBSON(BSON("a" + << "string1"))); + ASSERT_TRUE(eq.matchesBSON(BSON("a" + << "STRING1"))); + ASSERT_FALSE(eq.matchesBSON(BSON("a" + << "string2"))); +} + +TEST(InternalExprEqMatchExpression, CorrectlyMatchesArrayElement) { + BSONObj operand = BSON("a" << BSON_ARRAY("b" << 5)); + + InternalExprEqMatchExpression eq(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + ASSERT_TRUE(eq.matchesBSON(BSON("a" << BSON_ARRAY("b" << 5)))); + ASSERT_TRUE(eq.matchesBSON(BSON("a" << BSON_ARRAY("b" << BSON_ARRAY(5))))); + ASSERT_TRUE(eq.matchesBSON(BSON("a" << BSON_ARRAY(5 << "b")))); + ASSERT_TRUE(eq.matchesBSON(BSON("a" << BSON_ARRAY("b" << 5 << 5)))); + ASSERT_TRUE(eq.matchesBSON(BSON("a" << BSON_ARRAY("b" << 6)))); + ASSERT_TRUE(eq.matchesBSON(BSON("a" << BSON_ARRAY(BSON("b" << 6))))); + ASSERT_FALSE(eq.matchesBSON(BSON("a" << 1))); +} + +TEST(InternalSchemaEqMatchExpression, DoesNotTraverseThroughAnArrayWithANumericalPathComponent) { + BSONObj operand = BSON("" << 5); + InternalExprEqMatchExpression eq("a.0.b", operand.firstElement()); + ASSERT_TRUE(eq.matchesBSON(BSON("a" << BSON("0" << BSON("b" << 5))))); + ASSERT_FALSE(eq.matchesBSON(BSON("a" << BSON("0" << BSON("b" << 6))))); + ASSERT_TRUE(eq.matchesBSON(BSON("a" << BSON_ARRAY(BSON("b" << 7))))); +} + +TEST(InternalExprEqMatchExpression, CorrectlyMatchesNullElement) { + BSONObj operand = BSON("a" << BSONNULL); + + InternalExprEqMatchExpression eq(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + // Expression equality to null should match literal null, but not missing or undefined. + ASSERT_TRUE(eq.matchesBSON(BSON("a" << BSONNULL))); + ASSERT_FALSE(eq.matchesBSON(BSONObj())); + ASSERT_FALSE(eq.matchesBSON(BSON("a" << BSONUndefined))); + ASSERT_FALSE(eq.matchesBSON(BSON("a" << 4))); + ASSERT_TRUE(eq.matchesBSON(BSON("a" << BSON_ARRAY(1 << 2)))); +} + +TEST(InternalExprEqMatchExpression, CorrectlyMatchesUndefined) { + BSONObj operand = fromjson("{a: undefined}"); + + InternalExprEqMatchExpression eq(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + // Expression equality to undefined should match literal undefined and missing, but not null. + ASSERT_TRUE(eq.matchesBSON(BSON("a" << BSONUndefined))); + ASSERT_TRUE(eq.matchesBSON(BSONObj())); + ASSERT_FALSE(eq.matchesBSON(BSON("a" << BSONNULL))); + ASSERT_FALSE(eq.matchesBSON(BSON("a" << 4))); + ASSERT_TRUE(eq.matchesBSON(BSON("a" << BSON_ARRAY(1 << 2)))); +} + +TEST(InternalExprEqMatchExpression, CorrectlyMatchesNaN) { + BSONObj operand = BSON("x" << kNaN); + + InternalExprEqMatchExpression eq(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + ASSERT_TRUE(eq.matchesBSON(BSON("x" << kNaN))); + ASSERT_FALSE(eq.matchesBSON(BSON("x" << 0))); + ASSERT_FALSE(eq.matchesBSON(BSONObj())); + ASSERT_TRUE(eq.matchesBSON(BSON("x" << BSON_ARRAY(1)))); +} + +TEST(InternalExprEqMatchExpression, DoesNotTraverseLeafArrays) { + BSONObj operand = BSON("a" << 5); + + InternalExprEqMatchExpression eq(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + ASSERT_TRUE(eq.matchesBSON(BSON("a" << 5.0))); + ASSERT_TRUE(eq.matchesBSON(BSON("a" << BSON_ARRAY("foo")))); +} + +TEST(InternalExprEqMatchExpression, CorrectlyMatchesSubfieldAlongDottedPath) { + BSONObj operand = BSON("x.y.z" << 5); + + InternalExprEqMatchExpression eq(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + ASSERT_TRUE(eq.matchesBSON(BSON("x" << BSON("y" << BSON("z" << 5))))); + ASSERT_FALSE(eq.matchesBSON(BSON("x" << BSON("y" << BSON("z" << 4))))); + ASSERT_FALSE(eq.matchesBSON(BSON("x" << BSON("y" << 5)))); +} + +TEST(InternalExprEqMatchExpression, AlwaysMatchesDocumentWithArrayAlongPath) { + BSONObj operand = BSON("x.y.z" << 5); + + InternalExprEqMatchExpression eq(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + ASSERT_TRUE(eq.matchesBSON(BSON("x" << BSON_ARRAY(6)))); + ASSERT_TRUE(eq.matchesBSON(BSON("x" << BSON("y" << BSON_ARRAY(6))))); + ASSERT_TRUE(eq.matchesBSON(BSON("x" << BSON_ARRAY(BSON("y" << BSON("z" << 6)))))); + ASSERT_TRUE(eq.matchesBSON(BSON("x" << BSON("y" << BSON_ARRAY(BSON("z" << 6)))))); + ASSERT_TRUE(eq.matchesBSON(BSON("x" << BSON("y" << BSON("z" << BSON_ARRAY(10)))))); + + ASSERT_FALSE( + eq.matchesBSON(BSON("x" << BSON("y" << BSON("z" << BSON("foo" << BSON_ARRAY(10))))))); +} + +TEST(InternalExprEqMatchExpression, ConsidersFieldNameInObjectEquality) { + BSONObj operand = BSON("x" << BSON("a" << 1)); + + InternalExprEqMatchExpression eq(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + ASSERT_TRUE(eq.matchesBSON(BSON("x" << BSON("a" << 1)))); + ASSERT_FALSE(eq.matchesBSON(BSON("x" << BSON("y" << 1)))); + ASSERT_FALSE(eq.matchesBSON(BSON("y" << BSON("a" << 1)))); +} + +TEST(InternalExprEqMatchExpression, SerializesCorrectly) { + BSONObj operand = BSON("x" << 5); + + InternalExprEqMatchExpression eq(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + + BSONObjBuilder bob; + eq.serialize(&bob); + + ASSERT_BSONOBJ_EQ(BSON("x" << BSON("$_internalExprEq" << 5)), bob.obj()); +} + +TEST(InternalExprEqMatchExpression, EquivalentReturnsCorrectResults) { + auto query = fromjson("{a: {$_internalExprEq: {b: {c: 1, d: 1}}}}"); + boost::intrusive_ptr expCtx(new ExpressionContextForTest()); + Matcher eqExpr(query, expCtx); + + // Field order is considered when evaluating equivalency. + query = fromjson("{a: {$_internalExprEq: {b: {d: 1, c: 1}}}}"); + Matcher eqDifferentFieldOrder(query, expCtx); + ASSERT_FALSE( + eqExpr.getMatchExpression()->equivalent(eqDifferentFieldOrder.getMatchExpression())); + + query = fromjson("{a: {$_internalExprEq: {b: {d: 1}}}}"); + Matcher eqMissingField(query, expCtx); + ASSERT_FALSE(eqExpr.getMatchExpression()->equivalent(eqMissingField.getMatchExpression())); +} + +TEST(InternalExprEqMatchExpression, EquivalentToClone) { + auto query = fromjson("{a: {$_internalExprEq: {a:1, b: {c: 1, d: [1]}}}}"); + boost::intrusive_ptr expCtx(new ExpressionContextForTest()); + CollatorInterfaceMock collator(CollatorInterfaceMock::MockType::kReverseString); + Matcher eq(query, expCtx); + eq.getMatchExpression()->setCollator(&collator); + auto relevantTag = stdx::make_unique(); + relevantTag->first.push_back(0u); + relevantTag->notFirst.push_back(1u); + eq.getMatchExpression()->setTag(relevantTag.release()); + + auto clone = eq.getMatchExpression()->shallowClone(); + ASSERT_TRUE(eq.getMatchExpression()->equivalent(clone.get())); +} +} // namespace +} // namespace mongo diff --git a/src/mongo/db/matcher/expression_leaf.h b/src/mongo/db/matcher/expression_leaf.h index 77123742849..643ad22ab3b 100644 --- a/src/mongo/db/matcher/expression_leaf.h +++ b/src/mongo/db/matcher/expression_leaf.h @@ -47,12 +47,19 @@ class CollatorInterface; class LeafMatchExpression : public PathMatchExpression { public: - explicit LeafMatchExpression(MatchType matchType, StringData path) - : PathMatchExpression(matchType, path) { - setTraverseLeafArray(); - } + LeafMatchExpression(MatchType matchType, StringData path) + : LeafMatchExpression(matchType, + path, + ElementPath::LeafArrayBehavior::kTraverse, + ElementPath::NonLeafArrayBehavior::kTraverse) {} + + LeafMatchExpression(MatchType matchType, + StringData path, + ElementPath::LeafArrayBehavior leafArrBehavior, + ElementPath::NonLeafArrayBehavior nonLeafArrBehavior) + : PathMatchExpression(matchType, path, leafArrBehavior, nonLeafArrBehavior) {} - virtual ~LeafMatchExpression() {} + virtual ~LeafMatchExpression() = default; size_t numChildren() const override { return 0; @@ -66,10 +73,6 @@ public: return nullptr; } - bool shouldExpandLeafArray() const override { - return true; - } - MatchCategory getCategory() const override { return MatchCategory::kLeaf; } diff --git a/src/mongo/db/matcher/expression_parser.cpp b/src/mongo/db/matcher/expression_parser.cpp index aad03b33342..45888a3a241 100644 --- a/src/mongo/db/matcher/expression_parser.cpp +++ b/src/mongo/db/matcher/expression_parser.cpp @@ -43,6 +43,7 @@ #include "mongo/db/matcher/expression_array.h" #include "mongo/db/matcher/expression_expr.h" #include "mongo/db/matcher/expression_geo.h" +#include "mongo/db/matcher/expression_internal_expr_eq.h" #include "mongo/db/matcher/expression_leaf.h" #include "mongo/db/matcher/expression_tree.h" #include "mongo/db/matcher/expression_type.h" @@ -1564,6 +1565,11 @@ StatusWithMatchExpression parseSubField(const BSONObj& context, return {Status(ErrorCodes::BadValue, str::stream() << "near must be first in: " << context)}; + case PathAcceptingKeyword::INTERNAL_EXPR_EQ: { + auto exprEqExpr = stdx::make_unique(name, e); + exprEqExpr->setCollator(expCtx->getCollator()); + return {std::move(exprEqExpr)}; + } // Handles bitwise query operators. case PathAcceptingKeyword::BITS_ALL_SET: { @@ -1845,6 +1851,7 @@ MONGO_INITIALIZER(MatchExpressionParser)(InitializerContext* context) { queryOperatorMap = stdx::make_unique>(StringMap{ // TODO: SERVER-19565 Add $eq after auditing callers. + {"_internalExprEq", PathAcceptingKeyword::INTERNAL_EXPR_EQ}, {"_internalSchemaAllElemMatchFromIndex", PathAcceptingKeyword::INTERNAL_SCHEMA_ALL_ELEM_MATCH_FROM_INDEX}, {"_internalSchemaEq", PathAcceptingKeyword::INTERNAL_SCHEMA_EQ}, diff --git a/src/mongo/db/matcher/expression_parser.h b/src/mongo/db/matcher/expression_parser.h index bfc3affbff4..2a72cc6309f 100644 --- a/src/mongo/db/matcher/expression_parser.h +++ b/src/mongo/db/matcher/expression_parser.h @@ -61,6 +61,7 @@ enum class PathAcceptingKeyword { GEO_NEAR, GREATER_THAN, GREATER_THAN_OR_EQUAL, + INTERNAL_EXPR_EQ, INTERNAL_SCHEMA_ALL_ELEM_MATCH_FROM_INDEX, INTERNAL_SCHEMA_EQ, INTERNAL_SCHEMA_FMOD, diff --git a/src/mongo/db/matcher/expression_parser_test.cpp b/src/mongo/db/matcher/expression_parser_test.cpp index 669986004f4..a5904d68982 100644 --- a/src/mongo/db/matcher/expression_parser_test.cpp +++ b/src/mongo/db/matcher/expression_parser_test.cpp @@ -465,4 +465,31 @@ TEST(MatchExpressionParserTest, ExprFailsToParseWithinInternalSchemaObjectMatch) query, expCtx, ExtensionsCallbackNoop(), MatchExpressionParser::AllowedFeatures::kExpr) .getStatus()); } -} + +TEST(MatchExpressionParserTest, InternalExprEqParsesCorrectly) { + boost::intrusive_ptr expCtx(new ExpressionContextForTest()); + + auto query = fromjson("{a: {$_internalExprEq: 'foo'}}"); + auto statusWith = MatchExpressionParser::parse(query, expCtx); + ASSERT_OK(statusWith.getStatus()); + ASSERT_TRUE(statusWith.getValue()->matchesBSON(fromjson("{a: 'foo'}"))); + ASSERT_TRUE(statusWith.getValue()->matchesBSON(fromjson("{a: ['foo']}"))); + ASSERT_TRUE(statusWith.getValue()->matchesBSON(fromjson("{a: ['bar']}"))); + ASSERT_FALSE(statusWith.getValue()->matchesBSON(fromjson("{a: 'bar'}"))); + + query = fromjson("{'a.b': {$_internalExprEq: 5}}"); + statusWith = MatchExpressionParser::parse(query, expCtx); + ASSERT_OK(statusWith.getStatus()); + ASSERT_TRUE(statusWith.getValue()->matchesBSON(fromjson("{a: {b: 5}}"))); + ASSERT_TRUE(statusWith.getValue()->matchesBSON(fromjson("{a: {b: [5]}}"))); + ASSERT_TRUE(statusWith.getValue()->matchesBSON(fromjson("{a: {b: [6]}}"))); + ASSERT_FALSE(statusWith.getValue()->matchesBSON(fromjson("{a: {b: 6}}"))); + + query = fromjson("{'a.b': {$_internalExprEq: [5]}}"); + statusWith = MatchExpressionParser::parse(query, expCtx); + ASSERT_OK(statusWith.getStatus()); + ASSERT_TRUE(statusWith.getValue()->matchesBSON(fromjson("{a: {b: [5]}}"))); + ASSERT_TRUE(statusWith.getValue()->matchesBSON(fromjson("{a: {b: [6]}}"))); + ASSERT_FALSE(statusWith.getValue()->matchesBSON(fromjson("{a: {b: 5}}"))); +} +} // namespace mongo diff --git a/src/mongo/db/matcher/expression_path.h b/src/mongo/db/matcher/expression_path.h index b0a1ee5354c..189d9cb4a3b 100644 --- a/src/mongo/db/matcher/expression_path.h +++ b/src/mongo/db/matcher/expression_path.h @@ -41,23 +41,18 @@ namespace mongo { */ class PathMatchExpression : public MatchExpression { public: - explicit PathMatchExpression(MatchType matchType, StringData path) + PathMatchExpression(MatchType matchType, + StringData path, + ElementPath::LeafArrayBehavior leafArrBehavior, + ElementPath::NonLeafArrayBehavior nonLeafArrayBehavior) : MatchExpression(matchType), _path(path) { _elementPath.init(_path); + _elementPath.setLeafArrayBehavior(leafArrBehavior); + _elementPath.setNonLeafArrayBehavior(nonLeafArrayBehavior); } virtual ~PathMatchExpression() {} - /** - * Returns whether or not this expression should match against each element of an array (in - * addition to the array as a whole). - * - * For example, returns true if a path match expression on "f" should match against 1, 2, and - * [1, 2] for document {f: [1, 2]}. Returns false if this expression should only match against - * [1, 2]. - */ - virtual bool shouldExpandLeafArray() const = 0; - bool matches(const MatchableDocument* doc, MatchDetails* details = nullptr) const final { MatchableDocument::IteratorHolder cursor(doc, &_elementPath); while (cursor->more()) { @@ -82,10 +77,6 @@ public: _elementPath.init(_path); } - void setTraverseLeafArray() { - _elementPath.setTraverseLeafArray(shouldExpandLeafArray()); - } - /** * Finds an applicable rename from 'renameList' (if one exists) and applies it to the expression * path. Each pair in 'renameList' specifies a path prefix that should be renamed (as the first diff --git a/src/mongo/db/matcher/expression_serialization_test.cpp b/src/mongo/db/matcher/expression_serialization_test.cpp index fef34675935..f132116ee5f 100644 --- a/src/mongo/db/matcher/expression_serialization_test.cpp +++ b/src/mongo/db/matcher/expression_serialization_test.cpp @@ -1188,6 +1188,21 @@ TEST(SerializeBasic, ExpressionExprSerializesCorrectly) { ASSERT_EQ(original.matches(obj), reserialized.matches(obj)); } +TEST(SerializeBasic, ExpressionInternalExprEqSerializesCorrectly) { + boost::intrusive_ptr expCtx(new ExpressionContextForTest()); + Matcher original(fromjson("{'a.b': {$_internalExprEq: 'foo'}}"), expCtx); + Matcher reserialized(serialize(original.getMatchExpression()), expCtx); + + ASSERT_BSONOBJ_EQ(*reserialized.getQuery(), fromjson("{'a.b': {$_internalExprEq: 'foo'}}")); + ASSERT_BSONOBJ_EQ(*reserialized.getQuery(), serialize(reserialized.getMatchExpression())); + + BSONObj obj = fromjson("{a: {b: 'foo'}}"); + ASSERT_EQ(original.matches(obj), reserialized.matches(obj)); + + obj = fromjson("{a: {b: 3}}"); + ASSERT_EQ(original.matches(obj), reserialized.matches(obj)); +} + TEST(SerializeBasic, ExpressionCommentSerializesCorrectly) { boost::intrusive_ptr expCtx(new ExpressionContextForTest()); Matcher original(fromjson("{$comment: 'Hello'}"), diff --git a/src/mongo/db/matcher/expression_type.h b/src/mongo/db/matcher/expression_type.h index 9ccdbe62b63..d38ce365142 100644 --- a/src/mongo/db/matcher/expression_type.h +++ b/src/mongo/db/matcher/expression_type.h @@ -36,8 +36,13 @@ namespace mongo { template class TypeMatchExpressionBase : public LeafMatchExpression { public: - explicit TypeMatchExpressionBase(MatchType matchType, StringData path, MatcherTypeSet typeSet) - : LeafMatchExpression(matchType, path), _typeSet(std::move(typeSet)) {} + explicit TypeMatchExpressionBase(MatchType matchType, + StringData path, + ElementPath::LeafArrayBehavior leafArrBehavior, + MatcherTypeSet typeSet) + : LeafMatchExpression( + matchType, path, leafArrBehavior, ElementPath::NonLeafArrayBehavior::kTraverse), + _typeSet(std::move(typeSet)) {} virtual ~TypeMatchExpressionBase() = default; @@ -113,7 +118,10 @@ public: static constexpr StringData kName = "$type"_sd; TypeMatchExpression(StringData path, MatcherTypeSet typeSet) - : TypeMatchExpressionBase(MatchExpression::TYPE_OPERATOR, path, typeSet) {} + : TypeMatchExpressionBase(MatchExpression::TYPE_OPERATOR, + path, + ElementPath::LeafArrayBehavior::kTraverse, + typeSet) {} StringData name() const final { return kName; @@ -131,9 +139,10 @@ public: static constexpr StringData kName = "$_internalSchemaType"_sd; InternalSchemaTypeExpression(StringData path, MatcherTypeSet typeSet) - : TypeMatchExpressionBase(MatchExpression::INTERNAL_SCHEMA_TYPE, path, typeSet) { - setTraverseLeafArray(); - } + : TypeMatchExpressionBase(MatchExpression::INTERNAL_SCHEMA_TYPE, + path, + ElementPath::LeafArrayBehavior::kNoTraversal, + typeSet) {} StringData name() const final { return kName; @@ -142,10 +151,6 @@ public: MatchCategory getCategory() const final { return MatchCategory::kOther; } - - bool shouldExpandLeafArray() const final { - return false; - } }; } // namespace mongo diff --git a/src/mongo/db/matcher/path.cpp b/src/mongo/db/matcher/path.cpp index 8ecd2a99473..951ec5802c4 100644 --- a/src/mongo/db/matcher/path.cpp +++ b/src/mongo/db/matcher/path.cpp @@ -36,8 +36,8 @@ namespace mongo { void ElementPath::init(StringData path) { - _shouldTraverseNonleafArrays = true; - _shouldTraverseLeafArray = true; + _nonLeafArrayBehavior = NonLeafArrayBehavior::kTraverse; + _leafArrayBehavior = LeafArrayBehavior::kTraverse; _fieldRef.parse(path); } @@ -195,11 +195,12 @@ bool BSONElementIterator::subCursorHasMore() { _subCursorPath.reset(new ElementPath()); _subCursorPath->init(_arrayIterationState.restOfPath.substr( _arrayIterationState.nextPieceOfPath.size() + 1)); - _subCursorPath->setTraverseLeafArray(_path->shouldTraverseLeafArray()); + _subCursorPath->setLeafArrayBehavior(_path->leafArrayBehavior()); // If we're here, we must be able to traverse nonleaf arrays. - dassert(_path->shouldTraverseNonleafArrays()); - dassert(_subCursorPath->shouldTraverseNonleafArrays()); + dassert(_path->nonLeafArrayBehavior() == ElementPath::NonLeafArrayBehavior::kTraverse); + dassert(_subCursorPath->nonLeafArrayBehavior() == + ElementPath::NonLeafArrayBehavior::kTraverse); _subCursor.reset( new BSONElementIterator(_subCursorPath.get(), _arrayIterationState._current.Obj())); @@ -238,11 +239,19 @@ bool BSONElementIterator::more() { _arrayIterationState.reset(_path->fieldRef(), _traversalStartIndex + 1); - if (_arrayIterationState.hasMore && !_path->shouldTraverseNonleafArrays()) { + if (_arrayIterationState.hasMore && + _path->nonLeafArrayBehavior() != ElementPath::NonLeafArrayBehavior::kTraverse) { // Don't allow traversing the array. + if (_path->nonLeafArrayBehavior() == ElementPath::NonLeafArrayBehavior::kMatchSubpath) { + _next.reset(_traversalStart, BSONElement()); + _state = DONE; + return true; + } + _state = DONE; return false; - } else if (!_arrayIterationState.hasMore && !_path->shouldTraverseLeafArray()) { + } else if (!_arrayIterationState.hasMore && + _path->leafArrayBehavior() == ElementPath::LeafArrayBehavior::kNoTraversal) { // Return the leaf array. _next.reset(_traversalStart, BSONElement()); _state = DONE; @@ -275,7 +284,7 @@ bool BSONElementIterator::more() { // any elements matching the remaining subpath. _subCursorPath.reset(new ElementPath()); _subCursorPath->init(_arrayIterationState.restOfPath); - _subCursorPath->setTraverseLeafArray(_path->shouldTraverseLeafArray()); + _subCursorPath->setLeafArrayBehavior(_path->leafArrayBehavior()); _subCursor.reset(new BSONElementIterator(_subCursorPath.get(), eltInArray.Obj())); if (subCursorHasMore()) { @@ -302,7 +311,7 @@ bool BSONElementIterator::more() { _subCursorPath.reset(new ElementPath()); _subCursorPath->init(_arrayIterationState.restOfPath.substr( _arrayIterationState.nextPieceOfPath.size() + 1)); - _subCursorPath->setTraverseLeafArray(_path->shouldTraverseLeafArray()); + _subCursorPath->setLeafArrayBehavior(_path->leafArrayBehavior()); BSONElementIterator* real = new BSONElementIterator( _subCursorPath.get(), _arrayIterationState._current.Obj()); _subCursor.reset(real); diff --git a/src/mongo/db/matcher/path.h b/src/mongo/db/matcher/path.h index b769bd35c7e..db1ce321912 100644 --- a/src/mongo/db/matcher/path.h +++ b/src/mongo/db/matcher/path.h @@ -40,35 +40,75 @@ namespace mongo { class ElementPath { public: - ElementPath(StringData path, bool traverseNonLeafArr = true, bool traverseLeafArr = true) - : _shouldTraverseNonleafArrays(traverseNonLeafArr), - _shouldTraverseLeafArray(traverseLeafArr), + /** + * Controls how the element path interacts with leaf arrays, e.g. how we will handle the path + * "a.b" when "b" is an array. + */ + enum class LeafArrayBehavior { + // Matches against the elements of arrays at the end of the path (in addition to the array + // as a whole). + // + // For example, for the path "f" and document {f: [1, 2]}, causes the path iterator to + // return 1, 2, and [1, 2]. + kTraverse, + + // Does not traverse arrays at the end of the path. For the path "f" and document {f: [1, + // 2]}, the path iterator returns only the entire array [1, 2]. + kNoTraversal, + }; + + /** + * Controls how the element path interacts with non-leaf arrays, e.g. how we will handle the + * path "a.b" when "a" is an array. + */ + enum class NonLeafArrayBehavior { + // Path traversal spans non-leaf arrays. + kTraverse, + + // Path traversal stops at non-leaf array boundaries. The path iterator will return no + // elements. + kNoTraversal, + + // Path traversal stops at non-leaf array boundaries. The path iterator will return the + // array element. + kMatchSubpath, + }; + + ElementPath(StringData path, + LeafArrayBehavior leafArrayBehavior = LeafArrayBehavior::kTraverse, + NonLeafArrayBehavior nonLeafArrayBehavior = NonLeafArrayBehavior::kTraverse) + : _leafArrayBehavior(leafArrayBehavior), + _nonLeafArrayBehavior(nonLeafArrayBehavior), _fieldRef(path) {} // TODO: replace uses of members below with regular construction. ElementPath() {} void init(StringData path); - void setTraverseNonleafArrays(bool b) { - _shouldTraverseNonleafArrays = b; + void setLeafArrayBehavior(LeafArrayBehavior leafArrBehavior) { + _leafArrayBehavior = leafArrBehavior; } - void setTraverseLeafArray(bool b) { - _shouldTraverseLeafArray = b; + + LeafArrayBehavior leafArrayBehavior() const { + return _leafArrayBehavior; } - const FieldRef& fieldRef() const { - return _fieldRef; + void setNonLeafArrayBehavior(NonLeafArrayBehavior value) { + _nonLeafArrayBehavior = value; } - bool shouldTraverseNonleafArrays() const { - return _shouldTraverseNonleafArrays; + + NonLeafArrayBehavior nonLeafArrayBehavior() const { + return _nonLeafArrayBehavior; } - bool shouldTraverseLeafArray() const { - return _shouldTraverseLeafArray; + + const FieldRef& fieldRef() const { + return _fieldRef; } private: - bool _shouldTraverseNonleafArrays; - bool _shouldTraverseLeafArray; + LeafArrayBehavior _leafArrayBehavior; + NonLeafArrayBehavior _nonLeafArrayBehavior; + FieldRef _fieldRef; }; diff --git a/src/mongo/db/matcher/path_accepting_keyword_test.cpp b/src/mongo/db/matcher/path_accepting_keyword_test.cpp index 4830dd77505..d4aaafc714a 100644 --- a/src/mongo/db/matcher/path_accepting_keyword_test.cpp +++ b/src/mongo/db/matcher/path_accepting_keyword_test.cpp @@ -108,6 +108,9 @@ TEST(PathAcceptingKeyword, CanParseKnownMatchTypes) { ASSERT_TRUE(PathAcceptingKeyword::INTERNAL_SCHEMA_MAX_LENGTH == MatchExpressionParser::parsePathAcceptingKeyword( BSON("$_internalSchemaMaxLength" << 1).firstElement())); + ASSERT_TRUE(PathAcceptingKeyword::INTERNAL_EXPR_EQ == + MatchExpressionParser::parsePathAcceptingKeyword( + BSON("$_internalExprEq" << 1).firstElement())); } TEST(PathAcceptingKeyword, EqualityMatchReturnsDefault) { diff --git a/src/mongo/db/matcher/path_test.cpp b/src/mongo/db/matcher/path_test.cpp index f66b4764fad..d30cdbd60a3 100644 --- a/src/mongo/db/matcher/path_test.cpp +++ b/src/mongo/db/matcher/path_test.cpp @@ -79,7 +79,7 @@ TEST(Path, RootArray1) { TEST(Path, RootArray2) { ElementPath p; p.init("a"); - p.setTraverseLeafArray(false); + p.setLeafArrayBehavior(ElementPath::LeafArrayBehavior::kNoTraversal); BSONObj doc = BSON("x" << 4 << "a" << BSON_ARRAY(5 << 6)); @@ -181,7 +181,7 @@ TEST(Path, NestedEmptyArray) { TEST(Path, NestedNoLeaf1) { ElementPath p; p.init("a.b"); - p.setTraverseLeafArray(false); + p.setLeafArrayBehavior(ElementPath::LeafArrayBehavior::kNoTraversal); BSONObj doc = BSON("a" << BSON_ARRAY(BSON("b" << 5) << 3 << BSONObj() << BSON("b" << BSON_ARRAY(9 << 11)) @@ -210,6 +210,110 @@ TEST(Path, NestedNoLeaf1) { ASSERT(!cursor.more()); } +TEST(Path, MatchSubpathReturnsArrayOnSubpath) { + ElementPath path; + path.init("a.b.c"); + path.setLeafArrayBehavior(ElementPath::LeafArrayBehavior::kNoTraversal); + path.setNonLeafArrayBehavior(ElementPath::NonLeafArrayBehavior::kMatchSubpath); + + BSONObj doc = BSON("a" << BSON_ARRAY(BSON("b" << 5))); + + BSONElementIterator cursor(&path, doc); + + ASSERT(cursor.more()); + auto context = cursor.next(); + ASSERT_BSONELT_EQ(doc.firstElement(), context.element()); + + ASSERT(!cursor.more()); +} + +TEST(Path, MatchSubpathWithTraverseLeafFalseReturnsLeafArrayOnPath) { + ElementPath path; + path.init("a.b.c"); + path.setLeafArrayBehavior(ElementPath::LeafArrayBehavior::kNoTraversal); + path.setNonLeafArrayBehavior(ElementPath::NonLeafArrayBehavior::kMatchSubpath); + + BSONObj doc = BSON("a" << BSON("b" << BSON("c" << BSON_ARRAY(1 << 2)))); + + BSONElementIterator cursor(&path, doc); + + ASSERT(cursor.more()); + auto context = cursor.next(); + ASSERT_BSONELT_EQ(fromjson("{c: [1, 2]}").firstElement(), context.element()); + + ASSERT(!cursor.more()); +} + +TEST(Path, MatchSubpathWithTraverseLeafTrueReturnsLeafArrayAndValuesOnPath) { + ElementPath path; + path.init("a.b.c"); + path.setLeafArrayBehavior(ElementPath::LeafArrayBehavior::kTraverse); + path.setNonLeafArrayBehavior(ElementPath::NonLeafArrayBehavior::kMatchSubpath); + + BSONObj doc = BSON("a" << BSON("b" << BSON("c" << BSON_ARRAY(1 << 2)))); + + BSONElementIterator cursor(&path, doc); + + ASSERT(cursor.more()); + BSONElementIterator::Context context = cursor.next(); + ASSERT_EQUALS(1, context.element().numberInt()); + + ASSERT(cursor.more()); + context = cursor.next(); + ASSERT_EQUALS(2, context.element().numberInt()); + + ASSERT(cursor.more()); + context = cursor.next(); + ASSERT_BSONELT_EQ(fromjson("{c: [1, 2]}").firstElement(), context.element()); + + ASSERT(!cursor.more()); +} + +TEST(Path, MatchSubpathWithMultipleArraysReturnsOutermostArray) { + ElementPath path; + path.init("a.b.c"); + path.setLeafArrayBehavior(ElementPath::LeafArrayBehavior::kTraverse); + path.setNonLeafArrayBehavior(ElementPath::NonLeafArrayBehavior::kMatchSubpath); + + BSONObj doc = fromjson("{a: [{b: [{c: [1]}]}]}"); + + BSONElementIterator cursor(&path, doc); + + ASSERT(cursor.more()); + auto context = cursor.next(); + ASSERT_BSONELT_EQ(fromjson("{a: [{b: [{c: [1]}]}]}").firstElement(), context.element()); + + ASSERT(!cursor.more()); +} + +TEST(Path, NoTraversalOfNonLeafArrayReturnsNothingWithNonLeafArrayInDoc) { + ElementPath path; + path.init("a.b"); + path.setLeafArrayBehavior(ElementPath::LeafArrayBehavior::kTraverse); + path.setNonLeafArrayBehavior(ElementPath::NonLeafArrayBehavior::kNoTraversal); + + BSONObj doc = fromjson("{a: [{b: 1}]}"); + + BSONElementIterator cursor(&path, doc); + ASSERT(!cursor.more()); +} + +TEST(Path, MatchSubpathWithNumericalPathComponentReturnsEntireArray) { + ElementPath path; + path.init("a.0.b"); + path.setLeafArrayBehavior(ElementPath::LeafArrayBehavior::kTraverse); + path.setNonLeafArrayBehavior(ElementPath::NonLeafArrayBehavior::kMatchSubpath); + + BSONObj doc = fromjson("{a: [{b: 1}]}"); + + BSONElementIterator cursor(&path, doc); + + ASSERT(cursor.more()); + auto context = cursor.next(); + ASSERT_BSONELT_EQ(fromjson("{a: [{b: 1}]}").firstElement(), context.element()); + + ASSERT(!cursor.more()); +} TEST(Path, ArrayIndex1) { ElementPath p; diff --git a/src/mongo/db/matcher/schema/expression_internal_schema_eq.cpp b/src/mongo/db/matcher/schema/expression_internal_schema_eq.cpp index 54ec03a0bed..b28de000a3a 100644 --- a/src/mongo/db/matcher/schema/expression_internal_schema_eq.cpp +++ b/src/mongo/db/matcher/schema/expression_internal_schema_eq.cpp @@ -39,9 +39,12 @@ namespace mongo { constexpr StringData InternalSchemaEqMatchExpression::kName; InternalSchemaEqMatchExpression::InternalSchemaEqMatchExpression(StringData path, BSONElement rhs) - : LeafMatchExpression(MatchType::INTERNAL_SCHEMA_EQ, path), _rhsElem(rhs) { + : LeafMatchExpression(MatchType::INTERNAL_SCHEMA_EQ, + path, + ElementPath::LeafArrayBehavior::kNoTraversal, + ElementPath::NonLeafArrayBehavior::kTraverse), + _rhsElem(rhs) { invariant(_rhsElem); - setTraverseLeafArray(); } bool InternalSchemaEqMatchExpression::matchesSingleElement(const BSONElement& elem, diff --git a/src/mongo/db/matcher/schema/expression_internal_schema_eq.h b/src/mongo/db/matcher/schema/expression_internal_schema_eq.h index a8f85a2843e..5afa6860090 100644 --- a/src/mongo/db/matcher/schema/expression_internal_schema_eq.h +++ b/src/mongo/db/matcher/schema/expression_internal_schema_eq.h @@ -69,10 +69,6 @@ public: return nullptr; } - bool shouldExpandLeafArray() const final { - return false; - } - private: ExpressionOptimizerFunc getOptimizer() const final { return [](std::unique_ptr expression) { return expression; }; diff --git a/src/mongo/db/matcher/schema/expression_internal_schema_object_match.cpp b/src/mongo/db/matcher/schema/expression_internal_schema_object_match.cpp index d6a8c05342b..b1a3301ff78 100644 --- a/src/mongo/db/matcher/schema/expression_internal_schema_object_match.cpp +++ b/src/mongo/db/matcher/schema/expression_internal_schema_object_match.cpp @@ -36,9 +36,11 @@ constexpr StringData InternalSchemaObjectMatchExpression::kName; InternalSchemaObjectMatchExpression::InternalSchemaObjectMatchExpression( StringData path, std::unique_ptr expr) - : PathMatchExpression(INTERNAL_SCHEMA_OBJECT_MATCH, path), _sub(std::move(expr)) { - setTraverseLeafArray(); -} + : PathMatchExpression(INTERNAL_SCHEMA_OBJECT_MATCH, + path, + ElementPath::LeafArrayBehavior::kNoTraversal, + ElementPath::NonLeafArrayBehavior::kTraverse), + _sub(std::move(expr)) {} bool InternalSchemaObjectMatchExpression::matchesSingleElement(const BSONElement& elem, MatchDetails* details) const { diff --git a/src/mongo/db/matcher/schema/expression_internal_schema_object_match.h b/src/mongo/db/matcher/schema/expression_internal_schema_object_match.h index 776d35af6de..8f7b389de8b 100644 --- a/src/mongo/db/matcher/schema/expression_internal_schema_object_match.h +++ b/src/mongo/db/matcher/schema/expression_internal_schema_object_match.h @@ -67,10 +67,6 @@ public: return MatchCategory::kOther; } - bool shouldExpandLeafArray() const final { - return false; - } - private: ExpressionOptimizerFunc getOptimizer() const final; diff --git a/src/mongo/db/pipeline/document_source_match.cpp b/src/mongo/db/pipeline/document_source_match.cpp index 9d3b6365266..47e85a4c6e7 100644 --- a/src/mongo/db/pipeline/document_source_match.cpp +++ b/src/mongo/db/pipeline/document_source_match.cpp @@ -268,6 +268,7 @@ Document redactSafePortionDollarOps(BSONObj expr) { case PathAcceptingKeyword::EXISTS: case PathAcceptingKeyword::GEO_INTERSECTS: case PathAcceptingKeyword::GEO_NEAR: + case PathAcceptingKeyword::INTERNAL_EXPR_EQ: case PathAcceptingKeyword::INTERNAL_SCHEMA_ALL_ELEM_MATCH_FROM_INDEX: case PathAcceptingKeyword::INTERNAL_SCHEMA_EQ: case PathAcceptingKeyword::INTERNAL_SCHEMA_FMOD: diff --git a/src/mongo/db/pipeline/document_source_match_test.cpp b/src/mongo/db/pipeline/document_source_match_test.cpp index 84a3ac6c01c..4e14a990c7a 100644 --- a/src/mongo/db/pipeline/document_source_match_test.cpp +++ b/src/mongo/db/pipeline/document_source_match_test.cpp @@ -128,6 +128,10 @@ TEST_F(DocumentSourceMatchTest, RedactSafePortion) { assertExpectedRedactSafePortion("{a: {$_internalSchemaType: 2}}", "{}"); + // In some cases, $_internalExprEq could be redact-safe (just like a regular $eq match + // expression), but this optimization is not yet implemented. + assertExpectedRedactSafePortion("{a: {$_internalExprEq: 2}}", "{}"); + // Combinations assertExpectedRedactSafePortion("{a:1, b: 'asdf'}", "{a:1, b: 'asdf'}"); -- cgit v1.2.1