Revert "SERVER-14662 Reject ambiguous positional projections and updates"

This reverts commit 2eea3f09aea9c92df9aa0d4e47840869bf04d7b8.

9 files changed, 63 insertions, 79 deletions

diff --git a/jstests/core/and2.js b/jstests/core/and2.js
index f3b014c13ad..a794268130b 100644
--- a/jstests/core/and2.js
+++ b/jstests/core/and2.js
@@ -11,3 +11,17 @@ t.drop();
 t.save({a: [1, 2]});
 t.update({$and: [{a: 1}]}, {$set: {'a.$': 5}});
 assert.eq([5, 2], t.findOne().a);
+
+// Make sure dollar sign operator with $and is consistent with no $and case
+t.drop();
+t.save({a: [1, 2], b: [3, 4]});
+t.update({a: 1, b: 4}, {$set: {'a.$': 5}});
+// Probably not what we want here, just trying to make sure $and is consistent
+assert.eq({a: [1, 5], b: [3, 4]}, t.find({}, {_id: 0}).toArray()[0]);
+
+// Make sure dollar sign operator with $and is consistent with no $and case
+t.drop();
+t.save({a: [1, 2], b: [3, 4]});
+t.update({a: 1, $and: [{b: 4}]}, {$set: {'a.$': 5}});
+// Probably not what we want here, just trying to make sure $and is consistent
+assert.eq({a: [1, 5], b: [3, 4]}, t.find({}, {_id: 0}).toArray()[0]);
diff --git a/jstests/core/elemMatchProjection.js b/jstests/core/elemMatchProjection.js
index e7aa5194607..4e80e8a296e 100644
--- a/jstests/core/elemMatchProjection.js
+++ b/jstests/core/elemMatchProjection.js
@@ -214,14 +214,6 @@ assert.eq(
     t.find({group: 10}, {_id: 0, x: {$elemMatch: {a: 1}}, y: {$elemMatch: {c: 3}}}).toArray()[0],
     "multiple $elemMatch on unique fields 1");
 
