diff options
8 files changed, 192 insertions, 33 deletions
diff --git a/jstests/core/timeseries/timeseries_delete_hint.js b/jstests/core/timeseries/timeseries_delete_hint.js index ebcca3e3560..0facca816e1 100644 --- a/jstests/core/timeseries/timeseries_delete_hint.js +++ b/jstests/core/timeseries/timeseries_delete_hint.js @@ -131,6 +131,20 @@ const objC = { [metaFieldName]: {c: "C"} }; +// Query using a $natural hint. +validateDeleteIndex([objA, objB, objC], + [objB, objC], + 1, + [{q: {[metaFieldName]: {a: "A"}}, limit: 0, hint: {$natural: 1}}], + [{[metaFieldName]: 1}], + "COLLSCAN"); +validateDeleteIndex([objA, objB, objC], + [objB, objC], + 1, + [{q: {[metaFieldName]: {a: "A"}}, limit: 0, hint: {$natural: -1}}], + [{[metaFieldName]: 1}], + "COLLSCAN"); + // Query using the metaField index as a hint. validateDeleteIndex([objA, objB, objC], [objB, objC], diff --git a/jstests/core/timeseries/timeseries_hint.js b/jstests/core/timeseries/timeseries_hint.js new file mode 100644 index 00000000000..037de311829 --- /dev/null +++ b/jstests/core/timeseries/timeseries_hint.js @@ -0,0 +1,90 @@ +/** + * Test $natural hint on a time-series collection, for find and aggregate. + * + * @tags: [ + * does_not_support_stepdowns, + * does_not_support_transactions, + * requires_fcv_51, + * requires_getmore, + * requires_pipeline_optimization, + * # Explain of a resolved view must be executed by mongos. + * directly_against_shardsvrs_incompatible, + * ] + */ +(function() { +"use strict"; + +load("jstests/core/timeseries/libs/timeseries.js"); +load("jstests/libs/analyze_plan.js"); + +const coll = db.timeseries_hint; +coll.drop(); +assert.commandWorked(db.createCollection(coll.getName(), { + timeseries: {timeField: 't', metaField: 'm'}, +})); + +// With only one event per bucket, the order of results will be predictable. +// The min time of each bucket is encoded in its _id, and the clustered collection +// ensures that $natural order is also _id order. +const docsAsc = [ + {_id: 1, m: 1, t: ISODate('1970-01-01')}, + {_id: 2, m: 2, t: ISODate('1970-01-02')}, + {_id: 3, m: 3, t: ISODate('1970-01-03')}, +]; +const docsDesc = docsAsc.slice().reverse(); +assert.commandWorked(coll.insert(docsAsc)); + +function runTest({command, expectedResult, expectedDirection}) { + const result = assert.commandWorked(db.runCommand(command)); + assert.docEq(result.cursor.firstBatch, expectedResult); + + const plan = db.runCommand({explain: command}); + const scan = getAggPlanStage(plan, 'COLLSCAN'); + assert(scan, 'Expected a COLLSCAN stage' + tojson(plan)); + assert.eq(scan.direction, + expectedDirection, + 'Expected a ' + expectedDirection + ' COLLSCAN ' + tojson(scan)); +} + +// Test find: ascending and descending. +runTest({ + command: { + find: coll.getName(), + filter: {}, + hint: {$natural: 1}, + }, + expectedResult: docsAsc, + expectedDirection: 'forward', +}); +runTest({ + command: { + find: coll.getName(), + filter: {}, + hint: {$natural: -1}, + }, + expectedResult: docsDesc, + expectedDirection: 'backward', +}); + +// Test aggregate: ascending and descending. +runTest({ + command: { + aggregate: coll.getName(), + pipeline: [], + cursor: {}, + hint: {$natural: 1}, + }, + expectedResult: docsAsc, + expectedDirection: 'forward', +}); +runTest({ + command: { + aggregate: coll.getName(), + pipeline: [], + cursor: {}, + hint: {$natural: -1}, + }, + expectedResult: docsDesc, + expectedDirection: 'backward', +}); +})(); diff --git a/jstests/core/timeseries/timeseries_update_hint.js b/jstests/core/timeseries/timeseries_update_hint.js index 9dbc145a66f..e51e021c356 100644 --- a/jstests/core/timeseries/timeseries_update_hint.js +++ b/jstests/core/timeseries/timeseries_update_hint.js @@ -39,6 +39,9 @@ const dbName = jsTestName(); */ const testUpdateHintSucceeded = ({initialDocList, indexes, updateList, resultDocList, nModifiedBuckets, expectedPlan}) => { + const testDB = db.getSiblingDB(dbName); + const coll = testDB.getCollection(collName); + const awaitTestUpdate = startParallelShell(funWithArgs( function(dbName, collName, @@ -88,20 +91,18 @@ const testUpdateHintSucceeded = updateList, resultDocList, nModifiedBuckets)); - - const testDB = db.getSiblingDB(dbName); - const coll = testDB.getCollection(collName); - - const childCurOp = - waitForCurOpByFailPoint(testDB, coll.getFullName(), "hangAfterBatchUpdate")[0]; - - // Verify that the query plan uses the expected index. - assert.eq(childCurOp.planSummary, expectedPlan); - - assert.commandWorked( - testDB.adminCommand({configureFailPoint: "hangAfterBatchUpdate", mode: "off"})); - - awaitTestUpdate(); + try { + const childCurOp = + waitForCurOpByFailPoint(testDB, coll.getFullName(), "hangAfterBatchUpdate")[0]; + + // Verify that the query plan uses the expected index. + assert.eq(childCurOp.planSummary, expectedPlan); + } finally { + assert.commandWorked( + testDB.adminCommand({configureFailPoint: "hangAfterBatchUpdate", mode: "off"})); + + awaitTestUpdate(); + } }; /** @@ -153,6 +154,44 @@ const hintDoc3 = { }; /************* Tests passing a hint to an update on a collection with a single index. *************/ +// Query on and update the metaField using a forward collection scan: hint: {$natural 1}. +testUpdateHintSucceeded({ + initialDocList: [hintDoc1, hintDoc2, hintDoc3], + indexes: [{[metaFieldName]: 1}], + updateList: [{ + q: {[metaFieldName + ".a"]: {$lte: 2}}, + u: {$inc: {[metaFieldName + ".a"]: 10}}, + multi: true, + hint: {$natural: 1} + }], + resultDocList: [ + {_id: 1, [timeFieldName]: dateTime, [metaFieldName]: {"a": 11}}, + {_id: 2, [timeFieldName]: dateTime, [metaFieldName]: {"a": 12}}, + hintDoc3 + ], + nModifiedBuckets: 2, + expectedPlan: "COLLSCAN", +}); + +// Query on and update the metaField using a backward collection scan: hint: {$natural -1}. +testUpdateHintSucceeded({ + initialDocList: [hintDoc1, hintDoc2, hintDoc3], + indexes: [{[metaFieldName]: 1}], + updateList: [{ + q: {[metaFieldName + ".a"]: {$lte: 2}}, + u: {$inc: {[metaFieldName + ".a"]: 10}}, + multi: true, + hint: {$natural: -1} + }], + resultDocList: [ + {_id: 1, [timeFieldName]: dateTime, [metaFieldName]: {"a": 11}}, + {_id: 2, [timeFieldName]: dateTime, [metaFieldName]: {"a": 12}}, + hintDoc3 + ], + nModifiedBuckets: 2, + expectedPlan: "COLLSCAN", +}); + // Query on and update the metaField using the metaField index as a hint, specifying the hint with // an index specification document. testUpdateHintSucceeded({ diff --git a/src/mongo/db/ops/write_ops_exec.cpp b/src/mongo/db/ops/write_ops_exec.cpp index b0b23e3e42b..d1afdc8dde9 100644 --- a/src/mongo/db/ops/write_ops_exec.cpp +++ b/src/mongo/db/ops/write_ops_exec.cpp @@ -809,10 +809,8 @@ static SingleWriteResult performSingleUpdateOp(OperationContext* opCtx, "Cannot perform an upsert on a time-series collection", !updateRequest->isUpsert()); - // Only translate the hint (if there is one) if it is specified with an index specification - // document. - if (!updateRequest->getHint().isEmpty() && - updateRequest->getHint().firstElement().fieldNameStringData() != "$hint"_sd) { + // Only translate the hint if it is specified with an index key. + if (timeseries::isHintIndexKey(updateRequest->getHint())) { updateRequest->setHint( uassertStatusOK(timeseries::createBucketsIndexSpecFromTimeseriesIndexSpec( *timeseriesOptions, updateRequest->getHint()))); @@ -1144,10 +1142,8 @@ static SingleWriteResult performSingleDeleteOp(OperationContext* opCtx, "Time-series buckets collection is missing time-series options", timeseriesOptions); - // Only translate the hint if it is specified by index spec. - if (!request.getHint().isEmpty() && - (request.getHint().firstElement().fieldNameStringData() != "$hint"_sd || - request.getHint().firstElement().type() != BSONType::String)) { + // Only translate the hint if it is specified by index key. + if (timeseries::isHintIndexKey(request.getHint())) { request.setHint( uassertStatusOK(timeseries::createBucketsIndexSpecFromTimeseriesIndexSpec( *timeseriesOptions, request.getHint()))); diff --git a/src/mongo/db/timeseries/timeseries_index_schema_conversion_functions.cpp b/src/mongo/db/timeseries/timeseries_index_schema_conversion_functions.cpp index 1dff8c06341..9338f2c237d 100644 --- a/src/mongo/db/timeseries/timeseries_index_schema_conversion_functions.cpp +++ b/src/mongo/db/timeseries/timeseries_index_schema_conversion_functions.cpp @@ -71,6 +71,14 @@ std::pair<std::string, std::string> extractControlPrefixAndKey(const StringData& StatusWith<BSONObj> createBucketsSpecFromTimeseriesSpec(const TimeseriesOptions& timeseriesOptions, const BSONObj& timeseriesIndexSpecBSON, bool isShardKeySpec) { + tassert(6390200, "Empty object is not a valid index spec", !timeseriesIndexSpecBSON.isEmpty()); + tassert(6390201, + str::stream() << "Invalid index spec (perhaps it's a valid hint, that was incorrectly " + << "passed to createBucketsSpecFromTimeseriesSpec): " + << timeseriesIndexSpecBSON, + timeseriesIndexSpecBSON.firstElement().fieldNameStringData() != "$hint"_sd && + timeseriesIndexSpecBSON.firstElement().fieldNameStringData() != "$natural"_sd); + auto timeField = timeseriesOptions.getTimeField(); auto metaField = timeseriesOptions.getMetaField(); @@ -474,4 +482,16 @@ bool doesBucketsIndexIncludeMeasurement(OperationContext* opCtx, return false; } +bool isHintIndexKey(const BSONObj& obj) { + if (obj.isEmpty()) + return false; + StringData fieldName = obj.firstElement().fieldNameStringData(); + if (fieldName == "$hint"_sd) + return false; + if (fieldName == "$natural"_sd) + return false; + + return true; +} + } // namespace mongo::timeseries diff --git a/src/mongo/db/timeseries/timeseries_index_schema_conversion_functions.h b/src/mongo/db/timeseries/timeseries_index_schema_conversion_functions.h index ca9fc2e1a77..ad1bb795fd2 100644 --- a/src/mongo/db/timeseries/timeseries_index_schema_conversion_functions.h +++ b/src/mongo/db/timeseries/timeseries_index_schema_conversion_functions.h @@ -88,4 +88,13 @@ bool doesBucketsIndexIncludeMeasurement(OperationContext* opCtx, const TimeseriesOptions& timeseriesOptions, const BSONObj& bucketsIndex); +/** + * Takes a 'hint' object, in the same format used by FindCommandRequest, and returns + * true if the hint is an index key. + * + * Besides an index key, a hint can be {$hint: <index name>} or {$natural: <direction>}, + * or it can be {} which means no hint is given. + */ +bool isHintIndexKey(const BSONObj& obj); + } // namespace mongo::timeseries diff --git a/src/mongo/db/timeseries/timeseries_index_schema_conversion_functions_test.cpp b/src/mongo/db/timeseries/timeseries_index_schema_conversion_functions_test.cpp index 88a2fd76b7d..db6eadda021 100644 --- a/src/mongo/db/timeseries/timeseries_index_schema_conversion_functions_test.cpp +++ b/src/mongo/db/timeseries/timeseries_index_schema_conversion_functions_test.cpp @@ -128,14 +128,6 @@ TEST(TimeseriesIndexSchemaConversionTest, OriginalSpecFieldName) { } } -// {} <=> {} -TEST(TimeseriesIndexSchemaConversionTest, EmptyTimeseriesIndexSpecDoesNothing) { - TimeseriesOptions timeseriesOptions = makeTimeseriesOptions(); - BSONObj emptyIndexSpec = {}; - - testBothWaysIndexSpecConversion(timeseriesOptions, emptyIndexSpec, emptyIndexSpec); -} - // {tm: 1} <=> {control.min.tm: 1, control.max.tm: 1} TEST(TimeseriesIndexSchemaConversionTest, AscendingTimeIndexSpecConversion) { TimeseriesOptions timeseriesOptions = makeTimeseriesOptions(); diff --git a/src/mongo/db/views/resolved_view.cpp b/src/mongo/db/views/resolved_view.cpp index ec60c6f0826..244d38a0b55 100644 --- a/src/mongo/db/views/resolved_view.cpp +++ b/src/mongo/db/views/resolved_view.cpp @@ -183,9 +183,8 @@ AggregateCommandRequest ResolvedView::asExpandedViewAggregation( if (request.getHint() && _timeseriesOptions) { BSONObj original = *request.getHint(); BSONObj rewritten = original; - // Only convert if we are given an index spec, not an index name. An index name is provided - // in the form of {"$hint": <name>}. - if (!original.isEmpty() && original.firstElementFieldNameStringData() != "$hint"_sd) { + // Only convert if we are given an index spec, not an index name or a $natural hint. + if (timeseries::isHintIndexKey(original)) { auto converted = timeseries::createBucketsIndexSpecFromTimeseriesIndexSpec( *_timeseriesOptions, original); if (converted.isOK()) { |