From 3c0df34e8a6a4c8f68428c2fbfb965f0b2e42e64 Mon Sep 17 00:00:00 2001 From: Alexander Ignatyev Date: Tue, 26 Apr 2022 18:01:18 +0000 Subject: [PATCH] SERVER-65793 Do not parameterize queries with TEXT match expression node (cherry picked from commit 9ca1d39e1ad5317bf3e8dcbef937f165f991d74c) --- src/mongo/db/query/canonical_query.cpp | 18 +++-- .../db/query/canonical_query_encoder.cpp | 71 +++++++++++++------ src/mongo/db/query/canonical_query_test.cpp | 14 ++++ 3 files changed, 78 insertions(+), 25 deletions(-) diff --git a/src/mongo/db/query/canonical_query.cpp b/src/mongo/db/query/canonical_query.cpp index 7c4830be8ca..f3d5c53c483 100644 --- a/src/mongo/db/query/canonical_query.cpp +++ b/src/mongo/db/query/canonical_query.cpp @@ -47,6 +47,7 @@ #include "mongo/db/query/indexability.h" #include "mongo/db/query/projection_parser.h" #include "mongo/db/query/query_planner_common.h" +#include "mongo/logv2/log.h" namespace mongo { namespace { @@ -201,10 +202,19 @@ Status CanonicalQuery::init(OperationContext* opCtx, auto unavailableMetadata = validStatus.getValue(); _root = MatchExpression::normalize(std::move(root)); if (feature_flags::gFeatureFlagSbePlanCache.isEnabledAndIgnoreFCV()) { - // When the SBE plan cache is enabled, we auto-parameterize queries in the hopes of caching - // a parameterized plan. Here we add parameter markers to the appropriate match expression - // leaf nodes. - _inputParamIdToExpressionMap = MatchExpression::parameterize(_root.get()); + const bool hasNoTextNodes = + !QueryPlannerCommon::hasNode(_root.get(), MatchExpression::TEXT); + if (hasNoTextNodes) { + // When the SBE plan cache is enabled, we auto-parameterize queries in the hopes of + // caching a parameterized plan. Here we add parameter markers to the appropriate match + // expression leaf nodes. + _inputParamIdToExpressionMap = MatchExpression::parameterize(_root.get()); + } else { + LOGV2_DEBUG(6579310, + 5, + "The query was not auto-parameterized since its match expression tree " + "contains TEXT nodes"); + } } // The tree must always be valid after normalization. dassert(isValid(_root.get(), *_findCommand).isOK()); diff --git a/src/mongo/db/query/canonical_query_encoder.cpp b/src/mongo/db/query/canonical_query_encoder.cpp index a69f5edea98..303cdb6009a 100644 --- a/src/mongo/db/query/canonical_query_encoder.cpp +++ b/src/mongo/db/query/canonical_query_encoder.cpp @@ -730,30 +730,49 @@ public: void visit(const ModMatchExpression* expr) final { auto divisorParam = expr->getDivisorInputParamId(); auto remainderParam = expr->getRemainderInputParamId(); - tassert(6512901, "$mod expression should have divisor param", divisorParam); - tassert(6512902, "$mod expression should have remainder param", remainderParam); + if (divisorParam) { + tassert(6512902, + "$mod expression should have divisor and remainder params", + remainderParam); - encodeParamMarker(*divisorParam); - encodeParamMarker(*remainderParam); + encodeParamMarker(*divisorParam); + encodeParamMarker(*remainderParam); + } else { + tassert(6579300, + "If divisor param is not set in $mod expression reminder param must be unset " + "as well", + !remainderParam); + encodeFull(expr); + } } void visit(const RegexMatchExpression* expr) final { auto sourceRegexParam = expr->getSourceRegexInputParamId(); auto compiledRegexParam = expr->getCompiledRegexInputParamId(); - tassert(6512903, "regex expression should have source param", sourceRegexParam); - tassert(6512904, "regex expression should have compiled param", compiledRegexParam); + if (sourceRegexParam) { + tassert(6512904, + "regex expression should have source and compiled params", + compiledRegexParam); - encodeParamMarker(*sourceRegexParam); - encodeParamMarker(*compiledRegexParam); - // Encode a discriminator so that a "simple" regex which is exactly convertible into index - // bounds has a different shape from a non-simple regex. - // - // We don't actually need to know the contents of the prefix string, so we ignore the first - // member of the pair. - auto [_, isExact] = - analyze_regex::getRegexPrefixMatch(expr->getString().c_str(), expr->getFlags().c_str()); - _builder->appendChar(kEncodeBoundsTightnessDiscriminator); - _builder->appendChar(static_cast(isExact)); + encodeParamMarker(*sourceRegexParam); + encodeParamMarker(*compiledRegexParam); + + // Encode a discriminator so that a "simple" regex which is exactly convertible into + // index bounds has a different shape from a non-simple regex. + // + // We don't actually need to know the contents of the prefix string, so we ignore the + // first member of the pair. + auto [_, isExact] = analyze_regex::getRegexPrefixMatch(expr->getString().c_str(), + expr->getFlags().c_str()); + _builder->appendChar(kEncodeBoundsTightnessDiscriminator); + _builder->appendChar(static_cast(isExact)); + } else { + tassert(6579301, + "If source param is not set in $regex expression compiled param must be unset " + "as well", + !compiledRegexParam); + encodeFull(expr); + } } void visit(const SizeMatchExpression* expr) final { @@ -913,11 +932,21 @@ private: void encodeBitTestExpression(const BitTestMatchExpression* expr) { auto bitPositionsParam = expr->getBitPositionsParamId(); auto bitMaskParam = expr->getBitMaskParamId(); - tassert(6512905, "bit-test expression should have bit positions param", bitPositionsParam); - tassert(6512906, "$mod expression should have bitmask param", bitMaskParam); + if (bitPositionsParam) { - encodeParamMarker(*bitPositionsParam); - encodeParamMarker(*bitMaskParam); + tassert(6512906, + "bit-test expression should have bit positions and bitmask params", + bitMaskParam); + + encodeParamMarker(*bitPositionsParam); + encodeParamMarker(*bitMaskParam); + } else { + tassert(6579302, + "If positions param is not set in a bit-test expression bitmask param must be " + "unset as well", + !bitMaskParam); + encodeFull(expr); + } } /** diff --git a/src/mongo/db/query/canonical_query_test.cpp b/src/mongo/db/query/canonical_query_test.cpp index 3d5f27431a9..dbb2fd929da 100644 --- a/src/mongo/db/query/canonical_query_test.cpp +++ b/src/mongo/db/query/canonical_query_test.cpp @@ -34,6 +34,7 @@ #include "mongo/db/query/collation/collator_factory_interface.h" #include "mongo/db/query/collation/collator_interface_mock.h" #include "mongo/db/query/query_test_service_context.h" +#include "mongo/idl/server_parameter_test_util.h" #include "mongo/unittest/unittest.h" namespace mongo { @@ -454,5 +455,18 @@ TEST(CanonicalQueryTest, InvalidSortOrdersFailToCanonicalize) { assertInvalidSortOrder(fromjson("{'': -1}")); } +TEST(CanonicalQueryTest, DoNotParameterizeTextExpressions) { + RAIIServerParameterControllerForTest controllerSBEPlanCache("featureFlagSbePlanCache", true); + auto cq = + canonicalize("{$text: {$search: \"Hello World!\"}}", + MatchExpressionParser::kDefaultSpecialFeatures | MatchExpressionParser::kText); + ASSERT_FALSE(cq->isParameterized()); +} + +TEST(CanonicalQueryTest, DoParameterizeRegularExpressions) { + RAIIServerParameterControllerForTest controllerSBEPlanCache("featureFlagSbePlanCache", true); + auto cq = canonicalize("{a: 1, b: {$lt: 5}}"); + ASSERT_TRUE(cq->isParameterized()); +} } // namespace } // namespace mongo