SERVER-23611 MatchExpressionParser::_parse() should take a collator

Removes member MatchExpressionParser::_collator.

3 files changed, 66 insertions, 41 deletions

diff --git a/src/mongo/db/matcher/expression_parser.cpp b/src/mongo/db/matcher/expression_parser.cpp
index 97af471a317..be163353fb4 100644
--- a/src/mongo/db/matcher/expression_parser.cpp
+++ b/src/mongo/db/matcher/expression_parser.cpp
@@ -91,14 +91,15 @@ StatusWithMatchExpression MatchExpressionParser::_parseSubField(const BSONObj& c
                                                                 const AndMatchExpression* andSoFar,
                                                                 const char* name,
                                                                 const BSONElement& e,
+                                                                const CollatorInterface* collator,
                                                                 int level) {
     // TODO: these should move to getGtLtOp, or its replacement
 
     if (mongoutils::str::equals("$eq", e.fieldName()))
-        return _parseComparison(name, new EqualityMatchExpression(_collator), e);
+        return _parseComparison(name, new EqualityMatchExpression(collator), e);
 
     if (mongoutils::str::equals("$not", e.fieldName())) {
-        return _parseNot(name, e, level);
+        return _parseNot(name, e, collator, level);
     }
 
     int x = e.getGtLtOp(-1);
@@ -112,13 +113,13 @@ StatusWithMatchExpression MatchExpressionParser::_parseSubField(const BSONObj& c
             return {Status(ErrorCodes::BadValue,
                            mongoutils::str::stream() << "unknown operator: " << e.fieldName())};
         case BSONObj::LT:
-            return _parseComparison(name, new LTMatchExpression(_collator), e);
+            return _parseComparison(name, new LTMatchExpression(collator), e);
         case BSONObj::LTE:
-            return _parseComparison(name, new LTEMatchExpression(_collator), e);
+            return _parseComparison(name, new LTEMatchExpression(collator), e);
         case BSONObj::GT:
-            return _parseComparison(name, new GTMatchExpression(_collator), e);
+            return _parseComparison(name, new GTMatchExpression(collator), e);
         case BSONObj::GTE:
-            return _parseComparison(name, new GTEMatchExpression(_collator), e);
+            return _parseComparison(name, new GTEMatchExpression(collator), e);
         case BSONObj::NE: {
             if (RegEx == e.type()) {
                 // Just because $ne can be rewritten as the negation of an
@@ -126,7 +127,7 @@ StatusWithMatchExpression MatchExpressionParser::_parseSubField(const BSONObj& c
                 return {Status(ErrorCodes::BadValue, "Can't have regex as arg to $ne.")};
             }
             StatusWithMatchExpression s =
-                _parseComparison(name, new EqualityMatchExpression(_collator), e);
+                _parseComparison(name, new EqualityMatchExpression(collator), e);
             if (!s.isOK())
                 return s;
             std::unique_ptr<NotMatchExpression> n = stdx::make_unique<NotMatchExpression>();
@@ -136,13 +137,13 @@ StatusWithMatchExpression MatchExpressionParser::_parseSubField(const BSONObj& c
             return {std::move(n)};
         }
         case BSONObj::Equality:
-            return _parseComparison(name, new EqualityMatchExpression(_collator), e);
+            return _parseComparison(name, new EqualityMatchExpression(collator), e);
 
         case BSONObj::opIN: {
             if (e.type() != Array)
                 return {Status(ErrorCodes::BadValue, "$in needs an array")};
             std::unique_ptr<InMatchExpression> temp =
-                stdx::make_unique<InMatchExpression>(_collator);
+                stdx::make_unique<InMatchExpression>(collator);
             Status s = temp->init(name);
             if (!s.isOK())
                 return s;
@@ -156,7 +157,7 @@ StatusWithMatchExpression MatchExpressionParser::_parseSubField(const BSONObj& c
             if (e.type() != Array)
                 return {Status(ErrorCodes::BadValue, "$nin needs an array")};
             std::unique_ptr<InMatchExpression> temp =
-                stdx::make_unique<InMatchExpression>(_collator);
+                stdx::make_unique<InMatchExpression>(collator);
             Status s = temp->init(name);
             if (!s.isOK())
                 return s;
@@ -245,10 +246,10 @@ StatusWithMatchExpression MatchExpressionParser::_parseSubField(const BSONObj& c
         }
 
         case BSONObj::opELEM_MATCH:
-            return _parseElemMatch(name, e, level);
+            return _parseElemMatch(name, e, collator, level);
 
         case BSONObj::opALL:
-            return _parseAll(name, e, level);
+            return _parseAll(name, e, collator, level);
 
         case BSONObj::opWITHIN:
         case BSONObj::opGEO_INTERSECTS:
@@ -281,7 +282,9 @@ StatusWithMatchExpression MatchExpressionParser::_parseSubField(const BSONObj& c
                    mongoutils::str::stream() << "not handled: " << e.fieldName())};
 }
 
-StatusWithMatchExpression MatchExpressionParser::_parse(const BSONObj& obj, int level) {
+StatusWithMatchExpression MatchExpressionParser::_parse(const BSONObj& obj,
+                                                        const CollatorInterface* collator,
+                                                        int level) {
     if (level > kMaximumTreeDepth) {
         mongoutils::str::stream ss;
         ss << "exceeded maximum query tree depth of " << kMaximumTreeDepth << " at "
@@ -305,7 +308,7 @@ StatusWithMatchExpression MatchExpressionParser::_parse(const BSONObj& obj, int
                 if (e.type() != Array)
                     return {Status(ErrorCodes::BadValue, "$or must be an array")};
                 std::unique_ptr<OrMatchExpression> temp = stdx::make_unique<OrMatchExpression>();
-                Status s = _parseTreeList(e.Obj(), temp.get(), level);
+                Status s = _parseTreeList(e.Obj(), temp.get(), collator, level);
                 if (!s.isOK())
                     return s;
                 root->add(temp.release());
@@ -313,7 +316,7 @@ StatusWithMatchExpression MatchExpressionParser::_parse(const BSONObj& obj, int
                 if (e.type() != Array)
                     return {Status(ErrorCodes::BadValue, "$and must be an array")};
                 std::unique_ptr<AndMatchExpression> temp = stdx::make_unique<AndMatchExpression>();
-                Status s = _parseTreeList(e.Obj(), temp.get(), level);
+                Status s = _parseTreeList(e.Obj(), temp.get(), collator, level);
                 if (!s.isOK())
                     return s;
                 root->add(temp.release());
@@ -321,7 +324,7 @@ StatusWithMatchExpression MatchExpressionParser::_parse(const BSONObj& obj, int
                 if (e.type() != Array)
                     return {Status(ErrorCodes::BadValue, "$nor must be an array")};
                 std::unique_ptr<NorMatchExpression> temp = stdx::make_unique<NorMatchExpression>();
-                Status s = _parseTreeList(e.Obj(), temp.get(), level);
+                Status s = _parseTreeList(e.Obj(), temp.get(), collator, level);
                 if (!s.isOK())
                     return s;
                 root->add(temp.release());
@@ -348,9 +351,9 @@ StatusWithMatchExpression MatchExpressionParser::_parse(const BSONObj& obj, int
                        mongoutils::str::equals("id", rest) || mongoutils::str::equals("db", rest)) {
                 // DBRef fields.
                 // 'id' is collation-aware. 'ref' and 'db' are compared using binary comparison.
-                const CollatorInterface* collator = (str::equals("id", rest) ? _collator : nullptr);
                 std::unique_ptr<ComparisonMatchExpression> eq =
-                    stdx::make_unique<EqualityMatchExpression>(collator);
+                    stdx::make_unique<EqualityMatchExpression>(str::equals("id", rest) ? collator
+                                                                                       : nullptr);
                 Status s = eq->init(e.fieldName(), e);
                 if (!s.isOK())
                     return s;
@@ -366,7 +369,7 @@ StatusWithMatchExpression MatchExpressionParser::_parse(const BSONObj& obj, int
         }
 
         if (_isExpressionDocument(e, false)) {
-            Status s = _parseSub(e.fieldName(), e.Obj(), root.get(), level);
+            Status s = _parseSub(e.fieldName(), e.Obj(), root.get(), collator, level);
             if (!s.isOK())
                 return s;
             continue;
@@ -381,7 +384,7 @@ StatusWithMatchExpression MatchExpressionParser::_parse(const BSONObj& obj, int
         }
 
         std::unique_ptr<ComparisonMatchExpression> eq =
-            stdx::make_unique<EqualityMatchExpression>(_collator);
+            stdx::make_unique<EqualityMatchExpression>(collator);
         Status s = eq->init(e.fieldName(), e);
         if (!s.isOK())
             return s;
@@ -401,6 +404,7 @@ StatusWithMatchExpression MatchExpressionParser::_parse(const BSONObj& obj, int
 Status MatchExpressionParser::_parseSub(const char* name,
                                         const BSONObj& sub,
                                         AndMatchExpression* root,
+                                        const CollatorInterface* collator,
                                         int level) {
     // The one exception to {field : {fully contained argument} } is, of course, geo.  Example:
     // sub == { field : {$near[Sphere]: [0,0], $maxDistance: 1000, $minDistance: 10 } }
@@ -446,7 +450,7 @@ Status MatchExpressionParser::_parseSub(const char* name,
     while (j.more()) {
         BSONElement deep = j.next();
 
-        StatusWithMatchExpression s = _parseSubField(sub, root, name, deep, level);
+        StatusWithMatchExpression s = _parseSubField(sub, root, name, deep, collator, level);
         if (!s.isOK())
             return s.getStatus();
 
@@ -673,6 +677,7 @@ StatusWithMatchExpression MatchExpressionParser::_parseType(const char* name,
 
 StatusWithMatchExpression MatchExpressionParser::_parseElemMatch(const char* name,
                                                                  const BSONElement& e,
+                                                                 const CollatorInterface* collator,
                                                                  int level) {
     if (e.type() != Object)
         return {Status(ErrorCodes::BadValue, "$elemMatch needs an Object")};
@@ -703,7 +708,7 @@ StatusWithMatchExpression MatchExpressionParser::_parseElemMatch(const char* nam
         // value case
 
         AndMatchExpression theAnd;
-        Status s = _parseSub("", obj, &theAnd, level);
+        Status s = _parseSub("", obj, &theAnd, collator, level);
         if (!s.isOK())
             return s;
 
@@ -727,7 +732,7 @@ StatusWithMatchExpression MatchExpressionParser::_parseElemMatch(const char* nam
 
     // object case
 
-    StatusWithMatchExpression subRaw = _parse(obj, level);
+    StatusWithMatchExpression subRaw = _parse(obj, collator, level);
     if (!subRaw.isOK())
         return subRaw;
     std::unique_ptr<MatchExpression> sub = std::move(subRaw.getValue());
@@ -749,6 +754,7 @@ StatusWithMatchExpression MatchExpressionParser::_parseElemMatch(const char* nam
 
 StatusWithMatchExpression MatchExpressionParser::_parseAll(const char* name,
                                                            const BSONElement& e,
+                                                           const CollatorInterface* collator,
                                                            int level) {
     if (e.type() != Array)
         return {Status(ErrorCodes::BadValue, "$all needs an array")};
@@ -778,7 +784,7 @@ StatusWithMatchExpression MatchExpressionParser::_parseAll(const char* name,
             }
 
             StatusWithMatchExpression inner =
-                _parseElemMatch(name, hopefullyElemMatchObj.firstElement(), level);
+                _parseElemMatch(name, hopefullyElemMatchObj.firstElement(), collator, level);
             if (!inner.isOK())
                 return inner;
             myAnd->add(inner.getValue().release());
@@ -800,7 +806,7 @@ StatusWithMatchExpression MatchExpressionParser::_parseAll(const char* name,
             return {Status(ErrorCodes::BadValue, "no $ expressions in $all")};
         } else {
             std::unique_ptr<EqualityMatchExpression> x =
-                stdx::make_unique<EqualityMatchExpression>(_collator);
+                stdx::make_unique<EqualityMatchExpression>(collator);
             Status s = x->init(name, e);
             if (!s.isOK())
                 return s;
diff --git a/src/mongo/db/matcher/expression_parser.h b/src/mongo/db/matcher/expression_parser.h
index 3c150ef9c9d..c1b9e09fc2b 100644
--- a/src/mongo/db/matcher/expression_parser.h
+++ b/src/mongo/db/matcher/expression_parser.h
@@ -54,13 +54,12 @@ public:
                                            const ExtensionsCallback& extensionsCallback,
                                            const CollatorInterface* collator) {
         // The 0 initializes the match expression tree depth.
-        return MatchExpressionParser(&extensionsCallback, collator)._parse(obj, 0);
+        return MatchExpressionParser(&extensionsCallback)._parse(obj, collator, 0);
     }
 
 private:
-    MatchExpressionParser(const ExtensionsCallback* extensionsCallback,
-                          const CollatorInterface* collator)
-        : _extensionsCallback(extensionsCallback), _collator(collator) {}
+    MatchExpressionParser(const ExtensionsCallback* extensionsCallback)
+        : _extensionsCallback(extensionsCallback) {}
 
     /**
      * 5 = false
@@ -85,18 +84,27 @@ private:
     /**
      * Parse 'obj' and return either a MatchExpression or an error.
      *
+     * 'collator' is the collator that constructed collation-aware MatchExpressions will use.  It
+     * must outlive the returned MatchExpression and any clones made of it.
+     *
      * 'level' tracks the current depth of the tree across recursive calls to this
      * function. Used in order to apply special logic at the top-level and to return an
      * error if the tree exceeds the maximum allowed depth.
      */
-    StatusWithMatchExpression _parse(const BSONObj& obj, int level);
+    StatusWithMatchExpression _parse(const BSONObj& obj,
+                                     const CollatorInterface* collator,
+                                     int level);
 
     /**
      * parses a field in a sub expression
      * if the query is { x : { $gt : 5, $lt : 8 } }
      * e is { $gt : 5, $lt : 8 }
      */
-    Status _parseSub(const char* name, const BSONObj& obj, AndMatchExpression* root, int level);
+    Status _parseSub(const char* name,
+                     const BSONObj& obj,
+                     AndMatchExpression* root,
+                     const CollatorInterface* collator,
+                     int level);
 
     /**
      * parses a single field in a sub expression
@@ -107,6 +115,7 @@ private:
                                              const AndMatchExpression* andSoFar,
                                              const char* name,
                                              const BSONElement& e,
+                                             const CollatorInterface* collator,
                                              int level);
 
     StatusWithMatchExpression _parseComparison(const char* name,
@@ -126,15 +135,27 @@ private:
 
     // arrays
 
-    StatusWithMatchExpression _parseElemMatch(const char* name, const BSONElement& e, int level);
+    StatusWithMatchExpression _parseElemMatch(const char* name,
+                                              const BSONElement& e,
+                                              const CollatorInterface* collator,
+                                              int level);
 
-    StatusWithMatchExpression _parseAll(const char* name, const BSONElement& e, int level);
+    StatusWithMatchExpression _parseAll(const char* name,
+                                        const BSONElement& e,
+                                        const CollatorInterface* collator,
+                                        int level);
 
     // tree
 
-    Status _parseTreeList(const BSONObj& arr, ListOfMatchExpression* out, int level);
+    Status _parseTreeList(const BSONObj& arr,
+                          ListOfMatchExpression* out,
+                          const CollatorInterface* collator,
+                          int level);
 
-    StatusWithMatchExpression _parseNot(const char* name, const BSONElement& e, int level);
+    StatusWithMatchExpression _parseNot(const char* name,
+                                        const BSONElement& e,
+                                        const CollatorInterface* collator,
+                                        int level);
 
     /**
      * Parses 'e' into a BitTestMatchExpression.
@@ -153,10 +174,6 @@ private:
     // Performs parsing for the match extensions. We do not own this pointer - it has to live
     // as long as the parser is active.
     const ExtensionsCallback* _extensionsCallback;
-
-    // Collator that constructed collation-aware MatchExpressions will use.
-    // We do not own this pointer - it has to live as long as the parser is active.
-    const CollatorInterface* _collator;
 };
 
 typedef stdx::function<StatusWithMatchExpression(
diff --git a/src/mongo/db/matcher/expression_parser_tree.cpp b/src/mongo/db/matcher/expression_parser_tree.cpp
index 26ad561e756..1d1761f96e6 100644
--- a/src/mongo/db/matcher/expression_parser_tree.cpp
+++ b/src/mongo/db/matcher/expression_parser_tree.cpp
@@ -45,6 +45,7 @@ const int MatchExpressionParser::kMaximumTreeDepth = 100;
 
 Status MatchExpressionParser::_parseTreeList(const BSONObj& arr,
                                              ListOfMatchExpression* out,
+                                             const CollatorInterface* collator,
                                              int level) {
     if (arr.isEmpty())
         return Status(ErrorCodes::BadValue, "$and/$or/$nor must be a nonempty array");
@@ -56,7 +57,7 @@ Status MatchExpressionParser::_parseTreeList(const BSONObj& arr,
         if (e.type() != Object)
             return Status(ErrorCodes::BadValue, "$or/$and/$nor entries need to be full objects");
 
-        StatusWithMatchExpression sub = _parse(e.Obj(), level);
+        StatusWithMatchExpression sub = _parse(e.Obj(), collator, level);
         if (!sub.isOK())
             return sub.getStatus();
 
@@ -67,6 +68,7 @@ Status MatchExpressionParser::_parseTreeList(const BSONObj& arr,
 
 StatusWithMatchExpression MatchExpressionParser::_parseNot(const char* name,
                                                            const BSONElement& e,
+                                                           const CollatorInterface* collator,
                                                            int level) {
     if (e.type() == RegEx) {
         StatusWithMatchExpression s = _parseRegexElement(name, e);
@@ -87,7 +89,7 @@ StatusWithMatchExpression MatchExpressionParser::_parseNot(const char* name,
         return StatusWithMatchExpression(ErrorCodes::BadValue, "$not cannot be empty");
 
     std::unique_ptr<AndMatchExpression> theAnd = stdx::make_unique<AndMatchExpression>();
-    Status s = _parseSub(name, notObject, theAnd.get(), level);
+    Status s = _parseSub(name, notObject, theAnd.get(), collator, level);
     if (!s.isOK())
         return StatusWithMatchExpression(s);