-assert.eq({"x": [{"y": [{"a": 1, "b": 2}, {"a": 3, "b": 4}]}]},
-          t.find({group: 8}, {_id: 0, x: {$elemMatch: {y: {$elemMatch: {a: 3}}}}}).toArray()[0],
-          "nested $elemMatch");
-
-assert.throws(function() {
-    t.find({group: 3, 'x.a': 1}, {'x.$': 1, y: {$elemMatch: {aa: 1}}}).toArray();
-}, [], "throw on positional operator with $elemMatch");
-
 if (false) {
     assert.eq(2,  // SERVER-1243: handle multiple $elemMatch results
               t.find({group: 4}, {x: {$elemMatchAll: {a: {$lte: 2}}}}).toArray()[0].x.length,
@@ -254,3 +246,29 @@ a = t.find({group: 3}, {x: {$elemMatch: {a: 1}}}).batchSize(1);
 while (a.hasNext()) {
     assert.eq(1, a.next().x[0].a, "positional getMore test");
 }
+
+// verify the positional update operator matches the same element as the the positional find.  this
+// is to ensure consistent behavior with updates until SERVER-1013 is resolved, at which point the
+// following tests should be updated.
+
+t.update({group: 10, 'x.a': 3, 'y.c': 1}, {$set: {'x.$': 100}}, false, true);
+// updated the wrong element, so the following assertions should be true
+assert.eq(100,
+          t.find({group: 10, 'y.c': 1, x: 100}, {'x.$': 1}).toArray()[0].x[0],
+          "wrong single element match after update");
+
+assert.eq(100,
+          t.find({group: 10, x: 100, 'y.c': 1}, {'x.$': 1}).toArray()[0].x[0],
+          "wrong single element match after update");
+
+t.remove({group: 10});
+t.insert({group: 10, x: [{a: 1, b: 2}, {a: 3, b: 4}], y: [{c: 1, d: 2}, {c: 3, d: 4}]});
+
+t.update({group: 10, 'y.c': 1, 'x.a': 3}, {$set: {'x.$': 100}}, false, true);
+// updated the correct element
+assert.eq(100,
+          t.find({group: 10, 'y.c': 1, x: 100}, {'x.$': 1}).toArray()[0].x[0],
+          "right single element match after update");
+assert.eq(100,
+          t.find({group: 10, x: 100, 'y.c': 1}, {'x.$': 1}).toArray()[0].x[0],
+          "right single element match after update");
diff --git a/jstests/core/updatel.js b/jstests/core/updatel.js
index 69aa997a224..78bd9b252dc 100644
--- a/jstests/core/updatel.js
+++ b/jstests/core/updatel.js
@@ -11,60 +11,45 @@ t.drop();
 
 // The collection is empty, forcing an upsert.  In this case the query has no array position match
 // to substiture for the positional operator.  SERVER-4713
-assert.writeError(t.update({}, {$set: {'a.$.b': 1}}, true));
+res = t.update({}, {$set: {'a.$.b': 1}}, true);
+assert(res.hasWriteError(), "An error is reported.");
 assert.eq(0, t.count(), "No upsert occurred.");
 
 // Save a document to the collection so it is no longer empty.
-assert.writeOK(t.save({_id: 0}));
+t.save({_id: 0});
 
 // Now, with an existing document, trigger an update rather than an upsert.  The query has no array
 // position match to substiture for the positional operator.  SERVER-6669
-assert.writeError(t.update({}, {$set: {'a.$.b': 1}}));
+res = t.update({}, {$set: {'a.$.b': 1}});
+assert(res.hasWriteError(), "An error is reported.");
 assert.eq([{_id: 0}], t.find().toArray(), "No update occurred.");
 
 // Now, try with an update by _id (without a query array match).
-assert.writeError(t.update({_id: 0}, {$set: {'a.$.b': 1}}));
+res = t.update({_id: 0}, {$set: {'a.$.b': 1}});
+assert(res.hasWriteError(), "An error is reported.");
 assert.eq([{_id: 0}], t.find().toArray(), "No update occurred.");
 
 // Seed the collection with a document suitable for the following check.
-assert.writeOK(t.remove({}));
-assert.writeOK(t.save({_id: 0, a: [{b: {c: 1}}]}));
+t.remove({});
+t.save({_id: 0, a: [{b: {c: 1}}]});
 
 // Now, attempt to apply an update with two nested positional operators.  There is a positional
 // query match for the first positional operator but not the second.  Note that dollar sign
 // substitution for multiple positional opertors is not implemented (SERVER-831).
-assert.writeError(t.update({'a.b.c': 1}, {$set: {'a.$.b.$.c': 2}}));
+res = t.update({'a.b.c': 1}, {$set: {'a.$.b.$.c': 2}});
+assert(res.hasWriteError(), "An error is reported");
 assert.eq([{_id: 0, a: [{b: {c: 1}}]}], t.find().toArray(), "No update occurred.");
 
 // SERVER-1155 test an update with the positional operator
 // that has a regex in the query field
 t.drop();
-assert.writeOK(t.insert({_id: 1, arr: [{a: "z", b: 1}]}));
-assert.writeOK(t.update({"arr.a": /^z$/}, {$set: {"arr.$.b": 2}}, false, true));
+t.insert({_id: 1, arr: [{a: "z", b: 1}]});
+res = t.update({"arr.a": /^z$/}, {$set: {"arr.$.b": 2}}, false, true);
+assert.writeOK(res);
 assert.eq(t.findOne().arr[0], {a: "z", b: 2});
 
 t.drop();
-assert.writeOK(t.insert({_id: 1, arr: [{a: "z", b: 1}, {a: "abc", b: 2}, {a: "lmn", b: 3}]}));
-assert.writeOK(t.update({"arr.a": /l/}, {$inc: {"arr.$.b": 2}}, false, true));
+t.insert({_id: 1, arr: [{a: "z", b: 1}, {a: "abc", b: 2}, {a: "lmn", b: 3}]});
+res = t.update({"arr.a": /l/}, {$inc: {"arr.$.b": 2}}, false, true);
+assert.writeOK(res);
 assert.eq(t.findOne().arr[2], {a: "lmn", b: 5});
-
-// Test updates with ambiguous positional operator.
-t.drop();
-assert.writeOK(t.insert({_id: 0, a: [1, 2]}));
-assert.writeError(t.update({$and: [{a: 1}, {a: 2}]}, {$set: {'a.$': 5}}));
-assert.eq([{_id: 0, a: [1, 2]}], t.find().toArray(), "No update occurred.");
-
-t.drop();
-assert.writeOK(t.insert({_id: 0, a: [1], b: [2]}));
-assert.writeError(t.update({a: 1, b: 2}, {$set: {'a.$': 5}}));
-assert.eq([{_id: 0, a: [1], b: [2]}], t.find().toArray(), "No update occurred.");
-
-t.drop();
-assert.writeOK(t.insert({_id: 0, a: [1], b: [2]}));
-assert.writeError(t.update({a: {$elemMatch: {$lt: 2}}, b: 2}, {$set: {'a.$': 5}}));
-assert.eq([{_id: 0, a: [1], b: [2]}], t.find().toArray(), "No update occurred.");
-
-t.drop();
-assert.writeOK(t.insert({_id: 0, a: [{b: 1}, {c: 2}]}));
-assert.writeError(t.update({'a.b': 1, 'a.c': 2}, {$set: {'a.$': 5}}));
-assert.eq([{_id: 0, a: [{b: 1}, {c: 2}]}], t.find().toArray(), "No update occurred.");
\ No newline at end of file
diff --git a/src/mongo/db/exec/projection_exec.cpp b/src/mongo/db/exec/projection_exec.cpp
index 0bc0809e01a..21fef7d571a 100644
--- a/src/mongo/db/exec/projection_exec.cpp
+++ b/src/mongo/db/exec/projection_exec.cpp
@@ -267,13 +267,6 @@ Status ProjectionExec::transform(WorkingSetMember* member) const {
             matchDetails.requestElemMatchKey();
             verify(NULL != _queryExpression);
             verify(_queryExpression->matchesBSON(member->obj.value(), &matchDetails));
-
-            // Performing a positional projection requires valid MatchDetails. For example,
-            // ambiguity caused by multiple implicit array traversal predicates can lead to invalid
-            // match details.
-            if (!matchDetails.isValid()) {
-                return Status(ErrorCodes::InternalError, "ambiguous positional projection");
-            }
         }
 
         Status projStatus = transform(member->obj.value(), &bob, &matchDetails);
@@ -406,10 +399,6 @@ Status ProjectionExec::transform(const BSONObj& in,
         arrayDetails.requestElemMatchKey();
 
         if (matcher->second->matchesBSON(in, &arrayDetails)) {
-            // Since we create a special matcher for each $elemMatch projection, we should always
-            // have valid MatchDetails.
-            invariant(arrayDetails.isValid());
-
             FieldMap::const_iterator fieldIt = _fields.find(elt.fieldName());
             if (_fields.end() == fieldIt) {
                 return Status(ErrorCodes::BadValue,
diff --git a/src/mongo/db/exec/projection_exec_test.cpp b/src/mongo/db/exec/projection_exec_test.cpp
index b9e240a5a24..fc458b1c3f5 100644
--- a/src/mongo/db/exec/projection_exec_test.cpp
+++ b/src/mongo/db/exec/projection_exec_test.cpp
@@ -189,12 +189,6 @@ TEST(ProjectionExecTest, TransformPositionalDollar) {
 
     // Invalid position $ projections.
     testTransform("{'a.$': 1}", "{a: {$size: 1}}", "{a: [5]}", false, "");
-
-    // Ambigous position $ projections.
-    testTransform("{'a.$': 1}", "{$and: [{a: 1}, {a: 2}]}", "{a: [1, 2]}", false, "");
-    testTransform("{'a.$': 1}", "{a: 1, b: 2}", "{a: [1], b: [2]}", false, "");
-    testTransform("{'a.$': 1}", "{a: {$elemMatch: {$lt: 2}}, b: 2}", "{a: [1], b: [2]}", false, "");
-    testTransform("{'a.$': 1}", "{'a.b': 1, 'a.c': 2}", "{a: [{b: 1}, {c: 2}]}", false, "");
 }
 
 //
diff --git a/src/mongo/db/exec/update.cpp b/src/mongo/db/exec/update.cpp
index 7df6d43c3f6..b5dbe6eff99 100644
--- a/src/mongo/db/exec/update.cpp
+++ b/src/mongo/db/exec/update.cpp
@@ -508,10 +508,6 @@ BSONObj UpdateStage::transformAndUpdate(const Snapshotted<BSONObj>& oldObj, Reco
         dassert(cq);
         verify(cq->root()->matchesBSON(oldObj.value(), &matchDetails));
 
-        // If we have matched more than one array position, we cannot perform a positional update
-        // operation.
-        uassert(34411, "ambiguous positional update operation", matchDetails.isValid());
-
         string matchedField;
         if (matchDetails.hasElemMatchKey())
             matchedField = matchDetails.elemMatchKey();
diff --git a/src/mongo/db/matcher/expression.h b/src/mongo/db/matcher/expression.h
index a0ae4114991..59e18608cc9 100644
--- a/src/mongo/db/matcher/expression.h
+++ b/src/mongo/db/matcher/expression.h
@@ -189,16 +189,12 @@ public:
     // XXX document
     virtual bool equivalent(const MatchExpression* other) const = 0;
 
-    /**
-    * Determine if a document satisfies the tree-predicate.
-    *
-    * The caller may optionally provide a non-null MatchDetails as an out-parameter. For matching
-    *documents, the MatchDetails provide further info on how the document was
-    *matched---specifically, which array element matched an array predicate.
-    *
-    * The caller must check that the MatchDetails is valid via the isValid() method before using.
-    */
+    //
+    // Determine if a document satisfies the tree-predicate.
+    //
+
     virtual bool matches(const MatchableDocument* doc, MatchDetails* details = 0) const = 0;
+
     virtual bool matchesBSON(const BSONObj& doc, MatchDetails* details = 0) const;
 
     /**
diff --git a/src/mongo/db/matcher/match_details.cpp b/src/mongo/db/matcher/match_details.cpp
index 3d221a91562..6675bf08e15 100644
--- a/src/mongo/db/matcher/match_details.cpp
+++ b/src/mongo/db/matcher/match_details.cpp
@@ -38,7 +38,7 @@ namespace mongo {
 
 using std::string;
 
-MatchDetails::MatchDetails() : _elemMatchKeyRequested(false), _isValid(true) {
+MatchDetails::MatchDetails() : _elemMatchKeyRequested() {
     resetOutput();
 }
 
@@ -58,9 +58,6 @@ std::string MatchDetails::elemMatchKey() const {
 
 void MatchDetails::setElemMatchKey(const std::string& elemMatchKey) {
     if (_elemMatchKeyRequested) {
-        if (_elemMatchKey) {
-            _isValid = false;
-        }
         _elemMatchKey.reset(new std::string(elemMatchKey));
     }
 }
diff --git a/src/mongo/db/matcher/match_details.h b/src/mongo/db/matcher/match_details.h
index aaaa64e4ccd..15a0a606678 100644
--- a/src/mongo/db/matcher/match_details.h
+++ b/src/mongo/db/matcher/match_details.h
@@ -73,14 +73,9 @@ public:
 
     void setElemMatchKey(const std::string& elemMatchKey);
 
-    bool isValid() const {
-        return _isValid;
-    }
-
 private:
     bool _loadedRecord;
     bool _elemMatchKeyRequested;
-    bool _isValid;
     std::unique_ptr<std::string> _elemMatchKey;
 };
 }