diff options
author | Ted Tuckman <ted.tuckman@mongodb.com> | 2020-10-06 10:50:17 -0400 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2020-10-15 14:14:00 +0000 |
commit | 16b59bd6236a90ad3d807a66ec2fe529e09cf0ec (patch) | |
tree | c7e2025f51fde1118f9dd997fcfa81769ecd6683 /src | |
parent | 122e9e8fe5db8f4b1bfcc2f30e2f574045d19547 (diff) | |
download | mongo-16b59bd6236a90ad3d807a66ec2fe529e09cf0ec.tar.gz |
SERVER-50778 Compare array indexes numerically for updates
Diffstat (limited to 'src')
-rw-r--r-- | src/mongo/db/update/path_support.h | 22 | ||||
-rw-r--r-- | src/mongo/db/update/update_array_node.h | 2 | ||||
-rw-r--r-- | src/mongo/db/update/update_internal_node.cpp | 11 | ||||
-rw-r--r-- | src/mongo/db/update/update_internal_node.h | 10 | ||||
-rw-r--r-- | src/mongo/db/update/update_object_node.h | 5 | ||||
-rw-r--r-- | src/mongo/db/update/update_object_node_test.cpp | 2 |
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}}}")); } |