SERVER-28758 Add appendPart and removeLastPart operations to FieldRef

4 files changed, 593 insertions, 29 deletions

diff --git a/src/mongo/db/field_ref.cpp b/src/mongo/db/field_ref.cpp
index e3bc5305c0b..3a759febe3c 100644
--- a/src/mongo/db/field_ref.cpp
+++ b/src/mongo/db/field_ref.cpp
@@ -34,7 +34,9 @@
 
 namespace mongo {
 
-FieldRef::FieldRef() : _size(0) {}
+const size_t FieldRef::kReserveAhead;
+
+FieldRef::FieldRef() : _size(0), _cachedSize(0) {}
 
 FieldRef::FieldRef(StringData path) : _size(0) {
     parse(path);
@@ -74,9 +76,9 @@ void FieldRef::parse(StringData path) {
         // instead reach the break statement.
 
         if (cur != beg)
-            appendPart(StringData(&*beg, cur - beg));
+            appendParsedPart(StringData(&*beg, cur - beg));
         else
-            appendPart(StringData());
+            appendParsedPart(StringData());
 
         if (cur != end) {
             beg = ++cur;
@@ -90,25 +92,59 @@ void FieldRef::parse(StringData path) {
 void FieldRef::setPart(size_t i, StringData part) {
     dassert(i < _size);
 
-    if (_replacements.size() != _size) {
+    if (_replacements.empty()) {
         _replacements.resize(_size);
     }
 
     _replacements[i] = part.toString();
     if (i < kReserveAhead) {
-        _fixed[i] = _replacements[i];
+        _fixed[i] = boost::none;
     } else {
-        _variable[getIndex(i)] = _replacements[i];
+        _variable[getIndex(i)] = boost::none;
+    }
+}
+
+void FieldRef::appendPart(StringData part) {
+    if (_replacements.empty()) {
+        _replacements.resize(_size);
+    }
+
+    _replacements.push_back(part.toString());
+
+    if (_size < kReserveAhead) {
+        _fixed[_size] = boost::none;
+    } else {
+        _variable.push_back(boost::none);
+    }
+    _size++;
+}
+
+void FieldRef::removeLastPart() {
+    if (_size == 0) {
+        return;
+    }
+
+    if (!_replacements.empty()) {
+        _replacements.pop_back();
     }
+
+    if (_size > kReserveAhead) {
+        dassert(_variable.size() == _size - kReserveAhead);
+        _variable.pop_back();
+    }
+
+    _size--;
 }
 
-size_t FieldRef::appendPart(StringData part) {
+size_t FieldRef::appendParsedPart(StringData part) {
     if (_size < kReserveAhead) {
         _fixed[_size] = part;
     } else {
         _variable.push_back(part);
     }
-    return ++_size;
+    _size++;
+    _cachedSize++;
+    return _size;
 }
 
 void FieldRef::reserialize() const {
@@ -116,7 +152,7 @@ void FieldRef::reserialize() const {
     // Reserve some space in the string. We know we will have, at minimum, a character for
     // each component we are writing, and a dot for each component, less one. We don't want
     // to reserve more, since we don't want to forfeit the SSO if it is applicable.
-    nextDotted.reserve((_size * 2) - 1);
+    nextDotted.reserve((_size > 0) ? (_size * 2) - 1 : 0);
 
     // Concatenate the fields to a new string
     for (size_t i = 0; i != _size; ++i) {
@@ -129,13 +165,24 @@ void FieldRef::reserialize() const {
     // Make the new string our contents
     _dotted.swap(nextDotted);
 
+    // Before we reserialize, it's possible that _cachedSize != _size because parts were added or
+    // removed. This reserialization process reconciles the components in our cached string
+    // (_dotted) with the modified path.
+    _cachedSize = _size;
+
     // Fixup the parts to refer to the new string
     std::string::const_iterator where = _dotted.begin();
     const std::string::const_iterator end = _dotted.end();
     for (size_t i = 0; i != _size; ++i) {
-        StringData& part = (i < kReserveAhead) ? _fixed[i] : _variable[getIndex(i)];
-        const size_t size = part.size();
-        part = StringData(&*where, size);
+        boost::optional<StringData>& part =
+            (i < kReserveAhead) ? _fixed[i] : _variable[getIndex(i)];
+        const size_t size = part ? part.get().size() : _replacements[i].size();
+
+        // There is one case where we expect to see the "where" iterator to be at "end" here: we
+        // are at the last part of the FieldRef and that part is the empty string. In that case, we
+        // need to make sure we do not dereference the "where" iterator.
+        dassert(where != end || (size == 0 && i == _size - 1));
+        part = StringData((size > 0) ? &*where : nullptr, size);
         where += size;
         // skip over '.' unless we are at the end.
         if (where != end) {
@@ -151,10 +198,12 @@ void FieldRef::reserialize() const {
 StringData FieldRef::getPart(size_t i) const {
     dassert(i < _size);
 
-    if (i < kReserveAhead) {
-        return _fixed[i];
+    const boost::optional<StringData>& part =
+        (i < kReserveAhead) ? _fixed[i] : _variable[getIndex(i)];
+    if (part) {
+        return part.get();
     } else {
-        return _variable[getIndex(i)];
+        return StringData(_replacements[i]);
     }
 }
 
@@ -199,9 +248,9 @@ StringData FieldRef::dottedSubstring(size_t startPart, size_t endPart) const {
     if (_size == 0 || startPart >= endPart || endPart > numParts())
         return StringData();
 
-    if (!_replacements.empty())
+    if (!_replacements.empty() || _size != _cachedSize)
         reserialize();
-    dassert(_replacements.empty());
+    dassert(_replacements.empty() && _size == _cachedSize);
 
     StringData result(_dotted);
 
@@ -271,6 +320,7 @@ int FieldRef::compare(const FieldRef& other) const {
 
 void FieldRef::clear() {
     _size = 0;
+    _cachedSize = 0;
     _variable.clear();
     _dotted.clear();
     _replacements.clear();
diff --git a/src/mongo/db/field_ref.h b/src/mongo/db/field_ref.h
index b6b218630b8..fe185b093e3 100644
--- a/src/mongo/db/field_ref.h
+++ b/src/mongo/db/field_ref.h
@@ -28,6 +28,7 @@
 
 #pragma once
 
+#include <boost/optional.hpp>
 #include <iosfwd>
 #include <string>
 #include <vector>
@@ -73,6 +74,17 @@ public:
     void setPart(size_t i, StringData part);
 
     /**
+     * Adds a new field to the end of the path, increasing its size by 1.
+     */
+    void appendPart(StringData part);
+
+    /**
+     * Removes the last part from the path, decreasing its size by 1. Has no effect on a
+     * FieldRef with size 0.
+     */
+    void removeLastPart();
+
+    /**
      * Returns the 'i-th' field part. Assumes i < size(). Behavior is undefined otherwise.
      */
     StringData getPart(size_t i) const;
@@ -144,10 +156,10 @@ private:
     }
 
     /**
-     * Returns the new number of parts after appending 'part' to this field path. It
-     * assumes that 'part' is pointing to an internally allocated area.
+     * Returns the new number of parts after appending 'part' to this field path. This is
+     * private, because it is only intended for use by the parse function.
      */
-    size_t appendPart(StringData part);
+    size_t appendParsedPart(StringData part);
 
     /**
      * Re-assemble _dotted from components, including any replacements in _replacements,
@@ -161,16 +173,30 @@ private:
     // number of field parts stored
     size_t _size;
 
-    // first kResevedAhead field components
-    mutable StringData _fixed[kReserveAhead];
+    // Number of field parts in the cached dotted name (_dotted).
+    mutable size_t _cachedSize;
 
-    // remaining field components
-    mutable std::vector<StringData> _variable;
+    /**
+     * First kResevedAhead field components.
+     *
+     * Each component is either a StringData backed by the _dotted string or boost::none
+     * to indicate that getPart() should read the string from the _replacements list.
+     */
+    mutable boost::optional<StringData> _fixed[kReserveAhead];
+
+    // Remaining field components. (See comment above _fixed.)
+    mutable std::vector<boost::optional<StringData>> _variable;
 
-    // cached dotted name
+    /**
+     * Cached copy of the complete dotted name string. The StringData objects in "_fixed"
+     * and "_variable" reference this string.
+     */
     mutable std::string _dotted;
 
-    // back memory added with the setPart call pointed to by _fized and _variable
+    /**
+     * String storage for path parts that have been replaced with setPart() or added with
+     * appendPart() since the lasted time "_dotted" was materialized.
+     */
     mutable std::vector<std::string> _replacements;
 };
 
diff --git a/src/mongo/db/field_ref_test.cpp b/src/mongo/db/field_ref_test.cpp
index 26497d32840..fe28c5fc5e1 100644
--- a/src/mongo/db/field_ref_test.cpp
+++ b/src/mongo/db/field_ref_test.cpp
@@ -319,4 +319,475 @@ TEST(DottedSubstring, Nested) {
     ASSERT_EQUALS("", path.dottedSubstring(1, path.numParts() - 5));
 }
 
+// The "short" append tests operate entirely in "reserve" space.
+TEST(AppendShort, Simple) {
+    FieldRef path("a.b");
+    ASSERT_EQUALS(2u, path.numParts());
+
+    path.appendPart("c");
+    ASSERT_EQUALS(3u, path.numParts());
+    ASSERT_EQUALS("a.b.c", path.dottedField());
+}
+
+TEST(AppendShort, AppendAndReplace1) {
+    FieldRef path("a.b");
+    ASSERT_EQUALS(2u, path.numParts());
+
+    path.appendPart("c");
+    path.setPart(1, "0");
+    ASSERT_EQUALS(3u, path.numParts());
+    ASSERT_EQUALS("a.0.c", path.dottedField());
+}
+
+TEST(AppendShort, AppendAndReplace2) {
+    FieldRef path("a.b");
+    ASSERT_EQUALS(2u, path.numParts());
+
+    path.appendPart("c");
+    path.setPart(2, "0");
+    ASSERT_EQUALS(3u, path.numParts());
+    ASSERT_EQUALS("a.b.0", path.dottedField());
+}
+
+TEST(AppendShort, ReplaceAndAppend) {
+    FieldRef path("a.b");
+    ASSERT_EQUALS(2u, path.numParts());
+
+    path.setPart(1, "0");
+    path.appendPart("c");
+    ASSERT_EQUALS(3u, path.numParts());
+    ASSERT_EQUALS("0", path.getPart(1));
+    ASSERT_EQUALS("a.0.c", path.dottedField());
+}
+
+TEST(AppendShort, AppendAndGetPart) {
+    FieldRef path("a.b");
+    ASSERT_EQUALS(2u, path.numParts());
+
+    path.appendPart("c");
+    path.setPart(1, "0");
+    ASSERT_EQUALS("a", path.getPart(0));
+    ASSERT_EQUALS("0", path.getPart(1));
+    ASSERT_EQUALS("c", path.getPart(2));
+}
+
+TEST(AppendShort, AppendEmptyPart) {
+    FieldRef path("a.b");
+    ASSERT_EQUALS(2u, path.numParts());
+
+    path.appendPart("");
+    ASSERT_EQUALS(3u, path.numParts());
+    ASSERT_EQUALS("", path.getPart(2));
+    ASSERT_EQUALS("a.b.", path.dottedField());
+}
+
+TEST(AppendShort, SetEmptyPartThenAppend) {
+    FieldRef path("a.b");
+    ASSERT_EQUALS(2u, path.numParts());
+
+    path.setPart(1, "");
+    path.appendPart("c");
+    ASSERT_EQUALS(3u, path.numParts());
+    ASSERT_EQUALS("a..c", path.dottedField());
+    ASSERT_EQUALS("", path.getPart(1));
+}
+
+// The "medium" append tests feature an append operation that spills out of reserve space (i.e.,
+// we append to a path that has _size == kReserveAhead).
+TEST(AppendMedium, Simple) {
+    FieldRef path("a.b.c.d");
+    ASSERT_EQUALS(4u, path.numParts());
+
+    path.appendPart("e");
+    ASSERT_EQUALS(5u, path.numParts());
+    ASSERT_EQUALS("a.b.c.d.e", path.dottedField());
+}
+
+TEST(AppendMedium, AppendAndReplace1) {
+    FieldRef path("a.b.c.d");
+    ASSERT_EQUALS(4u, path.numParts());
+
+    path.appendPart("e");
+    path.setPart(1, "0");
+    ASSERT_EQUALS(5u, path.numParts());
+    ASSERT_EQUALS("a.0.c.d.e", path.dottedField());
+}
+
+TEST(AppendMedium, AppendAndReplace2) {
+    FieldRef path("a.b.c.d");
+    ASSERT_EQUALS(4u, path.numParts());
+
+    path.appendPart("e");
+    path.setPart(4, "0");
+    ASSERT_EQUALS(5u, path.numParts());
+    ASSERT_EQUALS("a.b.c.d.0", path.dottedField());
+}
+
+TEST(AppendMedium, ReplaceAndAppend) {
+    FieldRef path("a.b.c.d");
+    ASSERT_EQUALS(4u, path.numParts());
+
+    path.setPart(1, "0");
+    path.appendPart("e");
+    ASSERT_EQUALS(5u, path.numParts());
+    ASSERT_EQUALS("0", path.getPart(1));
+    ASSERT_EQUALS("a.0.c.d.e", path.dottedField());
+}
+
+TEST(AppendMedium, AppendAndGetPart) {
+    FieldRef path("a.b.c.d");
+    ASSERT_EQUALS(4u, path.numParts());
+
+    path.appendPart("e");
+    path.setPart(1, "0");
+    ASSERT_EQUALS("a", path.getPart(0));
+    ASSERT_EQUALS("0", path.getPart(1));
+    ASSERT_EQUALS("c", path.getPart(2));
+    ASSERT_EQUALS("d", path.getPart(3));
+    ASSERT_EQUALS("e", path.getPart(4));
+}
+
+TEST(AppendMedium, AppendEmptyPart) {
+    FieldRef path("a.b.c.d");
+    ASSERT_EQUALS(4u, path.numParts());
+
+    path.appendPart("");
+    ASSERT_EQUALS(5u, path.numParts());
+    ASSERT_EQUALS("", path.getPart(4));
+    ASSERT_EQUALS("a.b.c.d.", path.dottedField());
+}
+
+TEST(AppendMedium, SetEmptyPartThenAppend) {
+    FieldRef path("a.b.c.d");
+    ASSERT_EQUALS(4u, path.numParts());
+
+    path.setPart(1, "");
+    path.appendPart("e");
+    ASSERT_EQUALS(5u, path.numParts());
+    ASSERT_EQUALS("a..c.d.e", path.dottedField());
+    ASSERT_EQUALS("", path.getPart(1));
+}
+
+// The "long" append tests have paths that are bigger than the reserve space throughout their life
+// cycle.
+TEST(AppendLong, Simple) {
+    FieldRef path("a.b.c.d.e.f");
+    ASSERT_EQUALS(6u, path.numParts());
+
+    path.appendPart("g");
+    path.appendPart("h");
+    path.appendPart("i");
+    ASSERT_EQUALS(9u, path.numParts());
+    ASSERT_EQUALS("a.b.c.d.e.f.g.h.i", path.dottedField());
+}
+
+TEST(AppendLong, AppendAndReplace1) {
+    FieldRef path("a.b.c.d.e.f");
+    ASSERT_EQUALS(6u, path.numParts());
+
+    path.appendPart("g");
+    path.appendPart("h");
+    path.appendPart("i");
+    path.setPart(1, "0");
+    path.setPart(5, "1");
+    ASSERT_EQUALS(9u, path.numParts());
+    ASSERT_EQUALS("a.0.c.d.e.1.g.h.i", path.dottedField());
+}
+
+TEST(AppendLong, AppendAndReplace2) {
+    FieldRef path("a.b.c.d.e.f");
+    ASSERT_EQUALS(6u, path.numParts());
+
+    path.appendPart("g");
+    path.appendPart("h");
+    path.appendPart("i");
+    path.setPart(7, "0");
+    ASSERT_EQUALS(9u, path.numParts());
+    ASSERT_EQUALS("a.b.c.d.e.f.g.0.i", path.dottedField());
+}
+
+TEST(AppendLong, ReplaceAndAppend) {
+    FieldRef path("a.b.c.d.e.f");
+    ASSERT_EQUALS(6u, path.numParts());
+
+    path.setPart(1, "0");
+    path.setPart(5, "1");
+    path.appendPart("g");
+    path.appendPart("h");
+    path.appendPart("i");
+    ASSERT_EQUALS(9u, path.numParts());
+    ASSERT_EQUALS("0", path.getPart(1));
+    ASSERT_EQUALS("1", path.getPart(5));
+    ASSERT_EQUALS("a.0.c.d.e.1.g.h.i", path.dottedField());
+}
+
+TEST(AppendLong, AppendAndGetPart) {
+    FieldRef path("a.b.c.d.e.f");
+    ASSERT_EQUALS(6u, path.numParts());
+
+    path.appendPart("g");
+    path.appendPart("h");
+    path.appendPart("i");
+    path.setPart(1, "0");
+    path.setPart(5, "1");
+    path.setPart(7, "2");
+    ASSERT_EQUALS("a", path.getPart(0));
+    ASSERT_EQUALS("0", path.getPart(1));
+    ASSERT_EQUALS("c", path.getPart(2));
+    ASSERT_EQUALS("d", path.getPart(3));
+    ASSERT_EQUALS("e", path.getPart(4));
+    ASSERT_EQUALS("1", path.getPart(5));
+    ASSERT_EQUALS("g", path.getPart(6));
+    ASSERT_EQUALS("2", path.getPart(7));
+    ASSERT_EQUALS("i", path.getPart(8));
+}
+
+TEST(AppendLong, AppendEmptyPart) {
+    FieldRef path("a.b.c.d.e.f");
+    ASSERT_EQUALS(6u, path.numParts());
+
+    path.appendPart("");
+    ASSERT_EQUALS(7u, path.numParts());
+    ASSERT_EQUALS("", path.getPart(6));
+    ASSERT_EQUALS("a.b.c.d.e.f.", path.dottedField());
+}
+
+TEST(AppendLong, SetEmptyPartThenAppend) {
+    FieldRef path("a.b.c.d.e.f");
+    ASSERT_EQUALS(6u, path.numParts());
+
+    path.setPart(1, "");
+    path.setPart(4, "");
+    path.appendPart("g");
+    ASSERT_EQUALS(7u, path.numParts());
+    ASSERT_EQUALS("a..c.d..f.g", path.dottedField());
+    ASSERT_EQUALS("", path.getPart(1));
+    ASSERT_EQUALS("", path.getPart(4));
+}
+
+// The "short" removeLastPart tests operate entirely in "reserve" space.
+TEST(RemoveLastPartShort, Simple) {
+    FieldRef path("a.b");
+    ASSERT_EQUALS(2u, path.numParts());
+
+    path.removeLastPart();
+    ASSERT_EQUALS(1u, path.numParts());
+    ASSERT_EQUALS("a", path.dottedField());
+}
+
+TEST(RemoveLastPartShort, RemoveUntilEmpty) {
+    FieldRef path("a.b");
+    ASSERT_EQUALS(2u, path.numParts());
+
+    path.removeLastPart();
+    path.removeLastPart();
+    ASSERT_EQUALS(0u, path.numParts());
+    ASSERT_EQUALS("", path.dottedField());
+}
+
+TEST(RemoveLastPartShort, AppendThenRemove) {
+    FieldRef path("a.b");
+    ASSERT_EQUALS(2u, path.numParts());
+
+    path.appendPart("c");
+    path.removeLastPart();
+    ASSERT_EQUALS(2u, path.numParts());
+    ASSERT_EQUALS("a.b", path.dottedField());
+}
+
+TEST(RemoveLastPartShort, RemoveThenAppend) {
+    FieldRef path("a.b");
+    ASSERT_EQUALS(2u, path.numParts());
+
+    path.removeLastPart();
+    path.appendPart("b");
+    ASSERT_EQUALS(2u, path.numParts());
+    ASSERT_EQUALS("a.b", path.dottedField());
+}
+
+TEST(RemoveLastPartShort, RemoveThenSetPart) {
+    FieldRef path("a.b");
+    ASSERT_EQUALS(2u, path.numParts());
+
+    path.removeLastPart();
+    path.setPart(0, "0");
+    ASSERT_EQUALS(1u, path.numParts());
+    ASSERT_EQUALS("0", path.dottedField());
+}
+
+TEST(RemoveLastPartShort, SetPartThenRemove) {
+    FieldRef path("a.b");
+    ASSERT_EQUALS(2u, path.numParts());
+
+    path.setPart(0, "0");
+    path.setPart(1, "1");
+    path.removeLastPart();
+    ASSERT_EQUALS(1u, path.numParts());
+    ASSERT_EQUALS("0", path.dottedField());
+}
+
+TEST(RemoveLastPartShort, AppendThenSetPartThenRemove) {
+    FieldRef path("a.b");
+    ASSERT_EQUALS(2u, path.numParts());
+
+    path.appendPart("c");
+    path.setPart(2, "0");
+    path.removeLastPart();
+    ASSERT_EQUALS(2u, path.numParts());
+    ASSERT_EQUALS("a.b", path.dottedField());
+}
+
+// The "medium" removeLastPart tests feature paths that change in size during the test so that they
+// transition between fitting and not fitting in "reserve" space.
+TEST(RemoveLastPartMedium, Simple) {
+    FieldRef path("a.b.c.d.e");
+    ASSERT_EQUALS(5u, path.numParts());
+
+    path.removeLastPart();
+    ASSERT_EQUALS(4u, path.numParts());
+    ASSERT_EQUALS("a.b.c.d", path.dottedField());
+}
+
+TEST(RemoveLastPartMedium, RemoveUntilEmpty) {
+    FieldRef path("a.b.c.d.e");
+    ASSERT_EQUALS(5u, path.numParts());
+
+    while (path.numParts() > 0) {
+        path.removeLastPart();
+    }
+    ASSERT_EQUALS(0u, path.numParts());
+    ASSERT_EQUALS("", path.dottedField());
+}
+
+TEST(RemoveLastPartMedium, AppendThenRemove) {
+    FieldRef path("a.b.c.d");
+    ASSERT_EQUALS(4u, path.numParts());
+
+    path.appendPart("e");
+    path.removeLastPart();
+    ASSERT_EQUALS(4u, path.numParts());
+    ASSERT_EQUALS("a.b.c.d", path.dottedField());
+}
+
+TEST(RemoveLastPartMedium, RemoveThenAppend) {
+    FieldRef path("a.b.c.d.e");
+    ASSERT_EQUALS(5u, path.numParts());
+
+    path.removeLastPart();
+    path.appendPart("e");
+    ASSERT_EQUALS(5u, path.numParts());
+    ASSERT_EQUALS("a.b.c.d.e", path.dottedField());
+}
+
+TEST(RemoveLastPartMedium, RemoveThenSetPart) {
+    FieldRef path("a.b.c.d.e");
+    ASSERT_EQUALS(5u, path.numParts());
+
+    path.removeLastPart();
+    path.setPart(0, "0");
+    ASSERT_EQUALS(4u, path.numParts());
+    ASSERT_EQUALS("0.b.c.d", path.dottedField());
+}
+
+TEST(RemoveLastPartMedium, SetPartThenRemove) {
+    FieldRef path("a.b.c.d.e");
+    ASSERT_EQUALS(5u, path.numParts());
+
+    path.setPart(0, "0");
+    path.setPart(4, "1");
+    path.removeLastPart();
+    ASSERT_EQUALS(4u, path.numParts());
+    ASSERT_EQUALS("0.b.c.d", path.dottedField());
+}
+
+TEST(RemoveLastPartMedium, AppendThenSetPartThenRemove) {
+    FieldRef path("a.b.c.d");
+    ASSERT_EQUALS(4u, path.numParts());
+
+    path.appendPart("c");
+    path.setPart(2, "0");
+    path.setPart(4, "1");
+    path.removeLastPart();
+    ASSERT_EQUALS(4u, path.numParts());
+    ASSERT_EQUALS("a.b.0.d", path.dottedField());
+}
+
+// The "long" removeLastPart tests have paths that are bigger than the reserve space throughout
+// their life cycle (with the exception of RemoveUntilempty).
+TEST(RemoveLastPartLong, Simple) {
+    FieldRef path("a.b.c.d.e.f.g");
+    ASSERT_EQUALS(7u, path.numParts());
+
+    path.removeLastPart();
+    ASSERT_EQUALS(6u, path.numParts());
+    ASSERT_EQUALS("a.b.c.d.e.f", path.dottedField());
+}
+
+TEST(RemoveLastPartLong, RemoveUntilEmpty) {
+    FieldRef path("a.b.c.d.e.f.g");
+    ASSERT_EQUALS(7u, path.numParts());
+
+    while (path.numParts() > 0) {
+        path.removeLastPart();
+    }
+    ASSERT_EQUALS(0u, path.numParts());
+    ASSERT_EQUALS("", path.dottedField());
+}
+
+TEST(RemoveLastPartLong, AppendThenRemove) {
+    FieldRef path("a.b.c.d.e.f");
+    ASSERT_EQUALS(6u, path.numParts());
+
+    path.appendPart("g");
+    path.removeLastPart();
+    ASSERT_EQUALS(6u, path.numParts());
+    ASSERT_EQUALS("a.b.c.d.e.f", path.dottedField());
+}
+
+TEST(RemoveLastPartLong, RemoveThenAppend) {
+    FieldRef path("a.b.c.d.e.f.g");
+    ASSERT_EQUALS(7u, path.numParts());
+
+    path.removeLastPart();
+    path.appendPart("g");
+    ASSERT_EQUALS(7u, path.numParts());
+    ASSERT_EQUALS("a.b.c.d.e.f.g", path.dottedField());
+}
+
+TEST(RemoveLastPartLong, RemoveThenSetPart) {
+    FieldRef path("a.b.c.d.e.f.g");
+    ASSERT_EQUALS(7u, path.numParts());
+
+    path.removeLastPart();
+    path.setPart(0, "0");
+    path.setPart(4, "1");
+    ASSERT_EQUALS(6u, path.numParts());
+    ASSERT_EQUALS("0.b.c.d.1.f", path.dottedField());
+}
+
+TEST(RemoveLastPartLong, SetPartThenRemove) {
+    FieldRef path("a.b.c.d.e.f.g");
+    ASSERT_EQUALS(7u, path.numParts());
+
+    path.setPart(0, "0");
+    path.setPart(4, "1");
+    path.setPart(6, "2");
+    path.removeLastPart();
+    ASSERT_EQUALS(6u, path.numParts());
+    ASSERT_EQUALS("0.b.c.d.1.f", path.dottedField());
+}
+
+TEST(RemoveLastPartLong, AppendThenSetPartThenRemove) {
+    FieldRef path("a.b.c.d.e.f");
+    ASSERT_EQUALS(6u, path.numParts());
+
+    path.appendPart("g");
+    path.setPart(2, "0");
+    path.setPart(4, "1");
+    path.setPart(6, "2");
+    path.removeLastPart();
+    ASSERT_EQUALS(6u, path.numParts());
+    ASSERT_EQUALS("a.b.0.d.1.f", path.dottedField());
+}
+
 }  // namespace
diff --git a/src/mongo/db/update/update_object_node.cpp b/src/mongo/db/update/update_object_node.cpp
index beb0850828c..5217e4077eb 100644
--- a/src/mongo/db/update/update_object_node.cpp
+++ b/src/mongo/db/update/update_object_node.cpp
@@ -130,6 +130,24 @@ StatusWith<bool> UpdateObjectNode::parseAndMerge(UpdateObjectNode* root,
 
 namespace {
 /**
+ * Helper class for appending to a FieldRef for the duration of the current scope and then restoring
+ * the FieldRef at the end of the scope.
+ */
+class FieldRefTempAppend {
+public:
+    FieldRefTempAppend(FieldRef& fieldRef, const std::string& part) : _fieldRef(fieldRef) {
+        _fieldRef.appendPart(part);
+    }
+
+    ~FieldRefTempAppend() {
+        _fieldRef.removeLastPart();
+    }
+
+private:
+    FieldRef& _fieldRef;
+};
+
+/**
  * Helper for when performMerge wants to create a merged child from children that exist in two
  * merging nodes. If there is only one child (leftNode or rightNode is NULL), we clone it. If there
  * are two different children, we merge them recursively. If there are no children (leftNode and
@@ -146,9 +164,8 @@ std::unique_ptr<UpdateNode> copyOrMergeAsNecessary(UpdateNode* leftNode,
     } else if (!rightNode) {
         return leftNode->clone();
     } else {
-        std::unique_ptr<FieldRef> updatedFieldRef(
-            new FieldRef(pathTaken->dottedField() + "." + nextField));
-        return UpdateNode::createUpdateNodeByMerging(*leftNode, *rightNode, updatedFieldRef.get());
+        FieldRefTempAppend tempAppend(*pathTaken, nextField);
+        return UpdateNode::createUpdateNodeByMerging(*leftNode, *rightNode, pathTaken);
     }
 }
 }