Don't grow a document on $pull/$pullAll. SERVER-6047

3 files changed, 38 insertions, 0 deletions

diff --git a/db/ops/update.cpp b/db/ops/update.cpp
index 4fb8750b0dd..e54e4ef5af8 100644
--- a/db/ops/update.cpp
+++ b/db/ops/update.cpp
@@ -623,6 +623,17 @@ namespace mongo {
 
     template< class Builder >
     void ModSetState::_appendNewFromMods( const string& root , ModState& m , Builder& b , set<string>& onedownseen ) {
+        Mod& m2 = *((Mod*)(m.m)); // HACK
+        switch (m2.op) {
+        // unset/pull/pullAll on nothing does nothing, so don't append anything
+        case Mod::UNSET:
+        case Mod::PULL:
+        case Mod::PULL_ALL:
+            return;
+        default:
+            ;// fall through
+        }
+
         const char * temp = m.fieldName();
         temp += root.size();
         const char * dot = strchr( temp , '.' );
diff --git a/jstests/pull.js b/jstests/pull.js
index cf8147a456c..3cb6328e2de 100644
--- a/jstests/pull.js
+++ b/jstests/pull.js
@@ -17,3 +17,17 @@ t.save( { a: [ 2 ] } );
 t.update( {}, { $pull: { a: 2 } } );
 t.update( {}, { $pull: { a: 6 } } );
 assert.eq( [], t.findOne().a );
+
+// SERVER-6047: $pull creates empty nested docs for dotted fields
+// that don't exist.
+t.drop()
+t.save({ m : 1 } );
+t.update( { m : 1 }, { $pull : { 'a.b' : [ 1 ] } } );
+assert( ('a' in t.findOne()) == false );
+// Non-obvious bit: the implementation of non-in-place update
+// might do different things depending on whether the "new" field
+// comes before or after existing fields in the document.
+// So for now it's worth testing that too. Sorry, future; blame the past.
+t.update( { m : 1 }, { $pull : { 'x.y' : [ 1 ] } } );
+assert( ('z' in t.findOne()) == false );
+// End SERVER-6047  
diff --git a/jstests/pullall.js b/jstests/pullall.js
index 76b1b47a737..7dd932c4bbf 100644
--- a/jstests/pullall.js
+++ b/jstests/pullall.js
@@ -16,3 +16,16 @@ assert.eq( [ 1 ], t.findOne().a );
 t.update( {}, { $pullAll: { a: [ 1, 5 ] } } );
 assert.eq( [], t.findOne().a );
 
+// SERVER-6047: $pullAll creates empty nested docs for dotted fields
+// that don't exist.
+t.drop()
+t.save({ m : 1 } );
+t.update( { m : 1 }, { $pullAll : { 'a.b' : [ 1 ] } } );
+assert( ('a' in t.findOne()) == false );
+// Non-obvious bit: the implementation of non-in-place update
+// might do different things depending on whether the "new" field
+// comes before or after existing fields in the document.
+// So for now it's worth testing that too. Sorry, future; blame the past.
+t.update( { m : 1 }, { $pullAll : { 'x.y' : [ 1 ] } } );
+assert( ('z' in t.findOne()) == false );
+// End SERVER-6047