diff options
| author | Katherine Wu <katherine.wu@mongodb.com> | 2021-02-01 09:59:12 -0500 |
|---|---|---|
| committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2021-02-01 22:02:40 +0000 |
| commit | 120901099ba270b557054bc8ef06ac87418c0834 (patch) | |
| tree | fb2474b3c7621cd8b68325f71eac307d0e1b0518 /src/mongo/db | |
| parent | 060d4ef8514110ff4d03867b8f704cf9de307905 (diff) | |
| download | mongo-120901099ba270b557054bc8ef06ac87418c0834.tar.gz | |
SERVER-39943 Create match expressions for aggregation $gt/$gte/$lt/$lte and add to $expr rewrite
Diffstat (limited to 'src/mongo/db')
22 files changed, 1259 insertions, 208 deletions
diff --git a/src/mongo/db/matcher/SConscript b/src/mongo/db/matcher/SConscript index 1e9e2a7d199..eef4a2f9be7 100644 --- a/src/mongo/db/matcher/SConscript +++ b/src/mongo/db/matcher/SConscript @@ -27,7 +27,6 @@ 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', @@ -111,6 +110,7 @@ env.CppUnitTest( 'expression_array_test.cpp', 'expression_expr_test.cpp', 'expression_geo_test.cpp', + 'expression_internal_expr_comparison_test.cpp', 'expression_internal_expr_eq_test.cpp', 'expression_leaf_test.cpp', 'expression_optimize_test.cpp', diff --git a/src/mongo/db/matcher/doc_validation_error.cpp b/src/mongo/db/matcher/doc_validation_error.cpp index 8d0c73ab824..164bb3d16c2 100644 --- a/src/mongo/db/matcher/doc_validation_error.cpp +++ b/src/mongo/db/matcher/doc_validation_error.cpp @@ -806,6 +806,10 @@ public: generatePathError(*expr, kNormalReason, kInvertedReason); } void visit(const InternalExprEqMatchExpression* expr) final {} + void visit(const InternalExprGTMatchExpression* expr) final {} + void visit(const InternalExprGTEMatchExpression* expr) final {} + void visit(const InternalExprLTMatchExpression* expr) final {} + void visit(const InternalExprLTEMatchExpression* expr) final {} void visit(const InternalSchemaAllElemMatchFromIndexMatchExpression* expr) final { switch (toItemsKeywordType(*expr)) { case ItemsKeywordType::kItems: { @@ -1794,6 +1798,10 @@ public: } void visit(const InMatchExpression* expr) final {} void visit(const InternalExprEqMatchExpression* expr) final {} + void visit(const InternalExprGTMatchExpression* expr) final {} + void visit(const InternalExprGTEMatchExpression* expr) final {} + void visit(const InternalExprLTMatchExpression* expr) final {} + void visit(const InternalExprLTEMatchExpression* expr) final {} void visit(const InternalSchemaAllElemMatchFromIndexMatchExpression* expr) final {} void visit(const InternalSchemaAllowedPropertiesMatchExpression* expr) final { if (_context->shouldGenerateError(*expr)) { @@ -2002,6 +2010,10 @@ public: _context->finishCurrentError(expr); } void visit(const InternalExprEqMatchExpression* expr) final {} + void visit(const InternalExprGTMatchExpression* expr) final {} + void visit(const InternalExprGTEMatchExpression* expr) final {} + void visit(const InternalExprLTMatchExpression* expr) final {} + void visit(const InternalExprLTEMatchExpression* expr) final {} void visit(const InternalSchemaAllElemMatchFromIndexMatchExpression* expr) final { switch (toItemsKeywordType(*expr)) { case ItemsKeywordType::kItems: diff --git a/src/mongo/db/matcher/expression.h b/src/mongo/db/matcher/expression.h index 71c781fed09..3c3077ea5df 100644 --- a/src/mongo/db/matcher/expression.h +++ b/src/mongo/db/matcher/expression.h @@ -108,9 +108,14 @@ public: // Expressions that are only created internally 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. + // Used to represent expression language comparisons in a match expression tree, since $eq, + // $gt, $gte, $lt and $lte in the expression language has different semantics than their + // respective match expressions. INTERNAL_EXPR_EQ, + INTERNAL_EXPR_GT, + INTERNAL_EXPR_GTE, + INTERNAL_EXPR_LT, + INTERNAL_EXPR_LTE, // JSON Schema expressions. INTERNAL_SCHEMA_ALLOWED_PROPERTIES, diff --git a/src/mongo/db/matcher/expression_internal_expr_comparison.h b/src/mongo/db/matcher/expression_internal_expr_comparison.h new file mode 100644 index 00000000000..123813d1b80 --- /dev/null +++ b/src/mongo/db/matcher/expression_internal_expr_comparison.h @@ -0,0 +1,205 @@ +/** + * Copyright (C) 2021-present MongoDB, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * 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 + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * <http://www.mongodb.com/licensing/server-side-public-license>. + * + * 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 Server Side 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 { + +/** + * InternalExprComparisonMatchExpression consists of comparison expressions with similar semantics + * to the correlating $eq, $gt, $gte, $lt, and $lte aggregation expression. They differ from the + * regular comparison match expressions in the following ways: + * + * - There will be no type bracketing. For instance, null is considered less than 2, and objects + * are considered greater than 2. + * + * - 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. + * + * - Comparison to NaN will consider NaN as the less than all numbers. + * + * - Comparison to an array is illegal. It is invalid usage to construct a + * InternalExprComparisonMatchExpression node which compares to an array. + */ +template <typename T> +class InternalExprComparisonMatchExpression : public ComparisonMatchExpressionBase { +public: + InternalExprComparisonMatchExpression(MatchType type, StringData path, BSONElement value) + : ComparisonMatchExpressionBase(type, + path, + Value(value), + ElementPath::LeafArrayBehavior::kNoTraversal, + ElementPath::NonLeafArrayBehavior::kMatchSubpath) { + invariant(_rhs.type() != BSONType::Undefined); + invariant(_rhs.type() != BSONType::Array); + } + + virtual ~InternalExprComparisonMatchExpression() = default; + + bool matchesSingleElement(const BSONElement& elem, MatchDetails* details) const final { + // We use NonLeafArrayBehavior::kMatchSubpath traversal in + // InternalExprComparisonMatchExpression. 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; + } + + auto comp = elem.woCompare(_rhs, BSONElement::ComparisonRulesSet(0), _collator); + + switch (matchType()) { + case INTERNAL_EXPR_GT: + return comp > 0; + case INTERNAL_EXPR_GTE: + return comp >= 0; + case INTERNAL_EXPR_LT: + return comp < 0; + case INTERNAL_EXPR_LTE: + return comp <= 0; + case INTERNAL_EXPR_EQ: + return comp == 0; + default: + // This is a comparison match expression, so it must be either a $eq, $lt, $lte, $gt + // or $gte expression. + MONGO_UNREACHABLE_TASSERT(3994308); + } + }; + + std::unique_ptr<MatchExpression> shallowClone() const final { + auto clone = std::make_unique<T>(path(), _rhs); + clone->setCollator(_collator); + if (getTag()) { + clone->setTag(getTag()->clone()); + } + return clone; + } + + StringData name() const final { + return T::kName; + }; +}; + + +class InternalExprEqMatchExpression final + : public InternalExprComparisonMatchExpression<InternalExprEqMatchExpression> { +public: + static constexpr StringData kName = "$_internalExprEq"_sd; + + InternalExprEqMatchExpression(StringData path, BSONElement value) + : InternalExprComparisonMatchExpression<InternalExprEqMatchExpression>( + MatchType::INTERNAL_EXPR_EQ, path, value) {} + + void acceptVisitor(MatchExpressionMutableVisitor* visitor) final { + visitor->visit(this); + } + + void acceptVisitor(MatchExpressionConstVisitor* visitor) const final { + visitor->visit(this); + } +}; + +class InternalExprGTMatchExpression final + : public InternalExprComparisonMatchExpression<InternalExprGTMatchExpression> { +public: + static constexpr StringData kName = "$_internalExprGt"_sd; + + InternalExprGTMatchExpression(StringData path, BSONElement value) + : InternalExprComparisonMatchExpression<InternalExprGTMatchExpression>( + MatchType::INTERNAL_EXPR_GT, path, value) {} + + + void acceptVisitor(MatchExpressionMutableVisitor* visitor) final { + visitor->visit(this); + } + + void acceptVisitor(MatchExpressionConstVisitor* visitor) const final { + visitor->visit(this); + } +}; + +class InternalExprGTEMatchExpression final + : public InternalExprComparisonMatchExpression<InternalExprGTEMatchExpression> { +public: + static constexpr StringData kName = "$_internalExprGte"_sd; + + InternalExprGTEMatchExpression(StringData path, BSONElement value) + : InternalExprComparisonMatchExpression<InternalExprGTEMatchExpression>( + MatchType::INTERNAL_EXPR_GTE, path, value) {} + + void acceptVisitor(MatchExpressionMutableVisitor* visitor) final { + visitor->visit(this); + } + + void acceptVisitor(MatchExpressionConstVisitor* visitor) const final { + visitor->visit(this); + } +}; + +class InternalExprLTMatchExpression final + : public InternalExprComparisonMatchExpression<InternalExprLTMatchExpression> { +public: + static constexpr StringData kName = "$_internalExprLt"_sd; + + InternalExprLTMatchExpression(StringData path, BSONElement value) + : InternalExprComparisonMatchExpression<InternalExprLTMatchExpression>( + MatchType::INTERNAL_EXPR_LT, path, value) {} + + void acceptVisitor(MatchExpressionMutableVisitor* visitor) final { + visitor->visit(this); + } + + void acceptVisitor(MatchExpressionConstVisitor* visitor) const final { + visitor->visit(this); + } +}; + +class InternalExprLTEMatchExpression final + : public InternalExprComparisonMatchExpression<InternalExprLTEMatchExpression> { +public: + static constexpr StringData kName = "$_internalExprLte"_sd; + + InternalExprLTEMatchExpression(StringData path, BSONElement value) + : InternalExprComparisonMatchExpression<InternalExprLTEMatchExpression>( + MatchType::INTERNAL_EXPR_LTE, path, value) {} + + void acceptVisitor(MatchExpressionMutableVisitor* visitor) final { + visitor->visit(this); + } + + void acceptVisitor(MatchExpressionConstVisitor* visitor) const final { + visitor->visit(this); + } +}; + +} // namespace mongo diff --git a/src/mongo/db/matcher/expression_internal_expr_comparison_test.cpp b/src/mongo/db/matcher/expression_internal_expr_comparison_test.cpp new file mode 100644 index 00000000000..9085008ad75 --- /dev/null +++ b/src/mongo/db/matcher/expression_internal_expr_comparison_test.cpp @@ -0,0 +1,279 @@ +/** + * Copyright (C) 2021-present MongoDB, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * 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 + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * <http://www.mongodb.com/licensing/server-side-public-license>. + * + * 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 Server Side 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_comparison.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/death_test.h" +#include "mongo/unittest/unittest.h" + +namespace mongo { +namespace { + +const double kNaN = std::numeric_limits<double>::quiet_NaN(); + +TEST(InternalExprComparisonMatchExpression, DoesNotPerformTypeBracketing) { + BSONObj operand = BSON("x" << 2); + { + InternalExprGTMatchExpression gt(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + ASSERT_FALSE(gt.matchesBSON(BSON("x" << MINKEY))); + ASSERT_FALSE(gt.matchesBSON(BSON("y" << 0))); + ASSERT_FALSE(gt.matchesBSON(BSON("x" << BSONNULL))); + ASSERT_FALSE(gt.matchesBSON(BSON("x" << BSONUndefined))); + ASSERT_FALSE(gt.matchesBSON(BSON("x" << 1))); + ASSERT_FALSE(gt.matchesBSON(BSON("x" << 2))); + ASSERT_TRUE(gt.matchesBSON(BSON("x" << 3.5))); + ASSERT_TRUE(gt.matchesBSON(BSON("x" + << "string"))); + ASSERT_TRUE(gt.matchesBSON(BSON("x" << BSON("a" << 1)))); + ASSERT_TRUE(gt.matchesBSON(BSON("x" << BSON_ARRAY(1 << 2 << 3)))); + ASSERT_TRUE(gt.matchesBSON(BSON("x" << OID()))); + ASSERT_TRUE(gt.matchesBSON(BSON("x" << DATENOW))); + ASSERT_TRUE(gt.matchesBSON(BSON("x" << Timestamp(0, 3)))); + ASSERT_TRUE(gt.matchesBSON(BSON("x" + << "/^m/"))); + ASSERT_TRUE(gt.matchesBSON(BSON("x" << MAXKEY))); + } + { + InternalExprGTEMatchExpression gte(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + ASSERT_FALSE(gte.matchesBSON(BSON("x" << MINKEY))); + ASSERT_FALSE(gte.matchesBSON(BSON("y" << 0))); + ASSERT_FALSE(gte.matchesBSON(BSON("x" << BSONNULL))); + ASSERT_FALSE(gte.matchesBSON(BSON("x" << BSONUndefined))); + ASSERT_FALSE(gte.matchesBSON(BSON("x" << 1))); + ASSERT_TRUE(gte.matchesBSON(BSON("x" << 2))); + ASSERT_TRUE(gte.matchesBSON(BSON("x" << 3.5))); + ASSERT_TRUE(gte.matchesBSON(BSON("x" + << "string"))); + ASSERT_TRUE(gte.matchesBSON(BSON("x" << BSON("a" << 1)))); + ASSERT_TRUE(gte.matchesBSON(BSON("x" << BSON_ARRAY(1 << 2 << 3)))); + ASSERT_TRUE(gte.matchesBSON(BSON("x" << OID()))); + ASSERT_TRUE(gte.matchesBSON(BSON("x" << DATENOW))); + ASSERT_TRUE(gte.matchesBSON(BSON("x" << Timestamp(0, 3)))); + ASSERT_TRUE(gte.matchesBSON(BSON("x" + << "/^m/"))); + ASSERT_TRUE(gte.matchesBSON(BSON("x" << MAXKEY))); + } + { + InternalExprLTMatchExpression lt(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + ASSERT_TRUE(lt.matchesBSON(BSON("x" << MINKEY))); + ASSERT_TRUE(lt.matchesBSON(BSON("y" << 0))); + ASSERT_TRUE(lt.matchesBSON(BSON("x" << BSONNULL))); + ASSERT_TRUE(lt.matchesBSON(BSON("x" << BSONUndefined))); + ASSERT_TRUE(lt.matchesBSON(BSON("x" << 1))); + ASSERT_FALSE(lt.matchesBSON(BSON("x" << 2))); + ASSERT_FALSE(lt.matchesBSON(BSON("x" << 3.5))); + ASSERT_FALSE(lt.matchesBSON(BSON("x" + << "string"))); + ASSERT_FALSE(lt.matchesBSON(BSON("x" << BSON("a" << 1)))); + ASSERT_TRUE(lt.matchesBSON(BSON( + "x" << BSON_ARRAY(1 << 2 << 3)))); // Always returns true if path contains an array. + ASSERT_FALSE(lt.matchesBSON(BSON("x" << OID()))); + ASSERT_FALSE(lt.matchesBSON(BSON("x" << DATENOW))); + ASSERT_FALSE(lt.matchesBSON(BSON("x" << Timestamp(0, 3)))); + ASSERT_FALSE(lt.matchesBSON(BSON("x" + << "/^m/"))); + ASSERT_FALSE(lt.matchesBSON(BSON("x" << MAXKEY))); + } + { + InternalExprLTEMatchExpression lte(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + ASSERT_TRUE(lte.matchesBSON(BSON("x" << MINKEY))); + ASSERT_TRUE(lte.matchesBSON(BSON("y" << 0))); + ASSERT_TRUE(lte.matchesBSON(BSON("x" << BSONNULL))); + ASSERT_TRUE(lte.matchesBSON(BSON("x" << BSONUndefined))); + ASSERT_TRUE(lte.matchesBSON(BSON("x" << 1))); + ASSERT_TRUE(lte.matchesBSON(BSON("x" << 2))); + ASSERT_FALSE(lte.matchesBSON(BSON("x" << 3.5))); + ASSERT_FALSE(lte.matchesBSON(BSON("x" + << "string"))); + ASSERT_FALSE(lte.matchesBSON(BSON("x" << BSON("a" << 1)))); + ASSERT_TRUE(lte.matchesBSON(BSON( + "x" << BSON_ARRAY(1 << 2 << 3)))); // Always returns true if path contains an array. + ASSERT_FALSE(lte.matchesBSON(BSON("x" << OID()))); + ASSERT_FALSE(lte.matchesBSON(BSON("x" << DATENOW))); + ASSERT_FALSE(lte.matchesBSON(BSON("x" << Timestamp(0, 3)))); + ASSERT_FALSE(lte.matchesBSON(BSON("x" + << "/^m/"))); + ASSERT_FALSE(lte.matchesBSON(BSON("x" << MAXKEY))); + } +} + +TEST(InternalExprComparisonMatchExpression, CorrectlyComparesNaN) { + BSONObj operand = BSON("x" << kNaN); + // This behavior differs from how regular comparison MatchExpressions treat NaN, and places NaN + // within the total order of values as less than all numbers. + { + InternalExprGTMatchExpression gt(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + ASSERT_FALSE(gt.matchesBSON(BSON("x" << MINKEY))); + ASSERT_FALSE(gt.matchesBSON(BSON("x" << BSONNULL))); + ASSERT_FALSE(gt.matchesBSON(BSON("x" << kNaN))); + ASSERT_TRUE(gt.matchesBSON(BSON("x" << Decimal128::kNegativeInfinity))); + ASSERT_TRUE(gt.matchesBSON(BSON("x" << 2))); + ASSERT_TRUE(gt.matchesBSON(BSON("x" << 3.5))); + ASSERT_TRUE(gt.matchesBSON(BSON("x" << MAXKEY))); + } + { + InternalExprGTEMatchExpression gte(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + ASSERT_FALSE(gte.matchesBSON(BSON("x" << MINKEY))); + ASSERT_FALSE(gte.matchesBSON(BSON("x" << BSONNULL))); + ASSERT_TRUE(gte.matchesBSON(BSON("x" << kNaN))); + ASSERT_TRUE(gte.matchesBSON(BSON("x" << Decimal128::kNegativeInfinity))); + ASSERT_TRUE(gte.matchesBSON(BSON("x" << 2))); + ASSERT_TRUE(gte.matchesBSON(BSON("x" << 3.5))); + ASSERT_TRUE(gte.matchesBSON(BSON("x" << MAXKEY))); + } + { + InternalExprLTMatchExpression lt(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + ASSERT_TRUE(lt.matchesBSON(BSON("x" << MINKEY))); + ASSERT_TRUE(lt.matchesBSON(BSON("x" << BSONNULL))); + ASSERT_FALSE(lt.matchesBSON(BSON("x" << kNaN))); + ASSERT_FALSE(lt.matchesBSON(BSON("x" << Decimal128::kNegativeInfinity))); + ASSERT_FALSE(lt.matchesBSON(BSON("x" << 2))); + ASSERT_FALSE(lt.matchesBSON(BSON("x" << 3.5))); + ASSERT_FALSE(lt.matchesBSON(BSON("x" << MAXKEY))); + } + { + InternalExprLTEMatchExpression lte(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + ASSERT_TRUE(lte.matchesBSON(BSON("x" << MINKEY))); + ASSERT_TRUE(lte.matchesBSON(BSON("x" << BSONNULL))); + ASSERT_TRUE(lte.matchesBSON(BSON("x" << kNaN))); + ASSERT_FALSE(lte.matchesBSON(BSON("x" << Decimal128::kNegativeInfinity))); + ASSERT_FALSE(lte.matchesBSON(BSON("x" << 2))); + ASSERT_FALSE(lte.matchesBSON(BSON("x" << 3.5))); + ASSERT_FALSE(lte.matchesBSON(BSON("x" << MAXKEY))); + } +} + +TEST(InternalExprComparisonMatchExpression, AlwaysReturnsTrueWithLeafArrays) { + BSONObj operand = BSON("x" << 2); + { + InternalExprGTMatchExpression gt(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + ASSERT_TRUE(gt.matchesBSON(BSON("x" << BSON_ARRAY(BSONNULL)))); + ASSERT_TRUE(gt.matchesBSON(BSON("x" << BSON_ARRAY(0)))); + ASSERT_TRUE(gt.matchesBSON(BSON("x" << BSON_ARRAY(1 << 2 << 3)))); + ASSERT_FALSE(gt.matchesBSON(BSON("x" << 1))); + } + { + InternalExprGTEMatchExpression gte(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + ASSERT_TRUE(gte.matchesBSON(BSON("x" << BSON_ARRAY(BSONNULL)))); + ASSERT_TRUE(gte.matchesBSON(BSON("x" << BSON_ARRAY(0)))); + ASSERT_TRUE(gte.matchesBSON(BSON("x" << BSON_ARRAY(1 << 2 << 3)))); + ASSERT_FALSE(gte.matchesBSON(BSON("x" << 1))); + } + { + InternalExprLTMatchExpression lt(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + ASSERT_TRUE(lt.matchesBSON(BSON("x" << BSON_ARRAY(BSONNULL)))); + ASSERT_TRUE(lt.matchesBSON(BSON("x" << BSON_ARRAY(0)))); + ASSERT_TRUE(lt.matchesBSON(BSON("x" << BSON_ARRAY(1 << 2 << 3)))); + ASSERT_FALSE(lt.matchesBSON(BSON("x" << 3))); + } + { + InternalExprLTEMatchExpression lte(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + ASSERT_TRUE(lte.matchesBSON(BSON("x" << BSON_ARRAY(BSONNULL)))); + ASSERT_TRUE(lte.matchesBSON(BSON("x" << BSON_ARRAY(0)))); + ASSERT_TRUE(lte.matchesBSON(BSON("x" << BSON_ARRAY(1 << 2 << 3)))); + ASSERT_FALSE(lte.matchesBSON(BSON("x" << 3))); + } +} + +TEST(InternalExprComparisonMatchExpression, AlwaysReturnsTrueWithNonLeafArrays) { + BSONObj operand = BSON("x.y" << 2); + { + InternalExprGTMatchExpression gt(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + ASSERT_TRUE(gt.matchesBSON(BSON("x" << BSON_ARRAY(BSONNULL)))); + ASSERT_TRUE(gt.matchesBSON(BSON("x" << BSON_ARRAY(1 << 2 << 3)))); + ASSERT_TRUE(gt.matchesBSON(BSON("x" << BSON_ARRAY(BSON("y" << 1))))); + ASSERT_TRUE(gt.matchesBSON(BSON("x" << BSON_ARRAY(BSON("y" << 2) << BSON("y" << 3))))); + ASSERT_FALSE(gt.matchesBSON(BSON("x" << BSON("y" << 1)))); + } + { + InternalExprGTEMatchExpression gte(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + ASSERT_TRUE(gte.matchesBSON(BSON("x" << BSON_ARRAY(BSONNULL)))); + ASSERT_TRUE(gte.matchesBSON(BSON("x" << BSON_ARRAY(1 << 2 << 3)))); + ASSERT_TRUE(gte.matchesBSON(BSON("x" << BSON_ARRAY(BSON("y" << 1))))); + ASSERT_TRUE(gte.matchesBSON(BSON("x" << BSON_ARRAY(BSON("y" << 2) << BSON("y" << 3))))); + ASSERT_FALSE(gte.matchesBSON(BSON("x" << BSON("y" << 1)))); + } + { + InternalExprLTMatchExpression lt(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + ASSERT_TRUE(lt.matchesBSON(BSON("x" << BSON_ARRAY(BSONNULL)))); + ASSERT_TRUE(lt.matchesBSON(BSON("x" << BSON_ARRAY(1 << 2 << 3)))); + ASSERT_TRUE(lt.matchesBSON(BSON("x" << BSON_ARRAY(BSON("y" << 3))))); + ASSERT_TRUE(lt.matchesBSON(BSON("x" << BSON_ARRAY(BSON("y" << 1) << BSON("y" << 2))))); + ASSERT_FALSE(lt.matchesBSON(BSON("x" << BSON("y" << 3)))); + } + { + InternalExprLTEMatchExpression lte(operand.firstElement().fieldNameStringData(), + operand.firstElement()); + ASSERT_TRUE(lte.matchesBSON(BSON("x" << BSON_ARRAY(BSONNULL)))); + ASSERT_TRUE(lte.matchesBSON(BSON("x" << BSON_ARRAY(1 << 2 << 3)))); + ASSERT_TRUE(lte.matchesBSON(BSON("x" << BSON_ARRAY(BSON("y" << 3))))); + ASSERT_TRUE(lte.matchesBSON(BSON("x" << BSON_ARRAY(BSON("y" << 1) << BSON("y" << 2))))); + ASSERT_FALSE(lte.matchesBSON(BSON("x" << BSON("y" << 3)))); + } +} + +DEATH_TEST_REGEX(InternalExprComparisonMatchExpression, + CannotCompareToArray, + R"#(Invariant failure.*_rhs.type\(\) != BSONType::Array)#") { + BSONObj operand = BSON("x" << BSON_ARRAY(1 << 2)); + InternalExprGTMatchExpression gt(operand.firstElement().fieldNameStringData(), + operand.firstElement()); +} + +DEATH_TEST_REGEX(InternalExprComparisonMatchExpression, + CannotCompareToUndefined, + R"#(Invariant failure.*_rhs.type\(\) != BSONType::Undefined)#") { + BSONObj operand = BSON("x" << BSONUndefined); + InternalExprGTMatchExpression gt(operand.firstElement().fieldNameStringData(), + operand.firstElement()); +} + + +} // namespace +} // namespace mongo diff --git a/src/mongo/db/matcher/expression_internal_expr_eq.cpp b/src/mongo/db/matcher/expression_internal_expr_eq.cpp deleted file mode 100644 index 83d84254c32..00000000000 --- a/src/mongo/db/matcher/expression_internal_expr_eq.cpp +++ /dev/null @@ -1,69 +0,0 @@ -/** - * Copyright (C) 2018-present MongoDB, Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the Server Side Public License, version 1, - * as published by MongoDB, Inc. - * - * 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 - * Server Side Public License for more details. - * - * You should have received a copy of the Server Side Public License - * along with this program. If not, see - * <http://www.mongodb.com/licensing/server-side-public-license>. - * - * 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 Server Side 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() != _rhs.canonicalType()) { - return false; - } - - auto comp = BSONElement::compareElements( - elem, _rhs, BSONElement::ComparisonRules::kConsiderFieldName, _collator); - return comp == 0; -} - -std::unique_ptr<MatchExpression> InternalExprEqMatchExpression::shallowClone() const { - auto clone = std::make_unique<InternalExprEqMatchExpression>(path(), _rhs); - clone->setCollator(_collator); - if (getTag()) { - clone->setTag(getTag()->clone()); - } - return 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 deleted file mode 100644 index 2c52eb4e67d..00000000000 --- a/src/mongo/db/matcher/expression_internal_expr_eq.h +++ /dev/null @@ -1,82 +0,0 @@ -/** - * Copyright (C) 2018-present MongoDB, Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the Server Side Public License, version 1, - * as published by MongoDB, Inc. - * - * 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 - * Server Side Public License for more details. - * - * You should have received a copy of the Server Side Public License - * along with this program. If not, see - * <http://www.mongodb.com/licensing/server-side-public-license>. - * - * 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 Server Side 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 an array is illegal. It is invalid usage to construct a - * InternalExprEqMatchExpression node which compares to an array. - */ -class InternalExprEqMatchExpression final : public ComparisonMatchExpressionBase { -public: - static constexpr StringData kName = "$_internalExprEq"_sd; - - InternalExprEqMatchExpression(StringData path, BSONElement value) - : ComparisonMatchExpressionBase(MatchType::INTERNAL_EXPR_EQ, - path, - Value(value), - ElementPath::LeafArrayBehavior::kNoTraversal, - ElementPath::NonLeafArrayBehavior::kMatchSubpath) { - invariant(_rhs.type() != BSONType::Undefined); - invariant(_rhs.type() != BSONType::Array); - } - - StringData name() const final { - return kName; - } - - bool matchesSingleElement(const BSONElement&, MatchDetails*) const final; - - std::unique_ptr<MatchExpression> shallowClone() const final; - - void acceptVisitor(MatchExpressionMutableVisitor* visitor) final { - visitor->visit(this); - } - - void acceptVisitor(MatchExpressionConstVisitor* visitor) const final { - visitor->visit(this); - } -}; - -} // 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 index bd5ad2d878a..0de1fbec00b 100644 --- a/src/mongo/db/matcher/expression_internal_expr_eq_test.cpp +++ b/src/mongo/db/matcher/expression_internal_expr_eq_test.cpp @@ -30,7 +30,7 @@ #include "mongo/platform/basic.h" #include "mongo/bson/json.h" -#include "mongo/db/matcher/expression_internal_expr_eq.h" +#include "mongo/db/matcher/expression_internal_expr_comparison.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" diff --git a/src/mongo/db/matcher/expression_leaf.h b/src/mongo/db/matcher/expression_leaf.h index d9a4d6192dc..e042e6d99fb 100644 --- a/src/mongo/db/matcher/expression_leaf.h +++ b/src/mongo/db/matcher/expression_leaf.h @@ -104,6 +104,19 @@ public: } } + static bool isInternalExprComparison(MatchType matchType) { + switch (matchType) { + case MatchExpression::INTERNAL_EXPR_EQ: + case MatchExpression::INTERNAL_EXPR_GT: + case MatchExpression::INTERNAL_EXPR_GTE: + case MatchExpression::INTERNAL_EXPR_LT: + case MatchExpression::INTERNAL_EXPR_LTE: + return true; + default: + return false; + } + } + ComparisonMatchExpressionBase(MatchType type, StringData path, Value rhs, diff --git a/src/mongo/db/matcher/expression_parser.cpp b/src/mongo/db/matcher/expression_parser.cpp index 197cc481e3c..7ef37bc2e02 100644 --- a/src/mongo/db/matcher/expression_parser.cpp +++ b/src/mongo/db/matcher/expression_parser.cpp @@ -44,7 +44,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_internal_expr_comparison.h" #include "mongo/db/matcher/expression_leaf.h" #include "mongo/db/matcher/expression_tree.h" #include "mongo/db/matcher/expression_type.h" @@ -1692,6 +1692,55 @@ StatusWithMatchExpression parseSubField(const BSONObj& context, return {std::move(exprEqExpr)}; } + case PathAcceptingKeyword::INTERNAL_EXPR_GT: { + if (e.type() == BSONType::Undefined || e.type() == BSONType::Array) { + return {Status(ErrorCodes::BadValue, + str::stream() << InternalExprGTMatchExpression::kName + << " cannot be used to compare to type: " + << typeName(e.type()))}; + } + + auto exprGtExpr = std::make_unique<InternalExprGTMatchExpression>(name, e); + exprGtExpr->setCollator(expCtx->getCollator()); + return {std::move(exprGtExpr)}; + } + case PathAcceptingKeyword::INTERNAL_EXPR_GTE: { + if (e.type() == BSONType::Undefined || e.type() == BSONType::Array) { + return {Status(ErrorCodes::BadValue, + str::stream() << InternalExprGTEMatchExpression::kName + << " cannot be used to compare to type: " + << typeName(e.type()))}; + } + + auto exprGteExpr = std::make_unique<InternalExprGTEMatchExpression>(name, e); + exprGteExpr->setCollator(expCtx->getCollator()); + return {std::move(exprGteExpr)}; + } + case PathAcceptingKeyword::INTERNAL_EXPR_LT: { + if (e.type() == BSONType::Undefined || e.type() == BSONType::Array) { + return {Status(ErrorCodes::BadValue, + str::stream() << InternalExprLTMatchExpression::kName + << " cannot be used to compare to type: " + << typeName(e.type()))}; + } + + auto exprLtExpr = std::make_unique<InternalExprLTMatchExpression>(name, e); + exprLtExpr->setCollator(expCtx->getCollator()); + return {std::move(exprLtExpr)}; + } + case PathAcceptingKeyword::INTERNAL_EXPR_LTE: { + if (e.type() == BSONType::Undefined || e.type() == BSONType::Array) { + return {Status(ErrorCodes::BadValue, + str::stream() << InternalExprLTEMatchExpression::kName + << " cannot be used to compare to type: " + << typeName(e.type()))}; + } + + auto exprLteExpr = std::make_unique<InternalExprLTEMatchExpression>(name, e); + exprLteExpr->setCollator(expCtx->getCollator()); + return {std::move(exprLteExpr)}; + } + // Handles bitwise query operators. case PathAcceptingKeyword::BITS_ALL_SET: { return parseBitTest<BitsAllSetMatchExpression>(name, e, expCtx); @@ -1990,6 +2039,10 @@ MONGO_INITIALIZER(MatchExpressionParser)(InitializerContext* context) { queryOperatorMap = std::make_unique<StringMap<PathAcceptingKeyword>>(StringMap<PathAcceptingKeyword>{ {"_internalExprEq", PathAcceptingKeyword::INTERNAL_EXPR_EQ}, + {"_internalExprGt", PathAcceptingKeyword::INTERNAL_EXPR_GT}, + {"_internalExprGte", PathAcceptingKeyword::INTERNAL_EXPR_GTE}, + {"_internalExprLt", PathAcceptingKeyword::INTERNAL_EXPR_LT}, + {"_internalExprLte", PathAcceptingKeyword::INTERNAL_EXPR_LTE}, {"_internalSchemaAllElemMatchFromIndex", PathAcceptingKeyword::INTERNAL_SCHEMA_ALL_ELEM_MATCH_FROM_INDEX}, {"_internalSchemaBinDataEncryptedType", diff --git a/src/mongo/db/matcher/expression_parser.h b/src/mongo/db/matcher/expression_parser.h index 98ed368d95e..101a7e10332 100644 --- a/src/mongo/db/matcher/expression_parser.h +++ b/src/mongo/db/matcher/expression_parser.h @@ -62,6 +62,10 @@ enum class PathAcceptingKeyword { GREATER_THAN, GREATER_THAN_OR_EQUAL, INTERNAL_EXPR_EQ, + INTERNAL_EXPR_GT, + INTERNAL_EXPR_GTE, + INTERNAL_EXPR_LT, + INTERNAL_EXPR_LTE, INTERNAL_SCHEMA_ALL_ELEM_MATCH_FROM_INDEX, INTERNAL_SCHEMA_BIN_DATA_ENCRYPTED_TYPE, INTERNAL_SCHEMA_BIN_DATA_SUBTYPE, diff --git a/src/mongo/db/matcher/expression_visitor.h b/src/mongo/db/matcher/expression_visitor.h index 237263cd4ba..59ec095ea9a 100644 --- a/src/mongo/db/matcher/expression_visitor.h +++ b/src/mongo/db/matcher/expression_visitor.h @@ -50,6 +50,10 @@ class GeoMatchExpression; class GeoNearMatchExpression; class InMatchExpression; class InternalExprEqMatchExpression; +class InternalExprGTMatchExpression; +class InternalExprGTEMatchExpression; +class InternalExprLTMatchExpression; +class InternalExprLTEMatchExpression; class InternalSchemaAllElemMatchFromIndexMatchExpression; class InternalSchemaAllowedPropertiesMatchExpression; class InternalSchemaBinDataEncryptedTypeExpression; @@ -115,6 +119,12 @@ public: virtual void visit(tree_walker::MaybeConstPtr<IsConst, GeoNearMatchExpression> expr) = 0; virtual void visit(tree_walker::MaybeConstPtr<IsConst, InMatchExpression> expr) = 0; virtual void visit(tree_walker::MaybeConstPtr<IsConst, InternalExprEqMatchExpression> expr) = 0; + virtual void visit(tree_walker::MaybeConstPtr<IsConst, InternalExprGTMatchExpression> expr) = 0; + virtual void visit( + tree_walker::MaybeConstPtr<IsConst, InternalExprGTEMatchExpression> expr) = 0; + virtual void visit(tree_walker::MaybeConstPtr<IsConst, InternalExprLTMatchExpression> expr) = 0; + virtual void visit( + tree_walker::MaybeConstPtr<IsConst, InternalExprLTEMatchExpression> expr) = 0; virtual void visit( tree_walker::MaybeConstPtr<IsConst, InternalSchemaAllElemMatchFromIndexMatchExpression> expr) = 0; diff --git a/src/mongo/db/matcher/rewrite_expr.cpp b/src/mongo/db/matcher/rewrite_expr.cpp index b5b12278d17..27e19d4c5fe 100644 --- a/src/mongo/db/matcher/rewrite_expr.cpp +++ b/src/mongo/db/matcher/rewrite_expr.cpp @@ -33,7 +33,7 @@ #include "mongo/db/matcher/rewrite_expr.h" -#include "mongo/db/matcher/expression_internal_expr_eq.h" +#include "mongo/db/matcher/expression_internal_expr_comparison.h" #include "mongo/db/matcher/expression_leaf.h" #include "mongo/db/matcher/expression_tree.h" #include "mongo/logv2/log.h" @@ -141,6 +141,32 @@ std::unique_ptr<MatchExpression> RewriteExpr::_rewriteComparisonExpression( lhs = dynamic_cast<ExpressionFieldPath*>(operandList[1].get()); rhs = dynamic_cast<ExpressionConstant*>(operandList[0].get()); invariant(lhs && rhs); + + // The MatchExpression is normalized so that the field path expression is on the left. For + // cases like {$gt: [1, "$x"]} where the order of the child expressions matter, we also + // change the comparison operator. + switch (cmpOperator) { + case ExpressionCompare::GT: { + cmpOperator = ExpressionCompare::LT; + break; + } + case ExpressionCompare::GTE: { + cmpOperator = ExpressionCompare::LTE; + break; + } + case ExpressionCompare::LT: { + cmpOperator = ExpressionCompare::GT; + break; + } + case ExpressionCompare::LTE: { + cmpOperator = ExpressionCompare::GTE; + break; + } + case ExpressionCompare::EQ: + break; + default: + MONGO_UNREACHABLE_TASSERT(3994306); + } } // Build argument for ComparisonMatchExpression. @@ -155,20 +181,56 @@ std::unique_ptr<MatchExpression> RewriteExpr::_rewriteComparisonExpression( std::unique_ptr<MatchExpression> RewriteExpr::_buildComparisonMatchExpression( ExpressionCompare::CmpOp comparisonOp, BSONElement fieldAndValue) { - invariant(comparisonOp == ExpressionCompare::EQ); - - auto eqMatchExpr = - std::make_unique<InternalExprEqMatchExpression>(fieldAndValue.fieldName(), fieldAndValue); - eqMatchExpr->setCollator(_collator); + tassert(3994301, + "comparisonOp must be one of the following: $eq, $gt, $gte, $lt, $lte", + comparisonOp == ExpressionCompare::EQ || comparisonOp == ExpressionCompare::GT || + comparisonOp == ExpressionCompare::GTE || comparisonOp == ExpressionCompare::LT || + comparisonOp == ExpressionCompare::LTE); + + std::unique_ptr<MatchExpression> matchExpr; + + switch (comparisonOp) { + case ExpressionCompare::EQ: { + matchExpr = std::make_unique<InternalExprEqMatchExpression>(fieldAndValue.fieldName(), + fieldAndValue); + break; + } + case ExpressionCompare::GT: { + matchExpr = std::make_unique<InternalExprGTMatchExpression>(fieldAndValue.fieldName(), + fieldAndValue); + break; + } + case ExpressionCompare::GTE: { + matchExpr = std::make_unique<InternalExprGTEMatchExpression>(fieldAndValue.fieldName(), + fieldAndValue); + break; + } + case ExpressionCompare::LT: { + matchExpr = std::make_unique<InternalExprLTMatchExpression>(fieldAndValue.fieldName(), + fieldAndValue); + break; + } + case ExpressionCompare::LTE: { + matchExpr = std::make_unique<InternalExprLTEMatchExpression>(fieldAndValue.fieldName(), + fieldAndValue); + break; + } + default: + MONGO_UNREACHABLE_TASSERT(3994307); + } + matchExpr->setCollator(_collator); - return eqMatchExpr; + return matchExpr; } bool RewriteExpr::_canRewriteComparison( const boost::intrusive_ptr<ExpressionCompare>& expression) const { - // Currently we only rewrite $eq expressions. - if (expression->getOp() != ExpressionCompare::EQ) { + // Currently we only rewrite $eq, $gt, $gte, $lt and $lte expressions. + auto op = expression->getOp(); + if (op != ExpressionCompare::EQ && op != ExpressionCompare::GT && + op != ExpressionCompare::GTE && op != ExpressionCompare::LT && + op != ExpressionCompare::LTE) { return false; } diff --git a/src/mongo/db/pipeline/document_source_match.cpp b/src/mongo/db/pipeline/document_source_match.cpp index 9ee3c4ccb67..e64151cb77f 100644 --- a/src/mongo/db/pipeline/document_source_match.cpp +++ b/src/mongo/db/pipeline/document_source_match.cpp @@ -266,6 +266,10 @@ Document redactSafePortionDollarOps(BSONObj expr) { case PathAcceptingKeyword::GEO_INTERSECTS: case PathAcceptingKeyword::GEO_NEAR: case PathAcceptingKeyword::INTERNAL_EXPR_EQ: + case PathAcceptingKeyword::INTERNAL_EXPR_GT: + case PathAcceptingKeyword::INTERNAL_EXPR_GTE: + case PathAcceptingKeyword::INTERNAL_EXPR_LT: + case PathAcceptingKeyword::INTERNAL_EXPR_LTE: case PathAcceptingKeyword::INTERNAL_SCHEMA_ALL_ELEM_MATCH_FROM_INDEX: case PathAcceptingKeyword::INTERNAL_SCHEMA_BIN_DATA_ENCRYPTED_TYPE: case PathAcceptingKeyword::INTERNAL_SCHEMA_BIN_DATA_SUBTYPE: diff --git a/src/mongo/db/query/canonical_query_encoder.cpp b/src/mongo/db/query/canonical_query_encoder.cpp index ce2f70ec0fb..fb73aa3e5c2 100644 --- a/src/mongo/db/query/canonical_query_encoder.cpp +++ b/src/mongo/db/query/canonical_query_encoder.cpp @@ -190,7 +190,19 @@ const char* encodeMatchType(MatchExpression::MatchType mt) { return "xp"; case MatchExpression::INTERNAL_EXPR_EQ: - return "ee"; + return "eeq"; + + case MatchExpression::INTERNAL_EXPR_GT: + return "egt"; + + case MatchExpression::INTERNAL_EXPR_GTE: + return "ege"; + + case MatchExpression::INTERNAL_EXPR_LT: + return "elt"; + + case MatchExpression::INTERNAL_EXPR_LTE: + return "ele"; case MatchExpression::INTERNAL_SCHEMA_ALL_ELEM_MATCH_FROM_INDEX: return "internalSchemaAllElemMatchFromIndex"; diff --git a/src/mongo/db/query/index_bounds_builder.cpp b/src/mongo/db/query/index_bounds_builder.cpp index e423f60844c..aa05e986aed 100644 --- a/src/mongo/db/query/index_bounds_builder.cpp +++ b/src/mongo/db/query/index_bounds_builder.cpp @@ -41,7 +41,7 @@ #include "mongo/db/index/expression_params.h" #include "mongo/db/index/s2_common.h" #include "mongo/db/matcher/expression_geo.h" -#include "mongo/db/matcher/expression_internal_expr_eq.h" +#include "mongo/db/matcher/expression_internal_expr_comparison.h" #include "mongo/db/query/collation/collation_index_key.h" #include "mongo/db/query/collation/collator_interface.h" #include "mongo/db/query/expression_index.h" @@ -626,6 +626,75 @@ void IndexBoundsBuilder::_translatePredicate(const MatchExpression* expr, // There is no need to sort intervals or merge overlapping intervals here since the output // is from one element. translateEquality(node->getData(), index, isHashed, oilOut, tightnessOut); + } else if (MatchExpression::LT == expr->matchType()) { + const LTMatchExpression* node = static_cast<const LTMatchExpression*>(expr); + BSONElement dataElt = node->getData(); + + // Everything is < MaxKey, except for MaxKey. However the bounds need to be inclusive to + // find the array [MaxKey] which is smaller for a comparison but equal in a multikey index. + if (MaxKey == dataElt.type()) { + oilOut->intervals.push_back(allValuesRespectingInclusion( + IndexBounds::makeBoundInclusionFromBoundBools(true, index.multikey))); + *tightnessOut = index.collator || index.multikey ? IndexBoundsBuilder::INEXACT_FETCH + : IndexBoundsBuilder::EXACT; + return; + } + + // Nothing is < NaN. + if (std::isnan(dataElt.numberDouble())) { + *tightnessOut = IndexBoundsBuilder::EXACT; + return; + } + + BSONObjBuilder bob; + buildBoundsForQueryElementForLT(dataElt, index.collator, &bob); + BSONObj dataObj = bob.done().getOwned(); + verify(dataObj.isOwned()); + bool inclusiveBounds = dataElt.type() == BSONType::Array; + Interval interval = + makeRangeInterval(dataObj, + IndexBounds::makeBoundInclusionFromBoundBools( + typeMatch(dataObj) || inclusiveBounds, inclusiveBounds)); + + // If the operand to LT is equal to the lower bound X, the interval [X, X) is invalid + // and should not be added to the bounds. + if (!interval.isNull()) { + oilOut->intervals.push_back(interval); + } + + *tightnessOut = getInequalityPredicateTightness(interval, dataElt, index); + } else if (MatchExpression::INTERNAL_EXPR_LT == expr->matchType()) { + const auto* node = static_cast<const InternalExprLTMatchExpression*>(expr); + BSONElement dataElt = node->getData(); + + // Unlike the regular $lt match expression, $_internalExprLt does not make special checks + // for when dataElt is MaxKey or NaN because it doesn't do type bracketing for any operand. + // Another difference is that $internalExprLt predicates on multikey paths will not use an + // index. + tassert(3994304, + "$expr comparison predicates on multikey paths cannot use an index", + !index.pathHasMultikeyComponent(elt.fieldNameStringData())); + + BSONObjBuilder bob; + bob.appendMinKey(""); + CollationIndexKey::collationAwareIndexKeyAppend(dataElt, index.collator, &bob); + BSONObj dataObj = bob.obj(); + + // Generally all intervals for $_internalExprLt will exclude the end key, however because + // null and missing are conflated in the index but treated as distinct values for + // expressions (with missing ordered as less than null), when dataElt is null we must build + // index bounds [MinKey, null] to include missing values and filter out the literal null + // values with an INEXACT_FETCH. + Interval interval = makeRangeInterval( + dataObj, IndexBounds::makeBoundInclusionFromBoundBools(true, dataElt.isNull())); + + // If the operand to $_internalExprLt is equal to the lower bound X, the interval [X, X) is + // invalid and should not be added to the bounds. Because $_internalExprLt doesn't perform + // type bracketing, here we need to avoid adding the interval [MinKey, MinKey). + if (!interval.isNull()) { + oilOut->intervals.push_back(interval); + } + *tightnessOut = getInequalityPredicateTightness(interval, dataElt, index); } else if (MatchExpression::LTE == expr->matchType()) { const LTEMatchExpression* node = static_cast<const LTEMatchExpression*>(expr); BSONElement dataElt = node->getData(); @@ -664,40 +733,32 @@ void IndexBoundsBuilder::_translatePredicate(const MatchExpression* expr, oilOut->intervals.push_back(interval); *tightnessOut = getInequalityPredicateTightness(interval, dataElt, index); - } else if (MatchExpression::LT == expr->matchType()) { - const LTMatchExpression* node = static_cast<const LTMatchExpression*>(expr); + } else if (MatchExpression::INTERNAL_EXPR_LTE == expr->matchType()) { + const auto* node = static_cast<const InternalExprLTEMatchExpression*>(expr); BSONElement dataElt = node->getData(); - // Everything is < MaxKey, except for MaxKey. However the bounds need to be inclusive to - // find the array [MaxKey] which is smaller for a comparison but equal in a multikey index. - if (MaxKey == dataElt.type()) { - oilOut->intervals.push_back(allValuesRespectingInclusion( - IndexBounds::makeBoundInclusionFromBoundBools(true, index.multikey))); - *tightnessOut = index.collator || index.multikey ? IndexBoundsBuilder::INEXACT_FETCH - : IndexBoundsBuilder::EXACT; - return; - } - - // Nothing is < NaN. - if (std::isnan(dataElt.numberDouble())) { - *tightnessOut = IndexBoundsBuilder::EXACT; - return; - } + // Unlike the regular $lte match expression, $_internalExprLte does not make special checks + // for when dataElt is MaxKey or NaN because it doesn't do type bracketing for any operand. + // Another difference is that $internalExprLte predicates on multikey paths will not use an + // index. + tassert(3994305, + "$expr comparison predicates on multikey paths cannot use an index", + !index.pathHasMultikeyComponent(elt.fieldNameStringData())); BSONObjBuilder bob; - buildBoundsForQueryElementForLT(dataElt, index.collator, &bob); - BSONObj dataObj = bob.done().getOwned(); - verify(dataObj.isOwned()); - bool inclusiveBounds = dataElt.type() == BSONType::Array; - Interval interval = - makeRangeInterval(dataObj, - IndexBounds::makeBoundInclusionFromBoundBools( - typeMatch(dataObj) || inclusiveBounds, inclusiveBounds)); + bob.appendMinKey(""); + CollationIndexKey::collationAwareIndexKeyAppend(dataElt, index.collator, &bob); + BSONObj dataObj = bob.obj(); - // If the operand to LT is equal to the lower bound X, the interval [X, X) is invalid - // and should not be added to the bounds. - if (!interval.isNull()) { - oilOut->intervals.push_back(interval); + Interval interval = makeRangeInterval(dataObj, BoundInclusion::kIncludeBothStartAndEndKeys); + oilOut->intervals.push_back(interval); + + // Expressions treat null and missing as distinct values, with missing ordered as less than + // null. Thus for $_internalExprLte when dataElt is null we can treat the bounds as EXACT, + // since both null and missing values should be included. + if (dataElt.isNull()) { + *tightnessOut = IndexBoundsBuilder::EXACT; + return; } *tightnessOut = getInequalityPredicateTightness(interval, dataElt, index); @@ -737,6 +798,41 @@ void IndexBoundsBuilder::_translatePredicate(const MatchExpression* expr, oilOut->intervals.push_back(interval); } *tightnessOut = getInequalityPredicateTightness(interval, dataElt, index); + } else if (MatchExpression::INTERNAL_EXPR_GT == expr->matchType()) { + const auto* node = static_cast<const InternalExprGTMatchExpression*>(expr); + BSONElement dataElt = node->getData(); + + // Unlike the regular $gt match expression, $_internalExprGt does not make special checks + // for when dataElt is MinKey or NaN because it doesn't do type bracketing for any operand. + // Another difference is that $internalExprGt predicates on multikey paths will not use an + // index. + tassert(3994302, + "$expr comparison predicates on multikey paths cannot use an index", + !index.pathHasMultikeyComponent(elt.fieldNameStringData())); + + BSONObjBuilder bob; + CollationIndexKey::collationAwareIndexKeyAppend(dataElt, index.collator, &bob); + bob.appendMaxKey(""); + BSONObj dataObj = bob.obj(); + + Interval interval = makeRangeInterval(dataObj, BoundInclusion::kIncludeEndKeyOnly); + + // If the operand to $_internalExprGt is equal to the upper bound X, the interval (X, X] is + // invalid and should not be added to the bounds. Because $_internalExprGt doesn't perform + // type bracketing, here we need to avoid adding the interval (MaxKey, MaxKey]. + if (!interval.isNull()) { + oilOut->intervals.push_back(interval); + } + + // Expressions treat null and missing as distinct values, with missing ordered as less than + // null. Thus for $_internalExprGt when dataElt is null we can treat the bounds as EXACT, + // since both null and missing values should be excluded. + if (dataElt.isNull()) { + *tightnessOut = IndexBoundsBuilder::EXACT; + return; + } + + *tightnessOut = getInequalityPredicateTightness(interval, dataElt, index); } else if (MatchExpression::GTE == expr->matchType()) { const GTEMatchExpression* node = static_cast<const GTEMatchExpression*>(expr); BSONElement dataElt = node->getData(); @@ -773,6 +869,26 @@ void IndexBoundsBuilder::_translatePredicate(const MatchExpression* expr, oilOut->intervals.push_back(interval); *tightnessOut = getInequalityPredicateTightness(interval, dataElt, index); + } else if (MatchExpression::INTERNAL_EXPR_GTE == expr->matchType()) { + const auto* node = static_cast<const InternalExprGTEMatchExpression*>(expr); + BSONElement dataElt = node->getData(); + + // Unlike the regular $gte match expression, $_internalExprGte does not make special checks + // for when dataElt is MinKey or NaN because it doesn't do type bracketing for any operand. + // Another difference is that $internalExprGte predicates on multikey paths will not use an + // index. + tassert(3994303, + "$expr comparison predicates on multikey paths cannot use an index", + !index.pathHasMultikeyComponent(elt.fieldNameStringData())); + + BSONObjBuilder bob; + CollationIndexKey::collationAwareIndexKeyAppend(dataElt, index.collator, &bob); + bob.appendMaxKey(""); + BSONObj dataObj = bob.obj(); + + Interval interval = makeRangeInterval(dataObj, BoundInclusion::kIncludeBothStartAndEndKeys); + oilOut->intervals.push_back(interval); + *tightnessOut = getInequalityPredicateTightness(interval, dataElt, index); } else if (MatchExpression::REGEX == expr->matchType()) { const RegexMatchExpression* rme = static_cast<const RegexMatchExpression*>(expr); translateRegex(rme, index, oilOut, tightnessOut); diff --git a/src/mongo/db/query/index_bounds_builder_test.cpp b/src/mongo/db/query/index_bounds_builder_test.cpp index 4c141b52ee2..8413c471214 100644 --- a/src/mongo/db/query/index_bounds_builder_test.cpp +++ b/src/mongo/db/query/index_bounds_builder_test.cpp @@ -38,6 +38,7 @@ #include "mongo/db/matcher/expression_parser.h" #include "mongo/db/query/collation/collator_interface_mock.h" #include "mongo/db/query/expression_index.h" +#include "mongo/unittest/death_test.h" #include "mongo/unittest/unittest.h" namespace { @@ -1358,4 +1359,387 @@ TEST_F(IndexBoundsBuilderTest, IntersectizeBasic) { ASSERT_TRUE(oil2 == expectedIntersection); } +TEST_F(IndexBoundsBuilderTest, TranslateInternalExprGT) { + BSONObj keyPattern = BSON("a" << 1); + BSONElement elt = keyPattern.firstElement(); + auto testIndex = buildSimpleIndexEntry(keyPattern); + BSONObj obj = BSON("a" << BSON("$_internalExprGt" << 4)); + auto expr = parseMatchExpression(obj); + OrderedIntervalList oil; + IndexBoundsBuilder::BoundsTightness tightness; + IndexBoundsBuilder::translate(expr.get(), elt, testIndex, &oil, &tightness); + ASSERT_EQUALS(oil.name, "a"); + ASSERT_EQUALS(oil.intervals.size(), 1U); + ASSERT_EQUALS(Interval::INTERVAL_EQUALS, + oil.intervals[0].compare(Interval(maxKeyIntObj(4), false, true))); + ASSERT_EQUALS(tightness, IndexBoundsBuilder::EXACT); +} + +TEST_F(IndexBoundsBuilderTest, TranslateInternalExprGTNull) { + BSONObj keyPattern = BSON("a" << 1); + BSONElement elt = keyPattern.firstElement(); + auto testIndex = buildSimpleIndexEntry(keyPattern); + BSONObj obj = BSON("a" << BSON("$_internalExprGt" << BSONNULL)); + auto expr = parseMatchExpression(obj); + OrderedIntervalList oil; + IndexBoundsBuilder::BoundsTightness tightness; + IndexBoundsBuilder::translate(expr.get(), elt, testIndex, &oil, &tightness); + ASSERT_EQUALS(oil.name, "a"); + ASSERT_EQUALS(oil.intervals.size(), 1U); + ASSERT_EQUALS(oil.intervals[0].toString(), "(null, MaxKey]"); + ASSERT_FALSE(oil.intervals[0].startInclusive); + ASSERT_TRUE(oil.intervals[0].endInclusive); + ASSERT_EQUALS(tightness, IndexBoundsBuilder::EXACT); +} + +TEST_F(IndexBoundsBuilderTest, TranslateInternalExprGTMaxKeyDoesNotGenerateBounds) { + BSONObj keyPattern = BSON("a" << 1); + BSONElement elt = keyPattern.firstElement(); + auto testIndex = buildSimpleIndexEntry(keyPattern); + BSONObj obj = BSON("a" << BSON("$_internalExprGt" << MAXKEY)); + auto expr = parseMatchExpression(obj); + OrderedIntervalList oil; + IndexBoundsBuilder::BoundsTightness tightness; + IndexBoundsBuilder::translate(expr.get(), elt, testIndex, &oil, &tightness); + ASSERT_EQUALS(oil.name, "a"); + ASSERT_EQUALS(oil.intervals.size(), 0U); + ASSERT_EQUALS(tightness, IndexBoundsBuilder::EXACT); +} + +DEATH_TEST_F(IndexBoundsBuilderTest, + TranslateInternalExprGTMultikeyPathFails, + "$expr comparison predicates on multikey paths cannot use an index") { + BSONObj keyPattern = BSON("a" << 1 << "b" << 1); + BSONElement elt = keyPattern.firstElement(); + auto testIndex = buildMultikeyIndexEntry(keyPattern, {{0U}, {}}); + BSONObj obj = BSON("a" << BSON("$_internalExprGt" << 4)); + auto expr = parseMatchExpression(obj); + OrderedIntervalList oil; + IndexBoundsBuilder::BoundsTightness tightness; + IndexBoundsBuilder::translate(expr.get(), elt, testIndex, &oil, &tightness); +} + +TEST_F(IndexBoundsBuilderTest, TranslateInternalExprGTNonMultikeyPathOnMultikeyIndexSucceeds) { + BSONObj keyPattern = BSON("a" << 1 << "b" << 1); + BSONElement elt = keyPattern.firstElement(); + auto testIndex = buildMultikeyIndexEntry(keyPattern, {{}, {0U}}); + BSONObj obj = BSON("a" << BSON("$_internalExprGt" << 4)); + auto expr = parseMatchExpression(obj); + OrderedIntervalList oil; + IndexBoundsBuilder::BoundsTightness tightness; + IndexBoundsBuilder::translate(expr.get(), elt, testIndex, &oil, &tightness); + ASSERT_EQUALS(oil.name, "a"); + ASSERT_EQUALS(oil.intervals.size(), 1U); + ASSERT_EQUALS(Interval::INTERVAL_EQUALS, + oil.intervals[0].compare(Interval(maxKeyIntObj(4), false, true))); + ASSERT_EQUALS(tightness, IndexBoundsBuilder::EXACT); +} + +TEST_F(IndexBoundsBuilderTest, TranslateInternalExprGTSubObjectContainingBadValuesSucceeds) { + BSONObj keyPattern = BSON("_id" << 1); + BSONElement elt = keyPattern.firstElement(); + auto testIndex = buildSimpleIndexEntry(keyPattern); + BSONObj subObj = BSON("subObj" << BSON("a" << BSONUndefined << "b" << BSON_ARRAY("array"))); + BSONObj obj = BSON("_id" << BSON("$_internalExprGt" << subObj)); + auto expr = parseMatchExpression(obj); + OrderedIntervalList oil; + IndexBoundsBuilder::BoundsTightness tightness; + IndexBoundsBuilder::translate(expr.get(), elt, testIndex, &oil, &tightness); + ASSERT_EQUALS(oil.name, "_id"); + ASSERT_EQUALS(oil.intervals.size(), 1U); + ASSERT_EQUALS( + Interval::INTERVAL_EQUALS, + oil.intervals[0].compare(Interval(BSON("" << subObj << "" << MAXKEY), false, true))); + ASSERT_EQUALS(tightness, IndexBoundsBuilder::EXACT); +} + +TEST_F(IndexBoundsBuilderTest, TranslateInternalExprGTE) { + BSONObj keyPattern = BSON("a" << 1); + BSONElement elt = keyPattern.firstElement(); + auto testIndex = buildSimpleIndexEntry(keyPattern); + BSONObj obj = BSON("a" << BSON("$_internalExprGte" << 4)); + auto expr = parseMatchExpression(obj); + OrderedIntervalList oil; + IndexBoundsBuilder::BoundsTightness tightness; + IndexBoundsBuilder::translate(expr.get(), elt, testIndex, &oil, &tightness); + ASSERT_EQUALS(oil.name, "a"); + ASSERT_EQUALS(oil.intervals.size(), 1U); + ASSERT_EQUALS(Interval::INTERVAL_EQUALS, + oil.intervals[0].compare(Interval(maxKeyIntObj(4), true, true))); + ASSERT_EQUALS(tightness, IndexBoundsBuilder::EXACT); +} + +TEST_F(IndexBoundsBuilderTest, TranslateInternalExprGTENull) { + BSONObj keyPattern = BSON("a" << 1); + BSONElement elt = keyPattern.firstElement(); + auto testIndex = buildSimpleIndexEntry(keyPattern); + BSONObj obj = BSON("a" << BSON("$_internalExprGte" << BSONNULL)); + auto expr = parseMatchExpression(obj); + OrderedIntervalList oil; + IndexBoundsBuilder::BoundsTightness tightness; + IndexBoundsBuilder::translate(expr.get(), elt, testIndex, &oil, &tightness); + ASSERT_EQUALS(oil.name, "a"); + ASSERT_EQUALS(oil.intervals.size(), 1U); + ASSERT_EQUALS(oil.intervals[0].toString(), "[null, MaxKey]"); + ASSERT_TRUE(oil.intervals[0].startInclusive); + ASSERT_TRUE(oil.intervals[0].endInclusive); + ASSERT_EQUALS(tightness, IndexBoundsBuilder::INEXACT_FETCH); +} + +TEST_F(IndexBoundsBuilderTest, TranslateInternalExprGTEMaxKeyGeneratesBounds) { + BSONObj keyPattern = BSON("a" << 1); + BSONElement elt = keyPattern.firstElement(); + auto testIndex = buildSimpleIndexEntry(keyPattern); + BSONObj obj = BSON("a" << BSON("$_internalExprGte" << MAXKEY)); + auto expr = parseMatchExpression(obj); + OrderedIntervalList oil; + IndexBoundsBuilder::BoundsTightness tightness; + IndexBoundsBuilder::translate(expr.get(), elt, testIndex, &oil, &tightness); + ASSERT_EQUALS(oil.name, "a"); + ASSERT_EQUALS(oil.intervals.size(), 1U); + ASSERT_EQUALS(oil.intervals[0].toString(), "[MaxKey, MaxKey]"); + ASSERT_TRUE(oil.intervals[0].startInclusive); + ASSERT_TRUE(oil.intervals[0].endInclusive); + ASSERT_EQUALS(tightness, IndexBoundsBuilder::EXACT); +} + +DEATH_TEST_F(IndexBoundsBuilderTest, + TranslateInternalExprGTEMultikeyPathFails, + "$expr comparison predicates on multikey paths cannot use an index") { + BSONObj keyPattern = BSON("a" << 1 << "b" << 1); + BSONElement elt = keyPattern.firstElement(); + auto testIndex = buildMultikeyIndexEntry(keyPattern, {{0U}, {}}); + BSONObj obj = BSON("a" << BSON("$_internalExprGte" << 4)); + auto expr = parseMatchExpression(obj); + OrderedIntervalList oil; + IndexBoundsBuilder::BoundsTightness tightness; + IndexBoundsBuilder::translate(expr.get(), elt, testIndex, &oil, &tightness); +} + +TEST_F(IndexBoundsBuilderTest, TranslateInternalExprGTENonMultikeyPathOnMultikeyIndexSucceeds) { + BSONObj keyPattern = BSON("a" << 1 << "b" << 1); + BSONElement elt = keyPattern.firstElement(); + auto testIndex = buildMultikeyIndexEntry(keyPattern, {{}, {0U}}); + BSONObj obj = BSON("a" << BSON("$_internalExprGte" << 4)); + auto expr = parseMatchExpression(obj); + OrderedIntervalList oil; + IndexBoundsBuilder::BoundsTightness tightness; + IndexBoundsBuilder::translate(expr.get(), elt, testIndex, &oil, &tightness); + ASSERT_EQUALS(oil.name, "a"); + ASSERT_EQUALS(oil.intervals.size(), 1U); + ASSERT_EQUALS(Interval::INTERVAL_EQUALS, + oil.intervals[0].compare(Interval(maxKeyIntObj(4), true, true))); + ASSERT_EQUALS(tightness, IndexBoundsBuilder::EXACT); +} + +TEST_F(IndexBoundsBuilderTest, TranslateInternalExprGTESubObjectContainingBadValuesSucceeds) { + BSONObj keyPattern = BSON("_id" << 1); + BSONElement elt = keyPattern.firstElement(); + auto testIndex = buildSimpleIndexEntry(keyPattern); + BSONObj subObj = BSON("subObj" << BSON("a" << BSONUndefined << "b" << BSON_ARRAY("array"))); + BSONObj obj = BSON("_id" << BSON("$_internalExprGte" << subObj)); + auto expr = parseMatchExpression(obj); + OrderedIntervalList oil; + IndexBoundsBuilder::BoundsTightness tightness; + IndexBoundsBuilder::translate(expr.get(), elt, testIndex, &oil, &tightness); + ASSERT_EQUALS(oil.name, "_id"); + ASSERT_EQUALS(oil.intervals.size(), 1U); + ASSERT_EQUALS( + Interval::INTERVAL_EQUALS, + oil.intervals[0].compare(Interval(BSON("" << subObj << "" << MAXKEY), true, true))); + ASSERT_EQUALS(tightness, IndexBoundsBuilder::EXACT); +} + +TEST_F(IndexBoundsBuilderTest, TranslateInternalExprLT) { + BSONObj keyPattern = BSON("a" << 1); + BSONElement elt = keyPattern.firstElement(); + auto testIndex = buildSimpleIndexEntry(keyPattern); + BSONObj obj = BSON("a" << BSON("$_internalExprLt" << 4)); + auto expr = parseMatchExpression(obj); + OrderedIntervalList oil; + IndexBoundsBuilder::BoundsTightness tightness; + IndexBoundsBuilder::translate(expr.get(), elt, testIndex, &oil, &tightness); + ASSERT_EQUALS(oil.name, "a"); + ASSERT_EQUALS(oil.intervals.size(), 1U); + ASSERT_EQUALS(Interval::INTERVAL_EQUALS, + oil.intervals[0].compare(Interval(minKeyIntObj(4), true, false))); + ASSERT_EQUALS(tightness, IndexBoundsBuilder::EXACT); +} + +TEST_F(IndexBoundsBuilderTest, TranslateInternalExprLTNull) { + BSONObj keyPattern = BSON("a" << 1); + BSONElement elt = keyPattern.firstElement(); + auto testIndex = buildSimpleIndexEntry(keyPattern); + BSONObj obj = BSON("a" << BSON("$_internalExprLt" << BSONNULL)); + auto expr = parseMatchExpression(obj); + OrderedIntervalList oil; + IndexBoundsBuilder::BoundsTightness tightness; + IndexBoundsBuilder::translate(expr.get(), elt, testIndex, &oil, &tightness); + ASSERT_EQUALS(oil.name, "a"); + ASSERT_EQUALS(oil.intervals.size(), 1U); + ASSERT_EQUALS(oil.intervals[0].toString(), "[MinKey, null]"); + ASSERT_TRUE(oil.intervals[0].startInclusive); + ASSERT_TRUE(oil.intervals[0].endInclusive); + ASSERT_EQUALS(tightness, IndexBoundsBuilder::INEXACT_FETCH); +} + +TEST_F(IndexBoundsBuilderTest, TranslateInternalExprLTMinKeyDoesNotGenerateBounds) { + BSONObj keyPattern = BSON("a" << 1); + BSONElement elt = keyPattern.firstElement(); + auto testIndex = buildSimpleIndexEntry(keyPattern); + BSONObj obj = BSON("a" << BSON("$_internalExprLt" << MINKEY)); + auto expr = parseMatchExpression(obj); + OrderedIntervalList oil; + IndexBoundsBuilder::BoundsTightness tightness; + IndexBoundsBuilder::translate(expr.get(), elt, testIndex, &oil, &tightness); + ASSERT_EQUALS(oil.name, "a"); + ASSERT_EQUALS(oil.intervals.size(), 0U); + ASSERT_EQUALS(tightness, IndexBoundsBuilder::EXACT); +} + +DEATH_TEST_F(IndexBoundsBuilderTest, + TranslateInternalExprLTMultikeyPathFails, + "$expr comparison predicates on multikey paths cannot use an index") { + BSONObj keyPattern = BSON("a" << 1 << "b" << 1); + BSONElement elt = keyPattern.firstElement(); + auto testIndex = buildMultikeyIndexEntry(keyPattern, {{0U}, {}}); + BSONObj obj = BSON("a" << BSON("$_internalExprLt" << 4)); + auto expr = parseMatchExpression(obj); + OrderedIntervalList oil; + IndexBoundsBuilder::BoundsTightness tightness; + IndexBoundsBuilder::translate(expr.get(), elt, testIndex, &oil, &tightness); +} + +TEST_F(IndexBoundsBuilderTest, TranslateInternalExprLTNonMultikeyPathOnMultikeyIndexSucceeds) { + BSONObj keyPattern = BSON("a" << 1 << "b" << 1); + BSONElement elt = keyPattern.firstElement(); + auto testIndex = buildMultikeyIndexEntry(keyPattern, {{}, {0U}}); + BSONObj obj = BSON("a" << BSON("$_internalExprLt" << 4)); + auto expr = parseMatchExpression(obj); + OrderedIntervalList oil; + IndexBoundsBuilder::BoundsTightness tightness; + IndexBoundsBuilder::translate(expr.get(), elt, testIndex, &oil, &tightness); + ASSERT_EQUALS(oil.name, "a"); + ASSERT_EQUALS(oil.intervals.size(), 1U); + ASSERT_EQUALS(Interval::INTERVAL_EQUALS, + oil.intervals[0].compare(Interval(minKeyIntObj(4), true, false))); + ASSERT_EQUALS(tightness, IndexBoundsBuilder::EXACT); +} + +TEST_F(IndexBoundsBuilderTest, TranslateInternalExprLTSubObjectContainingBadValuesSucceeds) { + BSONObj keyPattern = BSON("_id" << 1); + BSONElement elt = keyPattern.firstElement(); + auto testIndex = buildSimpleIndexEntry(keyPattern); + BSONObj subObj = BSON("subObj" << BSON("a" << BSONUndefined << "b" << BSON_ARRAY("array"))); + BSONObj obj = BSON("_id" << BSON("$_internalExprLt" << subObj)); + auto expr = parseMatchExpression(obj); + OrderedIntervalList oil; + IndexBoundsBuilder::BoundsTightness tightness; + IndexBoundsBuilder::translate(expr.get(), elt, testIndex, &oil, &tightness); + ASSERT_EQUALS(oil.name, "_id"); + ASSERT_EQUALS(oil.intervals.size(), 1U); + ASSERT_EQUALS( + Interval::INTERVAL_EQUALS, + oil.intervals[0].compare(Interval(BSON("" << MINKEY << "" << subObj), true, false))); + ASSERT_EQUALS(tightness, IndexBoundsBuilder::EXACT); +} + +TEST_F(IndexBoundsBuilderTest, TranslateInternalExprLTE) { + BSONObj keyPattern = BSON("a" << 1); + BSONElement elt = keyPattern.firstElement(); + auto testIndex = buildSimpleIndexEntry(keyPattern); + BSONObj obj = BSON("a" << BSON("$_internalExprLte" << 4)); + auto expr = parseMatchExpression(obj); + OrderedIntervalList oil; + IndexBoundsBuilder::BoundsTightness tightness; + IndexBoundsBuilder::translate(expr.get(), elt, testIndex, &oil, &tightness); + ASSERT_EQUALS(oil.name, "a"); + ASSERT_EQUALS(oil.intervals.size(), 1U); + ASSERT_EQUALS(Interval::INTERVAL_EQUALS, + oil.intervals[0].compare(Interval(minKeyIntObj(4), true, true))); + ASSERT_EQUALS(tightness, IndexBoundsBuilder::EXACT); +} + +TEST_F(IndexBoundsBuilderTest, TranslateInternalExprLTENull) { + BSONObj keyPattern = BSON("a" << 1); + BSONElement elt = keyPattern.firstElement(); + auto testIndex = buildSimpleIndexEntry(keyPattern); + BSONObj obj = BSON("a" << BSON("$_internalExprLte" << BSONNULL)); + auto expr = parseMatchExpression(obj); + OrderedIntervalList oil; + IndexBoundsBuilder::BoundsTightness tightness; + IndexBoundsBuilder::translate(expr.get(), elt, testIndex, &oil, &tightness); + ASSERT_EQUALS(oil.name, "a"); + ASSERT_EQUALS(oil.intervals.size(), 1U); + ASSERT_EQUALS(oil.intervals[0].toString(), "[MinKey, null]"); + ASSERT_TRUE(oil.intervals[0].startInclusive); + ASSERT_TRUE(oil.intervals[0].endInclusive); + ASSERT_EQUALS(tightness, IndexBoundsBuilder::EXACT); +} + +TEST_F(IndexBoundsBuilderTest, TranslateInternalExprLTEMinKeyGeneratesBounds) { + BSONObj keyPattern = BSON("a" << 1); + BSONElement elt = keyPattern.firstElement(); + auto testIndex = buildSimpleIndexEntry(keyPattern); + BSONObj obj = BSON("a" << BSON("$_internalExprLte" << MINKEY)); + auto expr = parseMatchExpression(obj); + OrderedIntervalList oil; + IndexBoundsBuilder::BoundsTightness tightness; + IndexBoundsBuilder::translate(expr.get(), elt, testIndex, &oil, &tightness); + ASSERT_EQUALS(oil.name, "a"); + ASSERT_EQUALS(oil.intervals.size(), 1U); + ASSERT_EQUALS(oil.intervals[0].toString(), "[MinKey, MinKey]"); + ASSERT_TRUE(oil.intervals[0].startInclusive); + ASSERT_TRUE(oil.intervals[0].endInclusive); + ASSERT_EQUALS(tightness, IndexBoundsBuilder::EXACT); +} + +DEATH_TEST_F(IndexBoundsBuilderTest, + TranslateInternalExprLTEMultikeyPathFails, + "$expr comparison predicates on multikey paths cannot use an index") { + BSONObj keyPattern = BSON("a" << 1 << "b" << 1); + BSONElement elt = keyPattern.firstElement(); + auto testIndex = buildMultikeyIndexEntry(keyPattern, {{0U}, {}}); + + BSONObj obj = BSON("a" << BSON("$_internalExprLte" << 4)); + auto expr = parseMatchExpression(obj); + OrderedIntervalList oil; + IndexBoundsBuilder::BoundsTightness tightness; + IndexBoundsBuilder::translate(expr.get(), elt, testIndex, &oil, &tightness); +} + +TEST_F(IndexBoundsBuilderTest, TranslateInternalExprLTENonMultikeyPathOnMultikeyIndexSucceeds) { + BSONObj keyPattern = BSON("a" << 1 << "b" << 1); + BSONElement elt = keyPattern.firstElement(); + auto testIndex = buildMultikeyIndexEntry(keyPattern, {{}, {0U}}); + BSONObj obj = BSON("a" << BSON("$_internalExprLte" << 4)); + auto expr = parseMatchExpression(obj); + OrderedIntervalList oil; + IndexBoundsBuilder::BoundsTightness tightness; + IndexBoundsBuilder::translate(expr.get(), elt, testIndex, &oil, &tightness); + ASSERT_EQUALS(oil.name, "a"); + ASSERT_EQUALS(oil.intervals.size(), 1U); + ASSERT_EQUALS(Interval::INTERVAL_EQUALS, + oil.intervals[0].compare(Interval(minKeyIntObj(4), true, true))); + ASSERT_EQUALS(tightness, IndexBoundsBuilder::EXACT); +} + +TEST_F(IndexBoundsBuilderTest, TranslateInternalExprLTESubObjectContainingBadValuesSucceeds) { + BSONObj keyPattern = BSON("_id" << 1); + BSONElement elt = keyPattern.firstElement(); + auto testIndex = buildSimpleIndexEntry(keyPattern); + BSONObj subObj = BSON("subObj" << BSON("a" << BSONUndefined << "b" << BSON_ARRAY("array"))); + BSONObj obj = BSON("_id" << BSON("$_internalExprLte" << subObj)); + auto expr = parseMatchExpression(obj); + OrderedIntervalList oil; + IndexBoundsBuilder::BoundsTightness tightness; + IndexBoundsBuilder::translate(expr.get(), elt, testIndex, &oil, &tightness); + ASSERT_EQUALS(oil.name, "_id"); + ASSERT_EQUALS(oil.intervals.size(), 1U); + ASSERT_EQUALS( + Interval::INTERVAL_EQUALS, + oil.intervals[0].compare(Interval(BSON("" << MINKEY << "" << subObj), true, true))); + ASSERT_EQUALS(tightness, IndexBoundsBuilder::EXACT); +} + } // namespace diff --git a/src/mongo/db/query/index_bounds_builder_test.h b/src/mongo/db/query/index_bounds_builder_test.h index f3abf8c212c..421794fecf4 100644 --- a/src/mongo/db/query/index_bounds_builder_test.h +++ b/src/mongo/db/query/index_bounds_builder_test.h @@ -69,6 +69,25 @@ public: } /** + * Make a multikey IndexEntry with the provided key pattern and multikey paths. + */ + IndexEntry buildMultikeyIndexEntry(const BSONObj& kp, const MultikeyPaths& mkp) { + return {kp, + IndexNames::nameToType(IndexNames::findPluginName(kp)), + IndexDescriptor::kLatestIndexVersion, + true, // multikey + mkp, // multikey paths + {}, + false, + false, + IndexEntry::Identifier{"test_multikey"}, + nullptr, + {}, + nullptr, + nullptr}; + } + + /** * Given a list of queries in 'toUnion', translate into index bounds and return * the union of these bounds in the out-parameter 'oilOut'. */ diff --git a/src/mongo/db/query/indexability.h b/src/mongo/db/query/indexability.h index 7442e251c3b..96930df7650 100644 --- a/src/mongo/db/query/indexability.h +++ b/src/mongo/db/query/indexability.h @@ -176,7 +176,11 @@ private: me->matchType() == MatchExpression::GEO_NEAR || me->matchType() == MatchExpression::EXISTS || me->matchType() == MatchExpression::TEXT || - me->matchType() == MatchExpression::INTERNAL_EXPR_EQ; + me->matchType() == MatchExpression::INTERNAL_EXPR_EQ || + me->matchType() == MatchExpression::INTERNAL_EXPR_GT || + me->matchType() == MatchExpression::INTERNAL_EXPR_GTE || + me->matchType() == MatchExpression::INTERNAL_EXPR_LT || + me->matchType() == MatchExpression::INTERNAL_EXPR_LTE; } }; diff --git a/src/mongo/db/query/plan_cache_indexability.cpp b/src/mongo/db/query/plan_cache_indexability.cpp index 216b714dbed..dbf0250966c 100644 --- a/src/mongo/db/query/plan_cache_indexability.cpp +++ b/src/mongo/db/query/plan_cache_indexability.cpp @@ -39,7 +39,7 @@ #include "mongo/db/index/wildcard_key_generator.h" #include "mongo/db/matcher/expression.h" #include "mongo/db/matcher/expression_algo.h" -#include "mongo/db/matcher/expression_internal_expr_eq.h" +#include "mongo/db/matcher/expression_internal_expr_comparison.h" #include "mongo/db/matcher/expression_leaf.h" #include "mongo/db/query/collation/collation_index_key.h" #include "mongo/db/query/collation/collator_interface.h" diff --git a/src/mongo/db/query/planner_ixselect.cpp b/src/mongo/db/query/planner_ixselect.cpp index 9e7fc493d77..d9cc7855d19 100644 --- a/src/mongo/db/query/planner_ixselect.cpp +++ b/src/mongo/db/query/planner_ixselect.cpp @@ -40,7 +40,7 @@ #include "mongo/db/index_names.h" #include "mongo/db/matcher/expression_algo.h" #include "mongo/db/matcher/expression_geo.h" -#include "mongo/db/matcher/expression_internal_expr_eq.h" +#include "mongo/db/matcher/expression_internal_expr_comparison.h" #include "mongo/db/matcher/expression_text.h" #include "mongo/db/query/canonical_query_encoder.h" #include "mongo/db/query/collation/collator_interface.h" @@ -383,9 +383,10 @@ bool QueryPlannerIXSelect::_compatible(const BSONElement& keyPatternElt, // We know keyPatternElt.fieldname() == node->path(). MatchExpression::MatchType exprtype = node->matchType(); - if (exprtype == MatchExpression::INTERNAL_EXPR_EQ && + if (ComparisonMatchExpressionBase::isInternalExprComparison(exprtype) && index.pathHasMultikeyComponent(keyPatternElt.fieldNameStringData())) { - // Expression language equality cannot be indexed if the field path has multikey components. + // Expression language comparisons cannot be indexed if the field path has multikey + // components. return false; } diff --git a/src/mongo/db/query/sbe_stage_builder_filter.cpp b/src/mongo/db/query/sbe_stage_builder_filter.cpp index 5b3e6672a15..b33b48efb6b 100644 --- a/src/mongo/db/query/sbe_stage_builder_filter.cpp +++ b/src/mongo/db/query/sbe_stage_builder_filter.cpp @@ -43,7 +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_internal_expr_comparison.h" #include "mongo/db/matcher/expression_leaf.h" #include "mongo/db/matcher/expression_text.h" #include "mongo/db/matcher/expression_text_noop.h" @@ -700,6 +700,10 @@ public: } void visit(const InMatchExpression* expr) final {} void visit(const InternalExprEqMatchExpression* expr) final {} + void visit(const InternalExprGTMatchExpression* expr) final {} + void visit(const InternalExprGTEMatchExpression* expr) final {} + void visit(const InternalExprLTMatchExpression* expr) final {} + void visit(const InternalExprLTEMatchExpression* expr) final {} void visit(const InternalSchemaAllElemMatchFromIndexMatchExpression* expr) final { unsupportedExpression(expr); } @@ -1185,13 +1189,24 @@ public: LeafTraversalMode::kArrayAndItsElements); } } - + // The following are no-ops. The internal expr comparison match expression are produced + // internally by rewriting an $expr expression to an AND($expr, $_internalExpr[OP]), which can + // later be eliminated by via a conversion into EXACT index bounds, or remains present. In the + // latter case we can simply ignore it, as the result of AND($expr, $_internalExpr[OP]) is equal + // to just $expr. void visit(const InternalExprEqMatchExpression* expr) final { - // This is a no-op. The $_internalExprEq match expression is produced internally by - // rewriting an $expr expression to an AND($expr, $_internalExprEq), which can later be - // eliminated by via a conversion into EXACT index bounds, or remains present. In the latter - // case we can simply ignore it, as the result of AND($expr, $_internalExprEq) is equal to - // just $expr. + generateAlwaysBoolean(_context, true); + } + void visit(const InternalExprGTMatchExpression* expr) final { + generateAlwaysBoolean(_context, true); + } + void visit(const InternalExprGTEMatchExpression* expr) final { + generateAlwaysBoolean(_context, true); + } + void visit(const InternalExprLTMatchExpression* expr) final { + generateAlwaysBoolean(_context, true); + } + void visit(const InternalExprLTEMatchExpression* expr) final { generateAlwaysBoolean(_context, true); } @@ -1388,6 +1403,10 @@ public: void visit(const GeoNearMatchExpression* expr) final {} void visit(const InMatchExpression* expr) final {} void visit(const InternalExprEqMatchExpression* expr) final {} + void visit(const InternalExprGTMatchExpression* expr) final {} + void visit(const InternalExprGTEMatchExpression* expr) final {} + void visit(const InternalExprLTMatchExpression* expr) final {} + void visit(const InternalExprLTEMatchExpression* expr) final {} void visit(const InternalSchemaAllElemMatchFromIndexMatchExpression* expr) final {} void visit(const InternalSchemaAllowedPropertiesMatchExpression* expr) final {} void visit(const InternalSchemaBinDataEncryptedTypeExpression* expr) final {} |