SERVER-12307 CanonicalQuery tracks whether an update or delete is isolated.

11 files changed, 69 insertions, 52 deletions

diff --git a/src/mongo/db/matcher/expression.cpp b/src/mongo/db/matcher/expression.cpp
index aa6a2ddbc30..989e075d1de 100644
--- a/src/mongo/db/matcher/expression.cpp
+++ b/src/mongo/db/matcher/expression.cpp
@@ -55,16 +55,6 @@ bool MatchExpression::matchesBSON(const BSONObj& doc, MatchDetails* details) con
     return matches(&mydoc, details);
 }
 
-
-void AtomicMatchExpression::debugString(StringBuilder& debug, int level) const {
-    _debugAddSpace(debug, level);
-    debug << "$atomic\n";
-}
-
-void AtomicMatchExpression::toBSON(BSONObjBuilder* out) const {
-    out->append("$isolated", 1);
-}
-
 void FalseMatchExpression::debugString(StringBuilder& debug, int level) const {
     _debugAddSpace(debug, level);
     debug << "$false\n";
diff --git a/src/mongo/db/matcher/expression.h b/src/mongo/db/matcher/expression.h
index 32473d78e94..43433346fa7 100644
--- a/src/mongo/db/matcher/expression.h
+++ b/src/mongo/db/matcher/expression.h
@@ -85,7 +85,6 @@ public:
         WHERE,
 
         // things that maybe shouldn't even be nodes
-        ATOMIC,
         ALWAYS_FALSE,
 
         // Things that we parse but cannot be answered without an index.
@@ -249,35 +248,6 @@ private:
     std::unique_ptr<TagData> _tagData;
 };
 
