diff options
author | Ribhav Jain <ribhav.jain@mongodb.com> | 2020-07-10 21:04:50 +0000 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2020-07-10 21:19:44 +0000 |
commit | 251e0da292c5ac2cd55f148a1cc6b32568087cce (patch) | |
tree | 9f98be24a01341c4a324a5b01ecf5158347bb2f5 | |
parent | 6c0ac0678f3c159c65c497acf155e964dec43f61 (diff) | |
download | mongo-251e0da292c5ac2cd55f148a1cc6b32568087cce.tar.gz |
Revert "SERVER-33966 Removed Redundant Sort"
This reverts commit 96cb86d41e4800b0f359a4d25dad2a6a94501e32.
-rw-r--r-- | jstests/sharding/query/agg_mongos_merge.js | 23 | ||||
-rw-r--r-- | src/mongo/db/pipeline/document_source_sort.cpp | 17 | ||||
-rw-r--r-- | src/mongo/db/pipeline/pipeline_test.cpp | 232 |
3 files changed, 8 insertions, 264 deletions
diff --git a/jstests/sharding/query/agg_mongos_merge.js b/jstests/sharding/query/agg_mongos_merge.js index e917fd2a810..f36e81b6005 100644 --- a/jstests/sharding/query/agg_mongos_merge.js +++ b/jstests/sharding/query/agg_mongos_merge.js @@ -9,7 +9,6 @@ * @tags: [ * requires_sharding, * requires_profiling, - * requires_fcv_46, * ] */ @@ -340,6 +339,14 @@ function runTestCasesWhoseMergeLocationDependsOnAllowDiskUse(allowDiskUse) { // All test cases should merge on mongoD if allowDiskUse is true, mongoS otherwise. const assertMergeOnMongoX = (allowDiskUse ? assertMergeOnMongoD : assertMergeOnMongoS); + // Test that a blocking $sort is only merged on mongoS if 'allowDiskUse' is not set. + assertMergeOnMongoX({ + testName: "agg_mongos_merge_blocking_sort_no_disk_use", + pipeline: [{$match: {_id: {$gte: -200, $lte: 200}}}, {$sort: {_id: -1}}, {$sort: {a: 1}}], + allowDiskUse: allowDiskUse, + expectedCount: 400 + }); + // Test that $group is only merged on mongoS if 'allowDiskUse' is not set. assertMergeOnMongoX({ testName: "agg_mongos_merge_group_allow_disk_use", @@ -349,20 +356,6 @@ function runTestCasesWhoseMergeLocationDependsOnAllowDiskUse(allowDiskUse) { expectedCount: 299 }); - // Adjacent $sort stages will be coalesced and merge sort will occur on anyShard when disk use - // is allowed, and on mongos otherwise. - assertMergeOnMongoX({ - testName: "agg_mongos_merge_blocking_sort_allow_disk_use", - pipeline: [ - {$match: {_id: {$gte: -200, $lte: 200}}}, - {$sort: {_id: 1}}, - {$_internalSplitPipeline: {}}, - {$sort: {a: 1}} - ], - allowDiskUse: allowDiskUse, - expectedCount: 400 - }); - // Test that a blocking $sample is only merged on mongoS if 'allowDiskUse' is not set. assertMergeOnMongoX({ testName: "agg_mongos_merge_blocking_sample_allow_disk_use", diff --git a/src/mongo/db/pipeline/document_source_sort.cpp b/src/mongo/db/pipeline/document_source_sort.cpp index 02290179439..dcf40ba110b 100644 --- a/src/mongo/db/pipeline/document_source_sort.cpp +++ b/src/mongo/db/pipeline/document_source_sort.cpp @@ -169,23 +169,6 @@ Pipeline::SourceContainer::iterator DocumentSourceSort::doOptimizeAt( _sortExecutor->setLimit(*limit); } - if (std::next(itr) == container->end()) { - return container->end(); - } - - if (auto nextSort = dynamic_cast<DocumentSourceSort*>((*std::next(itr)).get())) { - // If subsequent $sort stage exists, optimize by erasing the initial one. - // Since $sort is not guaranteed to be stable, we can blindly remove the first $sort. - auto thisLim = _sortExecutor->getLimit(); - auto otherLim = nextSort->_sortExecutor->getLimit(); - // When coalescing subsequent $sort stages, retain the existing/lower limit. - if (thisLim && (!otherLim || otherLim > thisLim)) { - nextSort->_sortExecutor->setLimit(thisLim); - } - Pipeline::SourceContainer::iterator ret = std::next(itr); - container->erase(itr); - return ret; - } return std::next(itr); } diff --git a/src/mongo/db/pipeline/pipeline_test.cpp b/src/mongo/db/pipeline/pipeline_test.cpp index 239924ce42e..43915889af4 100644 --- a/src/mongo/db/pipeline/pipeline_test.cpp +++ b/src/mongo/db/pipeline/pipeline_test.cpp @@ -293,238 +293,6 @@ TEST(PipelineOptimizationTest, SortMatchProjSkipLimBecomesMatchTopKSortSkipProj) assertPipelineOptimizesAndSerializesTo(inputPipe, outputPipe, serializedPipe); } -TEST(PipelineOptimizationTest, IdenticalSortSortBecomesSort) { - std::string inputPipe = - "[{$sort: {a: 1}}" - ",{$sort: {a: 1}}" - "]"; - - std::string outputPipe = - "[{$sort: {sortKey: {a: 1}}}" - "]"; - - std::string serializedPipe = - "[{$sort: {a: 1}}" - "]"; - - assertPipelineOptimizesAndSerializesTo(inputPipe, outputPipe, serializedPipe); -} - -TEST(PipelineOptimizationTest, IdenticalSortSortSortBecomesSort) { - std::string inputPipe = - "[{$sort: {a: 1}}" - ",{$sort: {a: 1}}" - ",{$sort: {a: 1}}" - "]"; - - std::string outputPipe = - "[{$sort: {sortKey: {a: 1}}}" - "]"; - - std::string serializedPipe = - "[{$sort: {a: 1}}" - "]"; - - assertPipelineOptimizesAndSerializesTo(inputPipe, outputPipe, serializedPipe); -} - -TEST(PipelineOptimizationTest, NonIdenticalSortsOnlySortOnFinalKey) { - std::string inputPipe = - "[{$sort: {a: -1}}" - ",{$sort: {a: 1}}" - ",{$sort: {a: -1}}" - "]"; - - std::string outputPipe = - "[{$sort: {sortKey: {a: -1}}}" - "]"; - - std::string serializedPipe = - "[{$sort: {a: -1}}" - "]"; - - assertPipelineOptimizesAndSerializesTo(inputPipe, outputPipe, serializedPipe); -} - -TEST(PipelineOptimizationTest, SortSortLimitBecomesFinalKeyTopKSort) { - std::string inputPipe = - "[{$sort: {a: -1}}" - ",{$sort: {a: 1}}" - ",{$limit: 5}" - "]"; - - std::string outputPipe = - "[{$sort: {sortKey: {a: 1}, limit: 5}}" - "]"; - - std::string serializedPipe = - "[{$sort: {a: 1}}" - ",{$limit: 5}" - "]"; - - assertPipelineOptimizesAndSerializesTo(inputPipe, outputPipe, serializedPipe); -} - -TEST(PipelineOptimizationTest, SortSortSkipLimitBecomesTopKSortSkip) { - std::string inputPipe = - "[{$sort: {b: 1}}" - ",{$sort: {a: 1}}" - ",{$skip : 3}" - ",{$limit: 5}" - "]"; - - std::string outputPipe = - "[{$sort: {sortKey: {a: 1}, limit: 8}}" - ",{$skip: 3}" - "]"; - - std::string serializedPipe = - "[{$sort: {a: 1}}" - ",{$limit: 8}" - ",{$skip : 3}" - "]"; - - assertPipelineOptimizesAndSerializesTo(inputPipe, outputPipe, serializedPipe); -} - -TEST(PipelineOptimizationTest, SortLimitSortLimitBecomesTopKSort) { - std::string inputPipe = - "[{$sort: {a: 1}}" - ",{$limit: 12}" - ",{$sort: {a: 1}}" - ",{$limit: 20}" - "]"; - - std::string outputPipe = - "[{$sort: {sortKey: {a: 1}, limit: 12}}" - "]"; - - std::string serializedPipe = - "[{$sort: {a: 1}}" - ",{$limit: 12}" - "]"; - - assertPipelineOptimizesAndSerializesTo(inputPipe, outputPipe, serializedPipe); -} - -TEST(PipelineOptimizationTest, SortLimitSortRetainsLimit) { - std::string inputPipe = - "[{$sort: {a: 1}}" - ",{$limit: 12}" - ",{$sort: {a: 1}}" - "]"; - - std::string outputPipe = - "[{$sort: {sortKey: {a: 1}, limit: 12}}" - "]"; - - std::string serializedPipe = - "[{$sort: {a: 1}}" - ",{$limit: 12}" - "]"; - - assertPipelineOptimizesAndSerializesTo(inputPipe, outputPipe, serializedPipe); -} - -TEST(PipelineOptimizationTest, SortSortLimitRetainsLimit) { - std::string inputPipe = - "[{$sort: {a: 1}}" - ",{$sort: {a: 1}}" - ",{$limit: 20}" - "]"; - - std::string outputPipe = - "[{$sort: {sortKey: {a: 1}, limit: 20}}" - "]"; - - std::string serializedPipe = - "[{$sort: {a: 1}}" - ",{$limit: 20}" - "]"; - - assertPipelineOptimizesAndSerializesTo(inputPipe, outputPipe, serializedPipe); -} - -TEST(PipelineOptimizationTest, SortSortSortMatchProjSkipLimBecomesMatchTopKSortSkipProj) { - std::string inputPipe = - "[{$sort: {a: 1}}" - ",{$sort: {a: 1}}" - ",{$sort: {a: 1}}" - ",{$match: {a: 1}}" - ",{$project : {a: 1}}" - ",{$skip : 3}" - ",{$limit: 5}" - "]"; - - std::string outputPipe = - "[{$match: {a: {$eq: 1}}}" - ",{$sort: {sortKey: {a: 1}, limit: 8}}" - ",{$skip: 3}" - ",{$project: {_id: true, a: true}}" - "]"; - - std::string serializedPipe = - "[{$match: {a: 1}}" - ",{$sort: {a: 1}}" - ",{$limit: 8}" - ",{$skip : 3}" - ",{$project : {_id: true, a: true}}" - "]"; - - assertPipelineOptimizesAndSerializesTo(inputPipe, outputPipe, serializedPipe); -} - -TEST(PipelineOptimizationTest, NonIdenticalSortsBecomeFinalKeyTopKSort) { - std::string inputPipe = - "[{$sort: {a: -1}}" - ",{$sort: {b: -1}}" - ",{$sort: {b: 1}}" - ",{$sort: {a: 1}}" - ",{$limit: 7}" - ",{$project : {a: 1}}" - ",{$limit: 5}" - "]"; - - std::string outputPipe = - "[{$sort: {sortKey: {a: 1}, limit: 5}}" - ",{$project: {_id: true, a: true}}" - "]"; - - std::string serializedPipe = - "[{$sort: {a: 1}}" - ",{$limit: 5}" - ",{$project : {_id: true, a: true}}" - "]"; - - assertPipelineOptimizesAndSerializesTo(inputPipe, outputPipe, serializedPipe); -} - -TEST(PipelineOptimizationTest, SubsequentSortsMergeAndBecomeTopKSortWithFinalKeyAndLowestLimit) { - std::string inputPipe = - "[{$sort: {a: 1}}" - ",{$sort: {a: -1}}" - ",{$limit: 8}" - ",{$limit: 7}" - ",{$project : {a: 1}}" - ",{$unwind: {path: '$a'}}" - "]"; - - std::string outputPipe = - "[{$sort: {sortKey: {a: -1}, limit: 7}}" - ",{$project: {_id: true, a: true}}" - ",{$unwind: {path: '$a'}}" - "]"; - - std::string serializedPipe = - "[{$sort: {a: -1}}" - ",{$limit: 7}" - ",{$project : {_id: true, a: true}}" - ",{$unwind: {path: '$a'}}" - "]"; - - assertPipelineOptimizesAndSerializesTo(inputPipe, outputPipe, serializedPipe); -} - TEST(PipelineOptimizationTest, RemoveSkipZero) { assertPipelineOptimizesTo("[{$skip: 0}]", "[]"); } |