SERVER-126021 Prohibit measurements with duplicate field names in timeseries collections (#53226)

GitOrigin-RevId: 18cf44d54b9dcdc0df43a984da7c6b12006c4217

src/mongo/db/timeseries/bucket_catalog/flat_bson.cpp

@@ -271,9 +271,14 @@ typename FlatBSONStore<Element, Value>::Iterator FlatBSONStore<Element, Value>::
     auto it = begin();
     auto itEnd = end();
     for (; it != itEnd; ++it) {
-        _pos->_fieldNameToIndex->try_emplace(
-            tracking::make_string(_trackingContext, it->fieldName().data(), it->fieldName().size()),
-            it._pos->_offsetParent);
+        uassert(12602100,
+                "Duplicate field names cannot be present in the same FlatBSON object",
+                _pos->_fieldNameToIndex
+                    ->try_emplace(tracking::make_string(_trackingContext,
+                                                        it->fieldName().data(),
+                                                        it->fieldName().size()),
+                                  it._pos->_offsetParent)
+                    .second);
     }
 
     // Retry the search now when the map is created.
@@ -303,11 +308,14 @@ FlatBSONStore<Element, Value>::Obj::insert(FlatBSONStore<Element, Value>::Iterat
 
     // Also store our offset in the fast lookup map if it is available.
     if (_pos->_fieldNameToIndex) {
-        _pos->_fieldNameToIndex->try_emplace(
-            tracking::make_string(_trackingContext,
-                                  inserted->_element.fieldName().data(),
-                                  inserted->_element.fieldName().size()),
-            inserted->_offsetParent);
+        uassert(12602101,
+                "Duplicate field names cannot be present in the same FlatBSON object",
+                _pos->_fieldNameToIndex
+                    ->try_emplace(tracking::make_string(_trackingContext,
+                                                        inserted->_element.fieldName().data(),
+                                                        inserted->_element.fieldName().size()),
+                                  inserted->_offsetParent)
+                    .second);
     }
 
     // We need to traverse the hiearchy up to the root and modify stored offsets to account for

src/mongo/db/timeseries/bucket_catalog/measurement_map.cpp

@@ -136,7 +136,11 @@ void MeasurementMap::insertOne(const BSONObj& measurement, boost::optional<Strin
             continue;
         }
 
-        fieldsSeen.insert(key);
+
+        uassert(12602102,
+                "Measurements with duplicate field names cannot be stored in timeseries "
+                "collections",
+                fieldsSeen.insert(key).second);
 
         auto builderIt = _builders.find(key);
         if (builderIt == _builders.end()) {

src/mongo/db/timeseries/bucket_catalog/measurement_map_test.cpp

@@ -34,6 +34,7 @@
 #include "mongo/bson/json.h"
 #include "mongo/db/timeseries/timeseries_gen.h"
 #include "mongo/unittest/death_test.h"
+#include "mongo/unittest/unittest.h"
 #include "mongo/util/tracking/context.h"
 
 #include <string>
@@ -263,6 +264,27 @@ TEST_F(MeasurementMapTest, InitBuilders) {
     invariant(measurementMap.numFields() == 3);
 }
 
+TEST_F(MeasurementMapTest, DuplicateFieldNameThrows) {
+    BSONObjBuilder builder;
+    builder.append("a", 1);
+    builder.append("a", 2);
+
+    ASSERT_THROWS(measurementMap.insertOne(builder.obj(), /*metaField=*/boost::none),
+                  AssertionException);
+}
+
+TEST_F(MeasurementMapTest, DuplicateFieldNameInSubsequentThrows) {
+    const BSONObj m = BSON("a" << 1);
+    measurementMap.insertOne(m, /*metaField=*/boost::none);
+
+    BSONObjBuilder builder;
+    builder.append("a", 2);
+    builder.append("a", 3);
+
+    ASSERT_THROWS(measurementMap.insertOne(builder.obj(), /*metaField=*/boost::none),
+                  AssertionException);
+}
+
 DEATH_TEST_REGEX_F(MeasurementMapTest, GetTimeForNonexistentField, "Invariant failure.*") {
     measurementMap.timeOfLastMeasurement(_timeField);
 }

src/mongo/db/timeseries/bucket_catalog/minmax_test.cpp

@@ -305,5 +305,38 @@ TEST(MinMax, SearchLookupMap) {
     ASSERT_EQ(obj.search(obj.begin(), "50")->fieldName(), "50");
 }
 
+TEST(MinMax, DuplicateFieldNamesWithLookupMap) {
+    tracking::Context trackingContext;
+    MinMaxStore minmax{trackingContext};
+    auto obj = minmax.root();
+
+    // Insert 12 (kMaxLinearSearchLength) distinct fields ("0".."11") followed by two duplicate "a"
+    // entries. This will trigger the lookup map internally in flat_bson.
+    for (int i = 0; i < 12; ++i) {
+        obj.insert(obj.end(), std::to_string(i));
+    }
+    obj.insert(obj.end(), "a");
+    obj.insert(obj.end(), "a");
+
+    // Try to search for "a", this will trigger the lookup map internally in flat_bson as we fail to
+    // find it within 'kMaxLinearSearchLength' attempts. The map cannot contain duplicates so this
+    // search is well defined and throws.
+    ASSERT_THROWS(obj.search(obj.begin(), "a"), AssertionException);
+
+    // Try to insert another duplicate which will throw earlier as the lookup map exists and needs
+    // to be maintained.
+    obj.insert(obj.begin(), "x");
+    ASSERT_THROWS(obj.insert(obj.begin(), "x"), AssertionException);
+
+    // Searching for "a" or "x" is possible as we inserted one of them into the map.
+    auto found = obj.search(obj.begin(), "a");
+    ASSERT(found != obj.end());
+    ASSERT_EQ(found->fieldName(), "a");
+
+    found = obj.search(obj.begin(), "x");
+    ASSERT(found != obj.end());
+    ASSERT_EQ(found->fieldName(), "x");
+}
+
 }  // namespace
 }  // namespace mongo::timeseries::bucket_catalog