SERVER-37058: Update with numeric field names inside an array can cause validation to fail

11 files changed, 154 insertions, 189 deletions

diff --git a/src/mongo/db/catalog/collection_info_cache_impl.cpp b/src/mongo/db/catalog/collection_info_cache_impl.cpp
index f653c68921e..d270654fea7 100644
--- a/src/mongo/db/catalog/collection_info_cache_impl.cpp
+++ b/src/mongo/db/catalog/collection_info_cache_impl.cpp
@@ -102,7 +102,7 @@ void CollectionInfoCacheImpl::computeIndexKeys(OperationContext* opCtx) {
                 // If a subtree was specified in the keyPattern, or if an inclusion projection is
                 // present, then we need only index the path(s) preserved by the projection.
                 for (const auto& path : pathProj->getExhaustivePaths()) {
-                    _indexedPaths.addPath(path.dottedField());
+                    _indexedPaths.addPath(path);
                 }
             }
         } else if (descriptor->getAccessMethodName() == IndexNames::TEXT) {
@@ -112,15 +112,15 @@ void CollectionInfoCacheImpl::computeIndexKeys(OperationContext* opCtx) {
                 _indexedPaths.allPathsIndexed();
             } else {
                 for (size_t i = 0; i < ftsSpec.numExtraBefore(); ++i) {
-                    _indexedPaths.addPath(ftsSpec.extraBefore(i));
+                    _indexedPaths.addPath(FieldRef(ftsSpec.extraBefore(i)));
                 }
                 for (fts::Weights::const_iterator it = ftsSpec.weights().begin();
                      it != ftsSpec.weights().end();
                      ++it) {
-                    _indexedPaths.addPath(it->first);
+                    _indexedPaths.addPath(FieldRef(it->first));
                 }
                 for (size_t i = 0; i < ftsSpec.numExtraAfter(); ++i) {
-                    _indexedPaths.addPath(ftsSpec.extraAfter(i));
+                    _indexedPaths.addPath(FieldRef(ftsSpec.extraAfter(i)));
                 }
                 // Any update to a path containing "language" as a component could change the
                 // language of a subdocument.  Add the override field as a path component.
@@ -135,7 +135,7 @@ void CollectionInfoCacheImpl::computeIndexKeys(OperationContext* opCtx) {
             BSONObjIterator j(key);
             while (j.more()) {
                 BSONElement e = j.next();
-                _indexedPaths.addPath(e.fieldName());
+                _indexedPaths.addPath(FieldRef(e.fieldName()));
             }
         }
 
@@ -146,7 +146,7 @@ void CollectionInfoCacheImpl::computeIndexKeys(OperationContext* opCtx) {
             stdx::unordered_set<std::string> paths;
             QueryPlannerIXSelect::getFields(filter, &paths);
             for (auto it = paths.begin(); it != paths.end(); ++it) {
-                _indexedPaths.addPath(*it);
+                _indexedPaths.addPath(FieldRef(*it));
             }
         }
     }
diff --git a/src/mongo/db/field_ref.cpp b/src/mongo/db/field_ref.cpp
index 1bce17b20e7..d6fb5e85f9d 100644
--- a/src/mongo/db/field_ref.cpp
+++ b/src/mongo/db/field_ref.cpp
@@ -251,18 +251,23 @@ size_t FieldRef::commonPrefixSize(const FieldRef& other) const {
     return prefixSize;
 }
 
-bool FieldRef::isNumericPathComponent(StringData component) {
+bool FieldRef::isNumericPathComponentStrict(StringData component) {
     return !component.empty() && !(component.size() > 1 && component[0] == '0') &&
+        FieldRef::isNumericPathComponentLenient(component);
+}
+
+bool FieldRef::isNumericPathComponentLenient(StringData component) {
+    return !component.empty() &&
         std::all_of(component.begin(), component.end(), [](auto c) { return std::isdigit(c); });
 }
 
-bool FieldRef::isNumericPathComponent(size_t i) const {
-    return FieldRef::isNumericPathComponent(getPart(i));
+bool FieldRef::isNumericPathComponentStrict(size_t i) const {
+    return FieldRef::isNumericPathComponentStrict(getPart(i));
 }
 
 bool FieldRef::hasNumericPathComponents() const {
     for (size_t i = 0; i < numParts(); ++i) {
-        if (isNumericPathComponent(i))
+        if (isNumericPathComponentStrict(i))
             return true;
     }
     return false;
@@ -271,7 +276,7 @@ bool FieldRef::hasNumericPathComponents() const {
 std::set<size_t> FieldRef::getNumericPathComponents(size_t startPart) const {
     std::set<size_t> numericPathComponents;
     for (auto i = startPart; i < numParts(); ++i) {
-        if (isNumericPathComponent(i))
+        if (isNumericPathComponentStrict(i))
             numericPathComponents.insert(i);
     }
     return numericPathComponents;
diff --git a/src/mongo/db/field_ref.h b/src/mongo/db/field_ref.h
index 96368b8f547..80ea501e778 100644
--- a/src/mongo/db/field_ref.h
+++ b/src/mongo/db/field_ref.h
@@ -55,7 +55,14 @@ public:
      * Returns true if the argument is a numeric string which is eligible to act as the key name for
      * an element in a BSON array; in other words, the string matches the regex ^(0|[1-9]+[0-9]*)$.
      */
-    static bool isNumericPathComponent(StringData component);
+    static bool isNumericPathComponentStrict(StringData component);
+
+    /**
+     * Similar to the function above except strings that contain leading zero's are considered
+     * numeric. For instance, the above function would return false for an input "01" however this
+     * function will return true.
+     */
+    static bool isNumericPathComponentLenient(StringData component);
 
     FieldRef() = default;
 
@@ -115,7 +122,7 @@ public:
      * the key name for an element in a BSON array; in other words, the fieldname matches the regex
      * ^(0|[1-9]+[0-9]*)$.
      */
-    bool isNumericPathComponent(size_t i) const;
+    bool isNumericPathComponentStrict(size_t i) const;
 
     /**
      * Returns true if this FieldRef has any numeric path components.
diff --git a/src/mongo/db/field_ref_test.cpp b/src/mongo/db/field_ref_test.cpp
index 616b8a4d5cc..7f0f0d4368b 100644
--- a/src/mongo/db/field_ref_test.cpp
+++ b/src/mongo/db/field_ref_test.cpp
@@ -853,11 +853,11 @@ TEST(RemoveLastPartLong, FieldRefFromCopyAssignmentIsADeepCopy) {
 
 TEST(NumericPathComponents, CanIdentifyNumericPathComponents) {
     FieldRef path("a.0.b.1.c");
-    ASSERT(!path.isNumericPathComponent(0));
-    ASSERT(path.isNumericPathComponent(1));
-    ASSERT(!path.isNumericPathComponent(2));
-    ASSERT(path.isNumericPathComponent(3));
-    ASSERT(!path.isNumericPathComponent(4));
+    ASSERT(!path.isNumericPathComponentStrict(0));
+    ASSERT(path.isNumericPathComponentStrict(1));
+    ASSERT(!path.isNumericPathComponentStrict(2));
+    ASSERT(path.isNumericPathComponentStrict(3));
+    ASSERT(!path.isNumericPathComponentStrict(4));
 }
 
 TEST(NumericPathComponents, CanObtainAllNumericPathComponents) {
diff --git a/src/mongo/db/query/planner_access.cpp b/src/mongo/db/query/planner_access.cpp
index 026e994b301..dcbd40c8ee4 100644
--- a/src/mongo/db/query/planner_access.cpp
+++ b/src/mongo/db/query/planner_access.cpp
@@ -571,7 +571,7 @@ void QueryPlannerAccess::finishWildcardIndexScanNode(QuerySolutionNode* node,
     // Helper function to check whether the final path component in 'queryPath' is an array index.
     const auto lastFieldIsArrayIndex = [&multikeyPaths](const auto& queryPath) {
         return (queryPath.numParts() > 1u && multikeyPaths.count(queryPath.numParts() - 2u) &&
-                queryPath.isNumericPathComponent(queryPath.numParts() - 1u));
+                queryPath.isNumericPathComponentStrict(queryPath.numParts() - 1u));
     };
 
     // For [MinKey,MaxKey] bounds, we build a range interval on all subpaths of the query path(s).
diff --git a/src/mongo/db/query/planner_wildcard_helpers.cpp b/src/mongo/db/query/planner_wildcard_helpers.cpp
index 6f44f0eb50c..23335b81741 100644
--- a/src/mongo/db/query/planner_wildcard_helpers.cpp
+++ b/src/mongo/db/query/planner_wildcard_helpers.cpp
@@ -63,7 +63,7 @@ bool fieldNameOrArrayIndexPathMatches(const FieldRef& fieldNameOrArrayIndexPath,
         if (fieldNameOrArrayIndexPath.getPart(i) == staticComparisonPath.getPart(i - offset)) {
             continue;
         } else if (multikeyPathComponents.count(i - 1) &&
-                   fieldNameOrArrayIndexPath.isNumericPathComponent(i)) {
+                   fieldNameOrArrayIndexPath.isNumericPathComponentStrict(i)) {
             ++offset;
             continue;
         }
@@ -105,7 +105,7 @@ std::vector<size_t> findArrayIndexPathComponents(const std::set<std::size_t>& mu
                                                  const FieldRef& queryPath) {
     std::vector<size_t> arrayIndices;
     for (auto i : multikeyPaths) {
-        if (i < queryPath.numParts() - 1 && queryPath.isNumericPathComponent(i + 1)) {
+        if (i < queryPath.numParts() - 1 && queryPath.isNumericPathComponentStrict(i + 1)) {
             arrayIndices.push_back(i + 1);
         }
     }
diff --git a/src/mongo/db/update/modifier_node.cpp b/src/mongo/db/update/modifier_node.cpp
index a85b636ac0d..37135de3b99 100644
--- a/src/mongo/db/update/modifier_node.cpp
+++ b/src/mongo/db/update/modifier_node.cpp
@@ -189,8 +189,7 @@ UpdateNode::ApplyResult ModifierNode::applyToExistingElement(ApplyParams applyPa
 
     ApplyResult applyResult;
 
-    if (!applyParams.indexData ||
-        !applyParams.indexData->mightBeIndexed(applyParams.pathTaken->dottedField())) {
+    if (!applyParams.indexData || !applyParams.indexData->mightBeIndexed(*applyParams.pathTaken)) {
         applyResult.indexesAffected = false;
     }
 
@@ -270,12 +269,12 @@ UpdateNode::ApplyResult ModifierNode::applyToNonexistentElement(ApplyParams appl
         }
 
         invariant(!applyParams.pathToCreate->empty());
-        std::string fullPath;
+        FieldRef fullPath;
         if (applyParams.pathTaken->empty()) {
-            fullPath = applyParams.pathToCreate->dottedField().toString();
+            fullPath = *applyParams.pathToCreate;
         } else {
-            fullPath = str::stream() << applyParams.pathTaken->dottedField() << "."
-                                     << applyParams.pathToCreate->dottedField();
+            fullPath = FieldRef(str::stream() << applyParams.pathTaken->dottedField() << "."
+                                              << applyParams.pathToCreate->dottedField());
         }
 
         ApplyResult applyResult;
@@ -289,12 +288,13 @@ UpdateNode::ApplyResult ModifierNode::applyToNonexistentElement(ApplyParams appl
         if (!applyParams.indexData ||
             !applyParams.indexData->mightBeIndexed(applyParams.element.getType() != BSONType::Array
                                                        ? fullPath
-                                                       : applyParams.pathTaken->dottedField())) {
+                                                       : *applyParams.pathTaken)) {
             applyResult.indexesAffected = false;
         }
 
         if (applyParams.logBuilder) {
-            logUpdate(applyParams.logBuilder, fullPath, newElement, ModifyResult::kCreated);
+            logUpdate(
+                applyParams.logBuilder, fullPath.dottedField(), newElement, ModifyResult::kCreated);
         }
 
         return applyResult;
diff --git a/src/mongo/db/update/update_node_test_fixture.h b/src/mongo/db/update/update_node_test_fixture.h
index 6da205a2cb8..725f9ba0c46 100644
--- a/src/mongo/db/update/update_node_test_fixture.h
+++ b/src/mongo/db/update/update_node_test_fixture.h
@@ -113,7 +113,7 @@ protected:
         if (!_indexData) {
             _indexData = stdx::make_unique<UpdateIndexData>();
         }
-        _indexData->addPath(path);
+        _indexData->addPath(FieldRef(path));
     }
 
     void setLogBuilderToNull() {
diff --git a/src/mongo/db/update_index_data.cpp b/src/mongo/db/update_index_data.cpp
index 2b1144d40a3..e3bd3c6d808 100644
--- a/src/mongo/db/update_index_data.cpp
+++ b/src/mongo/db/update_index_data.cpp
@@ -30,21 +30,13 @@
 
 #include "mongo/db/update_index_data.h"
 #include "mongo/bson/util/builder.h"
-#include "mongo/db/field_ref.h"
 
 namespace mongo {
 
-using std::string;
-
 UpdateIndexData::UpdateIndexData() : _allPathsIndexed(false) {}
 
-void UpdateIndexData::addPath(StringData path) {
-    string s;
-    if (getCanonicalIndexField(path, &s)) {
-        _canonicalPaths.insert(s);
-    } else {
-        _canonicalPaths.insert(path.toString());
-    }
+void UpdateIndexData::addPath(const FieldRef& path) {
+    _canonicalPaths.insert(getCanonicalIndexField(path));
 }
 
 void UpdateIndexData::addPathComponent(StringData pathComponent) {
@@ -61,33 +53,21 @@ void UpdateIndexData::clear() {
     _allPathsIndexed = false;
 }
 
-bool UpdateIndexData::mightBeIndexed(StringData path) const {
+bool UpdateIndexData::mightBeIndexed(const FieldRef& path) const {
     if (_allPathsIndexed) {
         return true;
     }
 
-    StringData use = path;
-    string x;
-    if (getCanonicalIndexField(path, &x))
-        use = StringData(x);
-
-    for (std::set<string>::const_iterator i = _canonicalPaths.begin(); i != _canonicalPaths.end();
-         ++i) {
-        StringData idx(*i);
-
-        if (_startsWith(use, idx))
-            return true;
+    FieldRef canonicalPath = getCanonicalIndexField(path);
 
-        if (_startsWith(idx, use))
+    for (const auto& idx : _canonicalPaths) {
+        if (_startsWith(canonicalPath, idx) || _startsWith(idx, canonicalPath))
             return true;
     }
 
-    FieldRef pathFieldRef(path);
-    for (std::set<string>::const_iterator i = _pathComponents.begin(); i != _pathComponents.end();
-         ++i) {
-        const string& pathComponent = *i;
-        for (size_t partIdx = 0; partIdx < pathFieldRef.numParts(); ++partIdx) {
-            if (pathComponent == pathFieldRef.getPart(partIdx)) {
+    for (const auto& pathComponent : _pathComponents) {
+        for (size_t partIdx = 0; partIdx < path.numParts(); ++partIdx) {
+            if (pathComponent == path.getPart(partIdx)) {
                 return true;
             }
         }
@@ -96,74 +76,41 @@ bool UpdateIndexData::mightBeIndexed(StringData path) const {
     return false;
 }
 
-bool UpdateIndexData::_startsWith(StringData a, StringData b) const {
-    if (!a.startsWith(b))
-        return false;
-
-    // make sure there is a dot or EOL right after
-
-    if (a.size() == b.size())
-        return true;
-
-    return a[b.size()] == '.';
+bool UpdateIndexData::_startsWith(const FieldRef& a, const FieldRef& b) const {
+    return (a == b) || (b.isPrefixOf(a));
 }
 
-bool getCanonicalIndexField(StringData fullName, string* out) {
-    // check if fieldName contains ".$" or ".###" substrings (#=digit) and skip them
-    // however do not skip the first field even if it meets these criteria
-
-    if (fullName.find('.') == string::npos)
-        return false;
+FieldRef UpdateIndexData::getCanonicalIndexField(const FieldRef& path) {
+    if (path.numParts() <= 1)
+        return path;
 
-    bool modified = false;
+    // The first part of the path must always be a valid field name, since it's not possible to
+    // store a top-level array or '$' field name in a document.
+    FieldRef buf(path.getPart(0));
+    for (size_t i = 1; i < path.numParts(); ++i) {
+        auto pathComponent = path.getPart(i);
 
-    StringBuilder buf;
-    for (size_t i = 0; i < fullName.size(); i++) {
-        char c = fullName[i];
-
-        if (c != '.') {
-            buf << c;
+        if (pathComponent == "$"_sd) {
             continue;
         }
 
-        if (i + 1 == fullName.size()) {
-            // ends with '.'
-            buf << c;
-            continue;
-        }
-
-        // check for ".$", skip if present
-        if (fullName[i + 1] == '$') {
-            // only do this if its not something like $a
-            if (i + 2 >= fullName.size() || fullName[i + 2] == '.') {
-                i++;
-                modified = true;
-                continue;
-            }
-        }
-
-        // check for ".###" for any number of digits (no letters)
-        if (isdigit(fullName[i + 1])) {
-            size_t j = i;
-            // skip digits
-            while (j + 1 < fullName.size() && isdigit(fullName[j + 1]))
-                j++;
-
-            if (j + 1 == fullName.size() || fullName[j + 1] == '.') {
-                // only digits found, skip forward
-                i = j;
-                modified = true;
-                continue;
+        if (FieldRef::isNumericPathComponentLenient(pathComponent)) {
+            // Peek ahead to see if the next component is also all digits. This implies that the
+            // update is attempting to create a numeric field name which would violate the
+            // "ambiguous field name in array" constraint for multi-key indexes. Break early in this
+            // case and conservatively return that this path affects the prefix of the consecutive
+            // numerical path components. For instance, an input such as 'a.0.1.b.c' would return
+            // the canonical index path of 'a'.
+            if ((i + 1) < path.numParts() &&
+                FieldRef::isNumericPathComponentLenient(path.getPart(i + 1))) {
+                break;
             }
+            continue;
         }
 
-        buf << c;
+        buf.appendPart(pathComponent);
     }
 
-    if (!modified)
-        return false;
-
-    *out = buf.str();
-    return true;
+    return buf;
 }
 }
diff --git a/src/mongo/db/update_index_data.h b/src/mongo/db/update_index_data.h
index 52113663df0..65dc892cd08 100644
--- a/src/mongo/db/update_index_data.h
+++ b/src/mongo/db/update_index_data.h
@@ -33,17 +33,11 @@
 #include <set>
 
 #include "mongo/base/string_data.h"
+#include "mongo/db/field_ref.h"
 
 namespace mongo {
 
 /**
- * a.$ -> a
- * @return true if out is set and we made a change
- */
-bool getCanonicalIndexField(StringData fullName, std::string* out);
-
-
-/**
  * Holds pre-processed index spec information to allow update to quickly determine if an update
  * can be applied as a delta to a document, or if the document must be re-indexed.
  */
@@ -52,10 +46,16 @@ public:
     UpdateIndexData();
 
     /**
+     * Returns the canonicalized index form for 'path', removing numerical path components as well
+     * as '$' path components.
+     */
+    static FieldRef getCanonicalIndexField(const FieldRef& path);
+
+    /**
      * Register a path.  Any update targeting this path (or a parent of this path) will
      * trigger a recomputation of the document's index keys.
      */
-    void addPath(StringData path);
+    void addPath(const FieldRef& path);
 
     /**
      * Register a path component.  Any update targeting a path that contains this exact
@@ -71,12 +71,15 @@ public:
 
     void clear();
 
-    bool mightBeIndexed(StringData path) const;
+    bool mightBeIndexed(const FieldRef& path) const;
 
 private:
-    bool _startsWith(StringData a, StringData b) const;
+    /**
+     * Returns true if 'b' is a prefix of 'a', or if the two paths are equal.
+     */
+    bool _startsWith(const FieldRef& a, const FieldRef& b) const;
 
-    std::set<std::string> _canonicalPaths;
+    std::set<FieldRef> _canonicalPaths;
     std::set<std::string> _pathComponents;
 
     bool _allPathsIndexed;
diff --git a/src/mongo/db/update_index_data_test.cpp b/src/mongo/db/update_index_data_test.cpp
index 167cb632ab1..a01a02fa0c4 100644
--- a/src/mongo/db/update_index_data_test.cpp
+++ b/src/mongo/db/update_index_data_test.cpp
@@ -33,97 +33,100 @@
 
 namespace mongo {
 
-using std::string;
-
 TEST(UpdateIndexDataTest, Simple1) {
     UpdateIndexData a;
-    a.addPath("a.b");
-    ASSERT_TRUE(a.mightBeIndexed("a.b"));
-    ASSERT_TRUE(a.mightBeIndexed("a"));
-    ASSERT_TRUE(a.mightBeIndexed("a.b.c"));
-    ASSERT_TRUE(a.mightBeIndexed("a.$.b"));
+    a.addPath(FieldRef("a.b"_sd));
+    ASSERT_TRUE(a.mightBeIndexed(FieldRef("a.b")));
+    ASSERT_TRUE(a.mightBeIndexed(FieldRef("a")));
+    ASSERT_TRUE(a.mightBeIndexed(FieldRef("a.b.c")));
+    ASSERT_TRUE(a.mightBeIndexed(FieldRef("a.$.b")));
 
-    ASSERT_FALSE(a.mightBeIndexed("b"));
-    ASSERT_FALSE(a.mightBeIndexed("a.c"));
+    ASSERT_FALSE(a.mightBeIndexed(FieldRef("b")));
+    ASSERT_FALSE(a.mightBeIndexed(FieldRef("a.c")));
 
     a.clear();
-    ASSERT_FALSE(a.mightBeIndexed("a.b"));
+    ASSERT_FALSE(a.mightBeIndexed(FieldRef("a.b")));
 }
 
 TEST(UpdateIndexDataTest, Simple2) {
     UpdateIndexData a;
-    a.addPath("ab");
-    ASSERT_FALSE(a.mightBeIndexed("a"));
+    a.addPath(FieldRef("ab"_sd));
+    ASSERT_FALSE(a.mightBeIndexed(FieldRef("a")));
     a.clear();
-    ASSERT_FALSE(a.mightBeIndexed("ab"));
+    ASSERT_FALSE(a.mightBeIndexed(FieldRef("ab")));
 }
 
 TEST(UpdateIndexDataTest, Component1) {
     UpdateIndexData a;
-    a.addPathComponent("a");
-    ASSERT_FALSE(a.mightBeIndexed(""));
-    ASSERT_TRUE(a.mightBeIndexed("a"));
-    ASSERT_TRUE(a.mightBeIndexed("b.a"));
-    ASSERT_TRUE(a.mightBeIndexed("a.b"));
-    ASSERT_TRUE(a.mightBeIndexed("b.a.c"));
-    ASSERT_FALSE(a.mightBeIndexed("b.c"));
-    ASSERT_FALSE(a.mightBeIndexed("ab"));
+    a.addPathComponent("a"_sd);
+    ASSERT_FALSE(a.mightBeIndexed(FieldRef("")));
+    ASSERT_TRUE(a.mightBeIndexed(FieldRef("a")));
+    ASSERT_TRUE(a.mightBeIndexed(FieldRef("b.a")));
+    ASSERT_TRUE(a.mightBeIndexed(FieldRef("a.b")));
+    ASSERT_TRUE(a.mightBeIndexed(FieldRef("b.a.c")));
+    ASSERT_FALSE(a.mightBeIndexed(FieldRef("b.c")));
+    ASSERT_FALSE(a.mightBeIndexed(FieldRef("ab")));
     a.clear();
-    ASSERT_FALSE(a.mightBeIndexed("a"));
+    ASSERT_FALSE(a.mightBeIndexed(FieldRef("a")));
 }
 
 TEST(UpdateIndexDataTest, AllPathsIndexed1) {
     UpdateIndexData a;
     a.allPathsIndexed();
-    ASSERT_TRUE(a.mightBeIndexed("a"));
+    ASSERT_TRUE(a.mightBeIndexed(FieldRef("a")));
     a.clear();
-    ASSERT_FALSE(a.mightBeIndexed("a"));
+    ASSERT_FALSE(a.mightBeIndexed(FieldRef("a")));
 }
 
 TEST(UpdateIndexDataTest, AllPathsIndexed2) {
     UpdateIndexData a;
     a.allPathsIndexed();
-    ASSERT_TRUE(a.mightBeIndexed("a"));
-    ASSERT_TRUE(a.mightBeIndexed(""));
-    a.addPathComponent("a");
-    ASSERT_TRUE(a.mightBeIndexed("a"));
-    ASSERT_TRUE(a.mightBeIndexed("b"));
+    ASSERT_TRUE(a.mightBeIndexed(FieldRef("a")));
+    ASSERT_TRUE(a.mightBeIndexed(FieldRef("")));
+    a.addPathComponent("a"_sd);
+    ASSERT_TRUE(a.mightBeIndexed(FieldRef("a")));
+    ASSERT_TRUE(a.mightBeIndexed(FieldRef("b")));
     a.clear();
-    ASSERT_FALSE(a.mightBeIndexed("a"));
+    ASSERT_FALSE(a.mightBeIndexed(FieldRef("a")));
 }
 
-TEST(UpdateIndexDataTest, getCanonicalIndexField1) {
-    string x;
-
-    ASSERT_FALSE(getCanonicalIndexField("a", &x));
-    ASSERT_FALSE(getCanonicalIndexField("aaa", &x));
-    ASSERT_FALSE(getCanonicalIndexField("a.b", &x));
-
-    ASSERT_TRUE(getCanonicalIndexField("a.$", &x));
-    ASSERT_EQUALS(x, "a");
-    ASSERT_TRUE(getCanonicalIndexField("a.0", &x));
-    ASSERT_EQUALS(x, "a");
-    ASSERT_TRUE(getCanonicalIndexField("a.123", &x));
-    ASSERT_EQUALS(x, "a");
-
-    ASSERT_TRUE(getCanonicalIndexField("a.$.b", &x));
-    ASSERT_EQUALS(x, "a.b");
-    ASSERT_TRUE(getCanonicalIndexField("a.0.b", &x));
-    ASSERT_EQUALS(x, "a.b");
-    ASSERT_TRUE(getCanonicalIndexField("a.123.b", &x));
-    ASSERT_EQUALS(x, "a.b");
-
-    ASSERT_FALSE(getCanonicalIndexField("a.$ref", &x));
-    ASSERT_FALSE(getCanonicalIndexField("a.$ref.b", &x));
-
-
-    ASSERT_FALSE(getCanonicalIndexField("a.c$d.b", &x));
-
-    ASSERT_FALSE(getCanonicalIndexField("a.123a", &x));
-    ASSERT_FALSE(getCanonicalIndexField("a.a123", &x));
-    ASSERT_FALSE(getCanonicalIndexField("a.123a.b", &x));
-    ASSERT_FALSE(getCanonicalIndexField("a.a123.b", &x));
+TEST(UpdateIndexDataTest, CanonicalIndexField) {
+    ASSERT_EQ(UpdateIndexData::getCanonicalIndexField(FieldRef("a")), FieldRef("a"_sd));
+    ASSERT_EQ(UpdateIndexData::getCanonicalIndexField(FieldRef("aaa")), FieldRef("aaa"_sd));
+    ASSERT_EQ(UpdateIndexData::getCanonicalIndexField(FieldRef("a.b")), FieldRef("a.b"_sd));
+
+    ASSERT_EQ(UpdateIndexData::getCanonicalIndexField(FieldRef("a.$")), FieldRef("a"_sd));
+    ASSERT_EQ(UpdateIndexData::getCanonicalIndexField(FieldRef("a.0")), FieldRef("a"_sd));
+    ASSERT_EQ(UpdateIndexData::getCanonicalIndexField(FieldRef("a.123")), FieldRef("a"_sd));
+
+    ASSERT_EQ(UpdateIndexData::getCanonicalIndexField(FieldRef("a.$.b")), FieldRef("a.b"_sd));
+    ASSERT_EQ(UpdateIndexData::getCanonicalIndexField(FieldRef("a.0.b")), FieldRef("a.b"_sd));
+    ASSERT_EQ(UpdateIndexData::getCanonicalIndexField(FieldRef("a.123.b")), FieldRef("a.b"_sd));
+
+    ASSERT_EQ(UpdateIndexData::getCanonicalIndexField(FieldRef("a.$ref")), FieldRef("a.$ref"_sd));
+    ASSERT_EQ(UpdateIndexData::getCanonicalIndexField(FieldRef("a.$ref.b")),
+              FieldRef("a.$ref.b"_sd));
+    ASSERT_EQ(UpdateIndexData::getCanonicalIndexField(FieldRef("a.c$d.b")), FieldRef("a.c$d.b"_sd));
+
+    ASSERT_EQ(UpdateIndexData::getCanonicalIndexField(FieldRef("a.123a")), FieldRef("a.123a"_sd));
+    ASSERT_EQ(UpdateIndexData::getCanonicalIndexField(FieldRef("a.a123")), FieldRef("a.a123"_sd));
+    ASSERT_EQ(UpdateIndexData::getCanonicalIndexField(FieldRef("a.123a.b")),
+              FieldRef("a.123a.b"_sd));
+    ASSERT_EQ(UpdateIndexData::getCanonicalIndexField(FieldRef("a.a123.b")),
+              FieldRef("a.a123.b"_sd));
+
+    ASSERT_EQ(UpdateIndexData::getCanonicalIndexField(FieldRef("a.")), FieldRef("a."_sd));
+    ASSERT_EQ(UpdateIndexData::getCanonicalIndexField(FieldRef("$")), FieldRef("$"_sd));
+    ASSERT_EQ(UpdateIndexData::getCanonicalIndexField(FieldRef("$.a")), FieldRef("$.a"_sd));
+    ASSERT_EQ(UpdateIndexData::getCanonicalIndexField(FieldRef("a.$")), FieldRef("a"_sd));
+}
 
-    ASSERT_FALSE(getCanonicalIndexField("a.", &x));
+TEST(UpdateIndexDataTest, CanonicalIndexFieldForNestedNumericFieldNames) {
+    ASSERT_EQ(UpdateIndexData::getCanonicalIndexField(FieldRef("a.0.0")), FieldRef("a"_sd));
+    ASSERT_EQ(UpdateIndexData::getCanonicalIndexField(FieldRef("a.55.01")), FieldRef("a"_sd));
+    ASSERT_EQ(UpdateIndexData::getCanonicalIndexField(FieldRef("a.0.0.b.1")), FieldRef("a"_sd));
+    ASSERT_EQ(UpdateIndexData::getCanonicalIndexField(FieldRef("a.0b.1")), FieldRef("a.0b"_sd));
+    ASSERT_EQ(UpdateIndexData::getCanonicalIndexField(FieldRef("a.0.b.1.2")), FieldRef("a.b"_sd));
+    ASSERT_EQ(UpdateIndexData::getCanonicalIndexField(FieldRef("a.01.02.b.c")), FieldRef("a"_sd));
 }
 }