-/**
- * this isn't really an expression, but a hint to other things
- * not sure where to put it in the end
- */
-class AtomicMatchExpression : public MatchExpression {
-public:
-    AtomicMatchExpression() : MatchExpression(ATOMIC) {}
-
-    virtual bool matches(const MatchableDocument* doc, MatchDetails* details = 0) const {
-        return true;
-    }
-
-    virtual bool matchesSingleElement(const BSONElement& e) const {
-        return true;
-    }
-
-    virtual std::unique_ptr<MatchExpression> shallowClone() const {
-        return stdx::make_unique<AtomicMatchExpression>();
-    }
-
-    virtual void debugString(StringBuilder& debug, int level = 0) const;
-
-    virtual void toBSON(BSONObjBuilder* out) const;
-
-    virtual bool equivalent(const MatchExpression* other) const {
-        return other->matchType() == ATOMIC;
-    }
-};
-
 class FalseMatchExpression : public MatchExpression {
 public:
     FalseMatchExpression() : MatchExpression(ALWAYS_FALSE) {}
diff --git a/src/mongo/db/matcher/expression_parser.cpp b/src/mongo/db/matcher/expression_parser.cpp
index 70fcc227de4..859458453dd 100644
--- a/src/mongo/db/matcher/expression_parser.cpp
+++ b/src/mongo/db/matcher/expression_parser.cpp
@@ -327,8 +327,8 @@ StatusWithMatchExpression MatchExpressionParser::_parse(const BSONObj& obj, int
                 if (!topLevel)
                     return {Status(ErrorCodes::BadValue,
                                    "$atomic/$isolated has to be at the top level")};
-                if (e.trueValue())
-                    root->add(new AtomicMatchExpression());
+                // Don't do anything with the expression; CanonicalQuery::init() will look through
+                // the BSONObj again for a $atomic/$isolated.
             } else if (mongoutils::str::equals("where", rest)) {
                 StatusWithMatchExpression s = _extensionsCallback->parseWhere(e);
                 if (!s.isOK())
diff --git a/src/mongo/db/matcher/expression_parser_test.cpp b/src/mongo/db/matcher/expression_parser_test.cpp
index 4846ea2aaeb..0c800d2863b 100644
--- a/src/mongo/db/matcher/expression_parser_test.cpp
+++ b/src/mongo/db/matcher/expression_parser_test.cpp
@@ -63,7 +63,7 @@ TEST(MatchExpressionParserTest, Multiple1) {
     ASSERT(!result.getValue()->matchesBSON(BSON("x" << 5 << "y" << 4)));
 }
 
-TEST(AtomicMatchExpressionTest, Simple1) {
+TEST(AtomicMatchExpressionTest, AtomicOperator1) {
     BSONObj query = BSON("x" << 5 << "$atomic" << BSON("$gt" << 5 << "$lt" << 8));
     StatusWithMatchExpression result =
         MatchExpressionParser::parse(query, ExtensionsCallbackDisallowExtensions());
diff --git a/src/mongo/db/ops/parsed_delete.cpp b/src/mongo/db/ops/parsed_delete.cpp
index 1acc76dcb3e..ba193ea9ac8 100644
--- a/src/mongo/db/ops/parsed_delete.cpp
+++ b/src/mongo/db/ops/parsed_delete.cpp
@@ -113,9 +113,8 @@ bool ParsedDelete::canYield() const {
 }
 
 bool ParsedDelete::isIsolated() const {
-    return _canonicalQuery.get()
-        ? QueryPlannerCommon::hasNode(_canonicalQuery->root(), MatchExpression::ATOMIC)
-        : LiteParsedQuery::isQueryIsolated(_request->getQuery());
+    return _canonicalQuery.get() ? _canonicalQuery->isIsolated()
+                                 : LiteParsedQuery::isQueryIsolated(_request->getQuery());
 }
 
 bool ParsedDelete::hasParsedQuery() const {
diff --git a/src/mongo/db/ops/parsed_update.cpp b/src/mongo/db/ops/parsed_update.cpp
index d24a2aeef95..174538f33a8 100644
--- a/src/mongo/db/ops/parsed_update.cpp
+++ b/src/mongo/db/ops/parsed_update.cpp
@@ -129,9 +129,8 @@ bool ParsedUpdate::canYield() const {
 }
 
 bool ParsedUpdate::isIsolated() const {
-    return _canonicalQuery.get()
-        ? QueryPlannerCommon::hasNode(_canonicalQuery->root(), MatchExpression::ATOMIC)
-        : LiteParsedQuery::isQueryIsolated(_request->getQuery());
+    return _canonicalQuery.get() ? _canonicalQuery->isIsolated()
+                                 : LiteParsedQuery::isQueryIsolated(_request->getQuery());
 }
 
 bool ParsedUpdate::hasParsedQuery() const {
diff --git a/src/mongo/db/query/canonical_query.cpp b/src/mongo/db/query/canonical_query.cpp
index c65eecd0707..1256d3530ed 100644
--- a/src/mongo/db/query/canonical_query.cpp
+++ b/src/mongo/db/query/canonical_query.cpp
@@ -315,6 +315,7 @@ Status CanonicalQuery::init(LiteParsedQuery* lpq,
     _pq.reset(lpq);
 
     _hasNoopExtensions = extensionsCallback.hasNoopExtensions();
+    _isIsolated = LiteParsedQuery::isQueryIsolated(lpq->getFilter());
 
     // Normalize, sort and validate tree.
     root = normalizeTree(root);
diff --git a/src/mongo/db/query/canonical_query.h b/src/mongo/db/query/canonical_query.h
index 3936809ad05..65741b8f204 100644
--- a/src/mongo/db/query/canonical_query.h
+++ b/src/mongo/db/query/canonical_query.h
@@ -205,6 +205,14 @@ public:
         return _hasNoopExtensions;
     }
 
+    /**
+     * Returns true if the query this CanonicalQuery was parsed from included a $isolated/$atomic
+     * operator.
+     */
+    bool isIsolated() const {
+        return _isIsolated;
+    }
+
 private:
     // You must go through canonicalize to create a CanonicalQuery.
     CanonicalQuery() {}
@@ -224,6 +232,8 @@ private:
     std::unique_ptr<ParsedProjection> _proj;
 
     bool _hasNoopExtensions = false;
+
+    bool _isIsolated;
 };
 
 }  // namespace mongo
diff --git a/src/mongo/db/query/canonical_query_test.cpp b/src/mongo/db/query/canonical_query_test.cpp
index 11bdf8140aa..e544d1a8aaa 100644
--- a/src/mongo/db/query/canonical_query_test.cpp
+++ b/src/mongo/db/query/canonical_query_test.cpp
@@ -554,6 +554,29 @@ std::unique_ptr<CanonicalQuery> canonicalize(const char* queryStr,
 }
 
 /**
+ * Test that CanonicalQuery::isIsolated() returns correctly.
+ */
+TEST(CanonicalQueryTest, IsIsolatedReturnsTrueWithIsolated) {
+    unique_ptr<CanonicalQuery> cq = canonicalize("{$isolated: 1, x: 3}");
+    ASSERT_TRUE(cq->isIsolated());
+}
+
+TEST(CanonicalQueryTest, IsIsolatedReturnsTrueWithAtomic) {
+    unique_ptr<CanonicalQuery> cq = canonicalize("{$atomic: 1, x: 3}");
+    ASSERT_TRUE(cq->isIsolated());
+}
+
+TEST(CanonicalQueryTest, IsIsolatedReturnsFalseWithIsolated) {
+    unique_ptr<CanonicalQuery> cq = canonicalize("{$isolated: 0, x: 3}");
+    ASSERT_FALSE(cq->isIsolated());
+}
+
+TEST(CanonicalQueryTest, IsIsolatedReturnsFalseWithAtomic) {
+    unique_ptr<CanonicalQuery> cq = canonicalize("{$atomic: 0, x: 3}");
+    ASSERT_FALSE(cq->isIsolated());
+}
+
+/**
  * Test function for CanonicalQuery::normalize.
  */
 void testNormalizeQuery(const char* queryStr, const char* expectedExprStr) {
diff --git a/src/mongo/db/query/lite_parsed_query_test.cpp b/src/mongo/db/query/lite_parsed_query_test.cpp
index 4ae2f3096b9..3bc70288b67 100644
--- a/src/mongo/db/query/lite_parsed_query_test.cpp
+++ b/src/mongo/db/query/lite_parsed_query_test.cpp
@@ -212,6 +212,34 @@ TEST(LiteParsedQueryTest, AllowTailableWithNaturalSort) {
     ASSERT_EQ(result.getValue()->getSort(), BSON("$natural" << 1));
 }
 
+TEST(LiteParsedQueryTest, IsIsolatedReturnsTrueWithIsolated) {
+    auto result = LiteParsedQuery::makeAsFindCmd(testns,
+                                                 BSON("$isolated" << 1));
+
+    ASSERT_TRUE(LiteParsedQuery::isQueryIsolated(result->getFilter()));
+}
+
+TEST(LiteParsedQueryTest, IsIsolatedReturnsTrueWithAtomic) {
+    auto result = LiteParsedQuery::makeAsFindCmd(testns,
+                                                 BSON("$atomic" << 1));
+
+    ASSERT_TRUE(LiteParsedQuery::isQueryIsolated(result->getFilter()));
+}
+
+TEST(LiteParsedQueryTest, IsIsolatedReturnsFalseWithIsolated) {
+    auto result = LiteParsedQuery::makeAsFindCmd(testns,
+                                                 BSON("$isolated" << false));
+
+    ASSERT_FALSE(LiteParsedQuery::isQueryIsolated(result->getFilter()));
+}
+
+TEST(LiteParsedQueryTest, IsIsolatedReturnsFalseWithAtomic) {
+    auto result = LiteParsedQuery::makeAsFindCmd(testns,
+                                                 BSON("$atomic" << false));
+
+    ASSERT_FALSE(LiteParsedQuery::isQueryIsolated(result->getFilter()));
+}
+
 // Helper function which returns the Status of creating a LiteParsedQuery object with the given
 // parameters.
 void assertLiteParsedQuerySuccess(const BSONObj& query, const BSONObj& proj, const BSONObj& sort) {
diff --git a/src/mongo/db/query/plan_cache.cpp b/src/mongo/db/query/plan_cache.cpp
index 8aabb36351a..ab64b8f1d26 100644
--- a/src/mongo/db/query/plan_cache.cpp
+++ b/src/mongo/db/query/plan_cache.cpp
@@ -144,9 +144,6 @@ const char* encodeMatchType(MatchExpression::MatchType mt) {
         case MatchExpression::WHERE:
             return "wh";
             break;
-        case MatchExpression::ATOMIC:
-            return "at";
-            break;
         case MatchExpression::ALWAYS_FALSE:
             return "af";
             break;