SERVER-50778 Compare array indexes numerically for updates

6 files changed, 41 insertions, 11 deletions

diff --git a/src/mongo/db/update/path_support.h b/src/mongo/db/update/path_support.h
index 808f2df5dff..c6df4afe3c8 100644
--- a/src/mongo/db/update/path_support.h
+++ b/src/mongo/db/update/path_support.h
@@ -32,6 +32,7 @@
 #include <cstdint>
 #include <string>
 
+#include "mongo/base/parse_number.h"
 #include "mongo/base/status.h"
 #include "mongo/bson/mutable/element.h"
 #include "mongo/db/field_ref.h"
@@ -50,6 +51,27 @@ static const size_t kMaxPaddingAllowed = 1500000;
 // Convenience type to hold equality matches at particular paths from a MatchExpression
 typedef std::map<StringData, const EqualityMatchExpression*> EqualityMatches;
 
+struct cmpPathsAndArrayIndexes {
+    // Assumes paths are valid.
+    bool operator()(const std::string& a, const std::string& b) const {
+        NumberParser parser;
+        long numA;
+        long numB;
+        auto status = parser(a, &numA);
+        if (!status.isOK())
+            return a < b;
+        status = parser(b, &numB);
+        if (!status.isOK())
+            return a < b;
+        // If the numbers are the same, and the strings are the same, 'numA' < 'numB' is equivalent
+        // to 'a' < 'b'. However, the strings could still be different (for example, "1" and "01")
+        // and we need to treat them differently.
+        if (numA == numB)
+            return a < b;
+        return numA < numB;
+    }
+};
+
 /**
  * Finds the longest portion of 'prefix' that exists in document rooted at 'root' and is
  * "viable." A viable path is one that, if fully created on a given doc, would not
diff --git a/src/mongo/db/update/update_array_node.h b/src/mongo/db/update/update_array_node.h
index c6e90c1d9c3..ff48b8dba59 100644
--- a/src/mongo/db/update/update_array_node.h
+++ b/src/mongo/db/update/update_array_node.h
@@ -99,7 +99,7 @@ public:
 
 private:
     const std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>>& _arrayFilters;
-    std::map<std::string, clonable_ptr<UpdateNode>> _children;
+    std::map<std::string, clonable_ptr<UpdateNode>, pathsupport::cmpPathsAndArrayIndexes> _children;
 
     // When calling apply() causes us to merge elements of '_children', we store the result of the
     // merge in case we need it for another array element or document.
diff --git a/src/mongo/db/update/update_internal_node.cpp b/src/mongo/db/update/update_internal_node.cpp
index 95cc60eda8e..fe5e14ab336 100644
--- a/src/mongo/db/update/update_internal_node.cpp
+++ b/src/mongo/db/update/update_internal_node.cpp
@@ -34,12 +34,15 @@
 namespace mongo {
 
 // static
-std::map<std::string, clonable_ptr<UpdateNode>> UpdateInternalNode::createUpdateNodeMapByMerging(
-    const std::map<std::string, clonable_ptr<UpdateNode>>& leftMap,
-    const std::map<std::string, clonable_ptr<UpdateNode>>& rightMap,
+std::map<std::string, clonable_ptr<UpdateNode>, pathsupport::cmpPathsAndArrayIndexes>
+UpdateInternalNode::createUpdateNodeMapByMerging(
+    const std::map<std::string, clonable_ptr<UpdateNode>, pathsupport::cmpPathsAndArrayIndexes>&
+        leftMap,
+    const std::map<std::string, clonable_ptr<UpdateNode>, pathsupport::cmpPathsAndArrayIndexes>&
+        rightMap,
     FieldRef* pathTaken,
     bool wrapFieldNameAsArrayFilterIdentifier) {
-    std::map<std::string, clonable_ptr<UpdateNode>> mergedMap;
+    std::map<std::string, clonable_ptr<UpdateNode>, pathsupport::cmpPathsAndArrayIndexes> mergedMap;
 
     // Get the union of the field names we know about among the leftMap and rightMap.
     stdx::unordered_set<std::string> allFields;
diff --git a/src/mongo/db/update/update_internal_node.h b/src/mongo/db/update/update_internal_node.h
index 9cadd04fc57..a3403879513 100644
--- a/src/mongo/db/update/update_internal_node.h
+++ b/src/mongo/db/update/update_internal_node.h
@@ -34,6 +34,7 @@
 
 #include "mongo/base/clonable_ptr.h"
 #include "mongo/db/field_ref.h"
+#include "mongo/db/update/path_support.h"
 #include "mongo/db/update/update_node.h"
 
 namespace mongo {
@@ -72,9 +73,12 @@ protected:
      * wrapFieldNameAsArrayFilterIdentifier is true, field names are wrapped as $[<field name>] for
      * error reporting.
      */
