diff options
5 files changed, 95 insertions, 36 deletions
diff --git a/jstests/core/timeseries/timeseries_metadata.js b/jstests/core/timeseries/timeseries_metadata.js index 95dcc35fbfa..26384adc4a8 100644 --- a/jstests/core/timeseries/timeseries_metadata.js +++ b/jstests/core/timeseries/timeseries_metadata.js @@ -118,10 +118,13 @@ runTest( ]); runTest( - // No metadata field in first bucket. + // Null metadata field in first bucket. Temporarily use null meta instead of missing meta + // to accomodate the new $_internalUnpackBucket behavior which is null meta in a bucket + // is materialized as "null" meta. + // TODO SERVER-55213: Need to test both missing meta case and null meta case. [ - {_id: 0, time: t[0], x: 0}, - {_id: 1, time: t[1], x: 10}, + {_id: 0, time: t[0], meta: null, x: 0}, + {_id: 1, time: t[1], meta: null, x: 10}, ], [ {_id: 2, time: t[2], meta: 123, x: 20}, @@ -134,10 +137,13 @@ runTest( {_id: 0, time: t[0], meta: [1, 2, 3], x: 0}, {_id: 1, time: t[1], meta: [1, 2, 3], x: 10}, ], - // No metadata field in second bucket. + // Null metadata field in second bucket. Temporarily use null meta instead of missing meta + // to accomodate the new $_internalUnpackBucket behavior which is null meta in a bucket + // is materialized as "null" meta. + // TODO SERVER-55213: Need to test both missing meta case and null meta case. [ - {_id: 2, time: t[2], x: 20}, - {_id: 3, time: t[3], x: 30}, + {_id: 2, time: t[2], meta: null, x: 20}, + {_id: 3, time: t[3], meta: null, x: 30}, ]); runTest( diff --git a/jstests/noPassthrough/timeseries_insert_ordered_false.js b/jstests/noPassthrough/timeseries_insert_ordered_false.js index e38147f9467..1e1e95d7fc8 100644 --- a/jstests/noPassthrough/timeseries_insert_ordered_false.js +++ b/jstests/noPassthrough/timeseries_insert_ordered_false.js @@ -33,10 +33,13 @@ resetColl(); configureFailPoint(conn, 'failTimeseriesInsert', {metadata: 'fail'}); +// Temporarily use null meta instead of missing meta to accomodate the new $_internalUnpackBucket +// behavior which is null meta in a bucket is materialized as "null" meta. +// TODO SERVER-55213: Remove 'meta: null'. const docs = [ - {_id: 0, [timeFieldName]: ISODate()}, - {_id: 1, [timeFieldName]: ISODate()}, - {_id: 2, [timeFieldName]: ISODate()}, + {_id: 0, meta: null, [timeFieldName]: ISODate()}, + {_id: 1, meta: null, [timeFieldName]: ISODate()}, + {_id: 2, meta: null, [timeFieldName]: ISODate()}, ]; let res = assert.commandWorked(coll.insert(docs, {ordered: false})); diff --git a/jstests/noPassthrough/timeseries_insert_ordered_true.js b/jstests/noPassthrough/timeseries_insert_ordered_true.js index d444981cbb4..7ef189b5ffc 100644 --- a/jstests/noPassthrough/timeseries_insert_ordered_true.js +++ b/jstests/noPassthrough/timeseries_insert_ordered_true.js @@ -33,10 +33,13 @@ resetColl(); configureFailPoint(conn, 'failTimeseriesInsert', {metadata: 'fail'}); +// Temporarily use null meta instead of missing meta to accomodate the new $_internalUnpackBucket +// behavior which is null meta in a bucket is materialized as "null" meta. +// TODO SERVER-55213: Remove 'meta: null'. const docs = [ - {_id: 0, [timeFieldName]: ISODate()}, - {_id: 1, [timeFieldName]: ISODate()}, - {_id: 2, [timeFieldName]: ISODate()}, + {_id: 0, meta: null, [timeFieldName]: ISODate()}, + {_id: 1, meta: null, [timeFieldName]: ISODate()}, + {_id: 2, meta: null, [timeFieldName]: ISODate()}, ]; let res = assert.commandWorked(coll.insert(docs, {ordered: true})); diff --git a/src/mongo/db/pipeline/document_source_internal_unpack_bucket.cpp b/src/mongo/db/pipeline/document_source_internal_unpack_bucket.cpp index b20be4cdd68..bb156781132 100644 --- a/src/mongo/db/pipeline/document_source_internal_unpack_bucket.cpp +++ b/src/mongo/db/pipeline/document_source_internal_unpack_bucket.cpp @@ -225,13 +225,13 @@ void BucketUnpacker::reset(BSONObj&& bucket) { _metaValue = _bucket[kBucketMetaFieldName]; if (_spec.metaField) { - // The spec indicates that there should be a metadata region. Missing metadata in this case - // is expressed with null, so the field is expected to be present. We also disallow - // undefined since the undefined BSON type is deprecated. + // The spec indicates that there might be a metadata region. Missing metadata in + // measurements is expressed with missing metadata in a bucket. But we disallow undefined + // since the undefined BSON type is deprecated. uassert(5369600, - "The $_internalUnpackBucket stage requires metadata to be present in a bucket if " - "metaField parameter is provided", - (_metaValue.type() != BSONType::Undefined) && _metaValue); + "The $_internalUnpackBucket stage allows metadata to be absent or otherwise, it " + "must not be the deprecated undefined bson type", + !_metaValue || _metaValue.type() != BSONType::Undefined); } else { // If the spec indicates that the time series collection has no metadata field, then we // should not find a metadata region in the underlying bucket documents. @@ -276,7 +276,8 @@ Document BucketUnpacker::getNext() { measurement.addField(_spec.timeField, Value{timeElem}); } - if (_includeMetaField && !_metaValue.isNull()) { + // Includes metaField when we're instructed to do so and metaField value exists. + if (_includeMetaField && _metaValue) { measurement.addField(*_spec.metaField, Value{_metaValue}); } diff --git a/src/mongo/db/pipeline/document_source_internal_unpack_bucket_test/unpack_bucket_exec_test.cpp b/src/mongo/db/pipeline/document_source_internal_unpack_bucket_test/unpack_bucket_exec_test.cpp index ae409de7e69..fcc63c7aaaa 100644 --- a/src/mongo/db/pipeline/document_source_internal_unpack_bucket_test/unpack_bucket_exec_test.cpp +++ b/src/mongo/db/pipeline/document_source_internal_unpack_bucket_test/unpack_bucket_exec_test.cpp @@ -560,30 +560,33 @@ TEST_F(InternalUnpackBucketExecTest, BucketUnpackerThrowsOnUndefinedMetadata) { ASSERT_THROWS_CODE(unpack->getNext(), AssertionException, 5369600); } -TEST_F(InternalUnpackBucketExecTest, BucketUnpackerThrowsOnMissingMetadataWhenExpectedInBuckets) { +TEST_F(InternalUnpackBucketExecTest, BucketUnpackerThrowsWhenMetadataIsPresentUnexpectedly) { auto expCtx = getExpCtx(); auto spec = BSON("$_internalUnpackBucket" << BSON("exclude" << BSONArray() << DocumentSourceInternalUnpackBucket::kTimeFieldName - << kUserDefinedTimeName - << DocumentSourceInternalUnpackBucket::kMetaFieldName - << kUserDefinedMetaName)); + << kUserDefinedTimeName)); auto unpack = DocumentSourceInternalUnpackBucket::createFromBson(spec.firstElement(), expCtx); auto source = - DocumentSourceMock::createForTest({"{data: {_id: {'1':1, " - "'0':2, '2': 3}, time: {'1':1, '0': 2, '2': 3}}}"}, + DocumentSourceMock::createForTest({"{meta: {'m1': 999, 'm2': 9999}, data: {_id: {'1':1, " + "'0':2, '2': 3}, time: {'1':1, '0': 2, '2': 3}}}", + "{meta: null, data: {_id: {'1':4, " + "'0':5, '2':6}, time: {'1':4, '0': 5, '2': 6}}}"}, expCtx); unpack->setSource(source.get()); - ASSERT_THROWS_CODE(unpack->getNext(), AssertionException, 5369600); + + ASSERT_THROWS_CODE(unpack->getNext(), AssertionException, 5369601); } -TEST_F(InternalUnpackBucketExecTest, BucketUnpackerThrowsWhenMetadataIsPresentUnexpectedly) { +TEST_F(InternalUnpackBucketExecTest, BucketUnpackerHandlesNullMetadata) { auto expCtx = getExpCtx(); auto spec = BSON("$_internalUnpackBucket" << BSON("exclude" << BSONArray() << DocumentSourceInternalUnpackBucket::kTimeFieldName - << kUserDefinedTimeName)); + << kUserDefinedTimeName + << DocumentSourceInternalUnpackBucket::kMetaFieldName + << kUserDefinedMetaName)); auto unpack = DocumentSourceInternalUnpackBucket::createFromBson(spec.firstElement(), expCtx); auto source = @@ -594,10 +597,39 @@ TEST_F(InternalUnpackBucketExecTest, BucketUnpackerThrowsWhenMetadataIsPresentUn expCtx); unpack->setSource(source.get()); - ASSERT_THROWS_CODE(unpack->getNext(), AssertionException, 5369601); + auto next = unpack->getNext(); + ASSERT_TRUE(next.isAdvanced()); + ASSERT_DOCUMENT_EQ(next.getDocument(), + Document(fromjson("{time: 1, myMeta: {m1: 999, m2: 9999}, _id: 1}"))); + + next = unpack->getNext(); + ASSERT_TRUE(next.isAdvanced()); + ASSERT_DOCUMENT_EQ(next.getDocument(), + Document(fromjson("{time: 2, myMeta: {m1: 999, m2: 9999}, _id: 2}"))); + + next = unpack->getNext(); + ASSERT_TRUE(next.isAdvanced()); + ASSERT_DOCUMENT_EQ(next.getDocument(), + Document(fromjson("{time: 3, myMeta: {m1: 999, m2: 9999}, _id: 3}"))); + + // Second bucket + next = unpack->getNext(); + ASSERT_TRUE(next.isAdvanced()); + ASSERT_DOCUMENT_EQ(next.getDocument(), Document(fromjson("{time: 4, myMeta: null, _id: 4}"))); + + next = unpack->getNext(); + ASSERT_TRUE(next.isAdvanced()); + ASSERT_DOCUMENT_EQ(next.getDocument(), Document(fromjson("{time: 5, myMeta: null, _id: 5}"))); + + next = unpack->getNext(); + ASSERT_TRUE(next.isAdvanced()); + ASSERT_DOCUMENT_EQ(next.getDocument(), Document(fromjson("{time: 6, myMeta: null, _id: 6}"))); + + next = unpack->getNext(); + ASSERT_TRUE(next.isEOF()); } -TEST_F(InternalUnpackBucketExecTest, BucketUnpackerHandlesNullMetadata) { +TEST_F(InternalUnpackBucketExecTest, BucketUnpackerHandlesMissingMetadata) { auto expCtx = getExpCtx(); auto spec = BSON("$_internalUnpackBucket" @@ -607,12 +639,26 @@ TEST_F(InternalUnpackBucketExecTest, BucketUnpackerHandlesNullMetadata) { << kUserDefinedMetaName)); auto unpack = DocumentSourceInternalUnpackBucket::createFromBson(spec.firstElement(), expCtx); - auto source = - DocumentSourceMock::createForTest({"{meta: {'m1': 999, 'm2': 9999}, data: {_id: {'1':1, " - "'0':2, '2': 3}, time: {'1':1, '0': 2, '2': 3}}}", - "{meta: null, data: {_id: {'1':4, " - "'0':5, '2':6}, time: {'1':4, '0': 5, '2': 6}}}"}, - expCtx); + auto source = DocumentSourceMock::createForTest( + { + R"( +{ + meta: { + 'm1': 999, 'm2': 9999 + }, + data: { + _id: {'1':1, '0':2, '2': 3}, + time: {'1':1, '0': 2, '2': 3} + } +})", + R"( +{ + data: { + _id: {'1':4, '0':5, '2':6}, + time: {'1':4, '0': 5, '2': 6} + } +})"}, + expCtx); unpack->setSource(source.get()); auto next = unpack->getNext(); |