-    static std::map<std::string, clonable_ptr<UpdateNode>> createUpdateNodeMapByMerging(
-        const std::map<std::string, clonable_ptr<UpdateNode>>& leftMap,
-        const std::map<std::string, clonable_ptr<UpdateNode>>& rightMap,
+    static std::map<std::string, clonable_ptr<UpdateNode>, pathsupport::cmpPathsAndArrayIndexes>
+    createUpdateNodeMapByMerging(
+        const std::map<std::string, clonable_ptr<UpdateNode>, pathsupport::cmpPathsAndArrayIndexes>&
+            leftMap,
+        const std::map<std::string, clonable_ptr<UpdateNode>, pathsupport::cmpPathsAndArrayIndexes>&
+            rightMap,
         FieldRef* pathTaken,
         bool wrapFieldNameAsArrayFilterIdentifier = false);
 
diff --git a/src/mongo/db/update/update_object_node.h b/src/mongo/db/update/update_object_node.h
index d7f2e56e9de..8c347968e74 100644
--- a/src/mongo/db/update/update_object_node.h
+++ b/src/mongo/db/update/update_object_node.h
@@ -39,6 +39,7 @@
 #include "mongo/bson/bsonelement.h"
 #include "mongo/db/matcher/expression_with_placeholder.h"
 #include "mongo/db/update/modifier_table.h"
+#include "mongo/db/update/path_support.h"
 #include "mongo/db/update/update_internal_node.h"
 #include "mongo/stdx/unordered_map.h"
 
@@ -126,12 +127,12 @@ public:
         visitor->visit(this);
     }
 
-    const std::map<std::string, clonable_ptr<UpdateNode>>& getChildren() const {
+    const auto& getChildren() const {
         return _children;
     }
 
 private:
-    std::map<std::string, clonable_ptr<UpdateNode>> _children;
+    std::map<std::string, clonable_ptr<UpdateNode>, pathsupport::cmpPathsAndArrayIndexes> _children;
     clonable_ptr<UpdateNode> _positionalChild;
 
     // When calling apply() causes us to merge an element of '_children' with '_positionalChild', we
diff --git a/src/mongo/db/update/update_object_node_test.cpp b/src/mongo/db/update/update_object_node_test.cpp
index a8d09f4f891..2e5906e8f30 100644
--- a/src/mongo/db/update/update_object_node_test.cpp
+++ b/src/mongo/db/update/update_object_node_test.cpp
@@ -2565,7 +2565,7 @@ TEST_F(UpdateObjectNodeTest, ApplyMultipleArrayUpdates) {
         doc.getObject());
     ASSERT_FALSE(doc.isInPlaceModeEnabled());
 
-    assertOplogEntry(fromjson("{$set: {'a.10': 10, 'a.2': 2}}"),
+    assertOplogEntry(fromjson("{$set: {'a.2': 2, 'a.10': 10}}"),
                      fromjson("{$v: 2, diff: {sa: {a: true, u2: 2, u10: 10}}}"));
 }