diff --git a/buildscripts/resmokeconfig/suites/timeseries_crud_jscore_passthrough.yml b/buildscripts/resmokeconfig/suites/timeseries_crud_jscore_passthrough.yml index 03e6dbf1208..a409f2154b2 100644 --- a/buildscripts/resmokeconfig/suites/timeseries_crud_jscore_passthrough.yml +++ b/buildscripts/resmokeconfig/suites/timeseries_crud_jscore_passthrough.yml @@ -80,6 +80,7 @@ selector: - jstests/core/query/distinct/distinct_compound_index.js - jstests/core/query/distinct/distinct_index1.js - jstests/core/query/distinct/distinct_multikey_dotted_path.js + - jstests/core/query/distinct/distinct_sbe_compatibility.js - jstests/core/query/exists/existsa.js - jstests/core/query/explain/**/*.js - jstests/core/query/expr/expr_index_use.js diff --git a/jstests/aggregation/bugs/sbe_pushdown_replan_reason.js b/jstests/aggregation/bugs/sbe_pushdown_replan_reason.js index 59a99f18204..31bb3594054 100644 --- a/jstests/aggregation/bugs/sbe_pushdown_replan_reason.js +++ b/jstests/aggregation/bugs/sbe_pushdown_replan_reason.js @@ -8,16 +8,9 @@ // does_not_support_stepdowns, // requires_profiling, // not_allowed_with_signed_security_token, -// # TODO SERVER-93694: The test requires SBE to be enabled, but currently -// # featureFlagShardFilteringDistinctScan prevents any query that could potentially use a -// # DistinctScan from running in SBE. Since this flag is FCV gated, upgrade downgrade tests will -// # toggle the enablement of this flag, meaning that we can't guarantee whether or not the flag -// # is enabled when we execute queries that need to run in SBE. Once we allow queries that can't -// # use DistinctScans to run in SBE, we can remove this tag. // cannot_run_during_upgrade_downgrade, // ] -import {FeatureFlagUtil} from "jstests/libs/feature_flag_util.js"; import {getLatestProfilerEntry} from "jstests/libs/profiler.js"; const testDB = db.getSiblingDB(jsTestName()); @@ -74,14 +67,6 @@ function testPushedDownSBEPlanReplanning(match1, match2, pushedDownStage) { })(); (function testPushedDownGroupReplanning() { - // TODO SERVER-93694: The posssiblity of choosing DistinctScans currently make queries SBE - // ineligble since DistinctScan isn't supported in SBE. After this ticket, we should be able to - // create SBE plans if we're unable to produce DistinctScans, making this flag check - // unecessary. - const featureShardFilteringDistinctScan = - FeatureFlagUtil.isEnabled(testDB, "ShardFilteringDistinctScan"); - if (!featureShardFilteringDistinctScan) { - testPushedDownSBEPlanReplanning( - {$match: {a: 5, b: 15}}, {$match: {a: 15, b: 10}}, {$group: {_id: "$a"}}); - } + testPushedDownSBEPlanReplanning( + {$match: {a: 5, b: 15}}, {$match: {a: 15, b: 10}}, {$group: {_id: "$a"}}); })(); diff --git a/jstests/core/query/distinct/distinct_sbe_compatibility.js b/jstests/core/query/distinct/distinct_sbe_compatibility.js new file mode 100644 index 00000000000..b8790416c69 --- /dev/null +++ b/jstests/core/query/distinct/distinct_sbe_compatibility.js @@ -0,0 +1,69 @@ +/** + * Ensures that distinct-like pipelines that aren't suitable for a DISTINCT_SCAN conversion utilize + * SBE (when appropriate). + * + * @tags: [ + * assumes_unsharded_collection, + * # During fcv upgrade/downgrade the engine might not be what we expect. + * cannot_run_during_upgrade_downgrade, + * # Explain does not support stepdowns. + * does_not_support_stepdowns, + * # Explain cannot run within a multi-document transaction. + * does_not_support_transactions, + * featureFlagShardFilteringDistinctScan + * ] + */ +import { + assertEngine, + getPlanStage, + getWinningPlanFromExplain +} from "jstests/libs/query/analyze_plan.js"; +import {checkSbeRestrictedOrFullyEnabled} from "jstests/libs/query/sbe_util.js"; + +if (!checkSbeRestrictedOrFullyEnabled(db)) { + quit(); +} + +const coll = db[jsTestName()]; +coll.drop(); +coll.insertMany([ + {_id: 1, a: 4, b: 2, c: 3, d: 4}, + {_id: 2, a: 4, b: 3, c: 6, d: 5}, + {_id: 3, a: 5, b: 4, c: 7, d: 5} +]); +coll.createIndex({a: 1}); +coll.createIndex({a: 1, b: 1}); +coll.createIndex({a: 1, b: 1, c: 1}); + +const distinctPipelines = [ + [{$group: {_id: "$a"}}], + [{$sort: {a: 1, b: 1}}, {$group: {_id: "$a", accum: {$first: "$b"}}}], + [{$sort: {a: -1, b: -1}}, {$group: {_id: "$a", accum: {$last: "$b"}}}], + [{$group: {_id: "$a", accum: {$top: {sortBy: {a: 1, b: 1}, output: "$c"}}}}], + [{$group: {_id: "$a", accum: {$bottom: {sortBy: {a: -1, b: -1}, output: "$c"}}}}], + [{$match: {a: 1}}, {$sort: {b: 1}}, {$group: {_id: "$b"}}], + [{$match: {a: 1}}, {$sort: {a: 1, b: 1}}, {$group: {_id: "$b"}}], + [{$match: {a: 1, b: 1}}, {$sort: {b: 1, c: 1}}, {$group: {_id: "$b"}}], +]; + +for (const pipeline of distinctPipelines) { + const explain = coll.explain().aggregate(pipeline); + const distinctScanStage = getPlanStage(getWinningPlanFromExplain(explain), "DISTINCT_SCAN"); + assert(distinctScanStage, explain); +} + +const nonDistinctPipelines = [ + [{$group: {_id: "$b"}}], + [{$group: {_id: "$a", accum: {$top: {sortBy: {a: 1, b: -1}, output: "$c"}}}}], + [ + {$sort: {a: 1, b: 1}}, + {$group: {_id: "$a", accum: {$top: {sortBy: {b: 1, a: 1}, output: "$c"}}}} + ], + [ + {$match: {d: {$gt: 3}}}, + {$group: {_id: "$a", accum: {$top: {output: "$b", sortBy: {a: 1, b: 1}}}}} + ], + +]; + +nonDistinctPipelines.forEach(pipeline => assertEngine(pipeline, "sbe", coll)); diff --git a/jstests/libs/pretty_md.js b/jstests/libs/pretty_md.js index c742528b485..ecff537c32b 100644 --- a/jstests/libs/pretty_md.js +++ b/jstests/libs/pretty_md.js @@ -7,6 +7,7 @@ import {normalizeArray, tojsonMultiLineSortKeys} from "jstests/libs/golden_test.js"; import { formatExplainRoot, + getEngine, } from "jstests/libs/query/analyze_plan.js"; let sectionCount = 1; @@ -60,6 +61,9 @@ export function outputAggregationPlanAndResults( code(normalizeArray(results, shouldSortResults)); subSection("Summarized explain"); + if (!explain.hasOwnProperty("shards")) { + line("Execution Engine: " + getEngine(explain)); + } code(tojsonMultiLineSortKeys(flatPlan)); linebreak(); diff --git a/jstests/libs/query/group_with_top_bottom_to_distinct_scan.js b/jstests/libs/query/group_with_top_bottom_to_distinct_scan.js index bde1fa4ca72..091279dfff4 100644 --- a/jstests/libs/query/group_with_top_bottom_to_distinct_scan.js +++ b/jstests/libs/query/group_with_top_bottom_to_distinct_scan.js @@ -14,12 +14,10 @@ import { assertPlanUsesCollScan, assertPlanUsesDistinctScan, assertPlanUsesIndexScan, - coll, removeIndex, } from "jstests/libs/query/group_to_distinct_scan_utils.js"; -import {checkSbeCompletelyDisabled} from "jstests/libs/query/sbe_util.js"; -export function runGroupWithTopBottomToDistinctScanTests(database, isSharded = false) { +export function runGroupWithTopBottomToDistinctScanTests(database) { /** * The tests below should only pass once distinct scan for $top and $bottom accumulators is * achieved. @@ -75,30 +73,22 @@ export function runGroupWithTopBottomToDistinctScanTests(database, isSharded = f assertPlanUsesDistinctScan(database, explain, {"foo.a": 1, "foo.b": 1}), }); + // // Verifies that we _do not_ attempt to use a DISTINCT_SCAN on a multikey dotted-path field. // - // This fails with an error of "cannot sort with keys that are parallel arrays". So, do not run - // this test case on the classic engine. - // - // $top requires sort key metadata for merging the results, so this query is SBE-ineligible when - // the collection is sharded. - if (!isSharded && !checkSbeCompletelyDisabled(database)) { - assertPipelineResultsAndExplain({ - pipeline: [{ - $group: { - _id: "$mkFoo.a", - accum: {$top: {sortBy: {"mkFoo.a": 1, "mkFoo.b": 1}, output: "$mkFoo.b"}} - } - }], - expectedOutput: [ - {_id: null, accum: null}, - {_id: 1, accum: 1}, - {_id: 2, accum: 2}, - {_id: [3, 4], accum: [4, 3]} - ], - validateExplain: assertPlanDoesNotUseDistinctScan, - }); - } + assertPipelineResultsAndExplain({ + pipeline: [ + {$sort: {"mkFoo.a": 1, "mkFoo.b": 1}}, + {$group: {_id: "$mkFoo.a", accum: {$first: "$mkFoo.b"}}} + ], + expectedOutput: [ + {_id: null, accum: null}, + {_id: 1, accum: 1}, + {_id: 2, accum: 2}, + {_id: [3, 4], accum: [4, 3]} + ], + validateExplain: assertPlanDoesNotUseDistinctScan, + }); // // Verifies that we use a DISTINCT_SCAN to satisfy a sort on a multikey field if the bounds diff --git a/jstests/noPassthrough/currentop_query.js b/jstests/noPassthrough/currentop_query.js index 418450111cf..edb3f3c9e00 100644 --- a/jstests/noPassthrough/currentop_query.js +++ b/jstests/noPassthrough/currentop_query.js @@ -257,13 +257,7 @@ function runTests({conn, currentOp, truncatedOps, localOps}) { }, command: "distinct", planSummary: "COLLSCAN", - /* Distinct multiplanning does not generate SBE plans, even if the resulting query - solution doesn't contain a DISTINCT_SCAN. */ - queryFramework: - (!FeatureFlagUtil.isPresentAndEnabled(conn, "ShardFilteringDistinctScan") && - sbeEnabled) - ? "sbe" - : "classic", + queryFramework: sbeEnabled ? "sbe" : "classic", currentOpFilter: {"command.collation.locale": "fr"} }, { diff --git a/jstests/query_golden/expected_output/distinct_aggregation_multiplanning.md b/jstests/query_golden/expected_output/distinct_aggregation_multiplanning.md index 073c7bee9ec..dbccb3f0b06 100644 --- a/jstests/query_golden/expected_output/distinct_aggregation_multiplanning.md +++ b/jstests/query_golden/expected_output/distinct_aggregation_multiplanning.md @@ -24,6 +24,7 @@ { "_id" : 5, "accum" : 4 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -191,6 +192,7 @@ { "_id" : 5, "accum" : 4 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -311,6 +313,7 @@ { "_id" : 5, "accum" : 4 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -478,6 +481,7 @@ { "_id" : 5, "accum" : 7 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -630,6 +634,7 @@ { "_id" : 5, "accum" : 7 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -782,6 +787,7 @@ { "_id" : 5, "accum" : 7 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -881,6 +887,7 @@ { "_id" : 3, "accum" : 4 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -947,6 +954,7 @@ { "_id" : 5, "accum" : 4 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -1067,6 +1075,7 @@ { "_id" : 5, "accum" : 4 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -1181,6 +1190,7 @@ { "_id" : 5, "accum" : 4 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -1262,6 +1272,7 @@ { "_id" : 5, "accum" : 6 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -1343,6 +1354,7 @@ { "_id" : 5, "accum" : 6 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -1429,6 +1441,7 @@ { "_id" : 5, "accum" : 4 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -1515,6 +1528,7 @@ { "_id" : 5, "accum" : 4 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -1600,6 +1614,7 @@ { "_id" : 5, "accum" : 4 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -1686,6 +1701,7 @@ { "_id" : 5, "accum" : 7 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -1764,6 +1780,7 @@ { "_id" : 5, "accum" : 7 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -1840,6 +1857,7 @@ { "_id" : 5, "accum" : 4 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -2007,6 +2025,7 @@ { "_id" : 5, "accum" : 4 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -2181,6 +2200,7 @@ { "_id" : 5, "accum" : 4 } ``` ### Summarized explain +Execution Engine: sbe ```json { "rejectedPlans" : [ ], @@ -2261,6 +2281,7 @@ { "_id" : 5, "accum" : 4 } ``` ### Summarized explain +Execution Engine: sbe ```json { "rejectedPlans" : [ ], @@ -2308,6 +2329,7 @@ { "_id" : 5 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -2381,6 +2403,7 @@ { "_id" : 5, "accumB" : 4, "accumC" : 7 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -2471,6 +2494,7 @@ { "_id" : 5, "accumB" : 4, "accumC" : 7, "accumD" : 5 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -2552,6 +2576,7 @@ { "_id" : 4, "accum" : -4 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -2702,6 +2727,7 @@ { "_id" : 9, "accum" : -9 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -2841,154 +2867,138 @@ { "_id" : 5, "accum" : 7 } ``` ### Summarized explain +Execution Engine: sbe ```json { - "stages" : [ - { - "$cursor" : { - "rejectedPlans" : [ - [ - { - "stage" : "PROJECTION_SIMPLE", - "transformBy" : { - "_id" : 0, - "a" : 1, - "b" : 1, - "c" : 1 - } - }, - { - "stage" : "FETCH" - }, - { - "direction" : "forward", - "indexBounds" : { - "a" : [ - "[MinKey, MaxKey]" - ], - "b" : [ - "[MinKey, MaxKey]" - ] - }, - "indexName" : "a_1_b_1", - "isMultiKey" : false, - "isPartial" : false, - "isSparse" : false, - "isUnique" : false, - "keyPattern" : { - "a" : 1, - "b" : 1 - }, - "multiKeyPaths" : { - "a" : [ ], - "b" : [ ] - }, - "stage" : "IXSCAN" - } + "rejectedPlans" : [ + [ + { + "stage" : "PROJECTION_SIMPLE", + "transformBy" : { + "_id" : 0, + "a" : 1, + "b" : 1, + "c" : 1 + } + }, + { + "stage" : "FETCH" + }, + { + "direction" : "forward", + "indexBounds" : { + "a" : [ + "[MinKey, MaxKey]" ], - [ - { - "stage" : "PROJECTION_SIMPLE", - "transformBy" : { - "_id" : 0, - "a" : 1, - "b" : 1, - "c" : 1 - } - }, - { - "stage" : "FETCH" - }, - { - "direction" : "forward", - "indexBounds" : { - "a" : [ - "[MinKey, MaxKey]" - ], - "b" : [ - "[MinKey, MaxKey]" - ], - "d" : [ - "[MinKey, MaxKey]" - ] - }, - "indexName" : "a_1_b_1_d_1", - "isMultiKey" : true, - "isPartial" : false, - "isSparse" : false, - "isUnique" : false, - "keyPattern" : { - "a" : 1, - "b" : 1, - "d" : 1 - }, - "multiKeyPaths" : { - "a" : [ ], - "b" : [ ], - "d" : [ - "d" - ] - }, - "stage" : "IXSCAN" - } + "b" : [ + "[MinKey, MaxKey]" ] - ], - "winningPlan" : [ - { - "stage" : "PROJECTION_COVERED", - "transformBy" : { - "_id" : 0, - "a" : 1, - "b" : 1, - "c" : 1 - } - }, - { - "direction" : "forward", - "indexBounds" : { - "a" : [ - "[MinKey, MaxKey]" - ], - "b" : [ - "[MinKey, MaxKey]" - ], - "c" : [ - "[MinKey, MaxKey]" - ] - }, - "indexName" : "a_1_b_1_c_1", - "isMultiKey" : false, - "isPartial" : false, - "isSparse" : false, - "isUnique" : false, - "keyPattern" : { - "a" : 1, - "b" : 1, - "c" : 1 - }, - "multiKeyPaths" : { - "a" : [ ], - "b" : [ ], - "c" : [ ] - }, - "stage" : "IXSCAN" - } - ] + }, + "indexName" : "a_1_b_1", + "isMultiKey" : false, + "isPartial" : false, + "isSparse" : false, + "isUnique" : false, + "keyPattern" : { + "a" : 1, + "b" : 1 + }, + "multiKeyPaths" : { + "a" : [ ], + "b" : [ ] + }, + "stage" : "IXSCAN" + } + ], + [ + { + "stage" : "PROJECTION_SIMPLE", + "transformBy" : { + "_id" : 0, + "a" : 1, + "b" : 1, + "c" : 1 + } + }, + { + "stage" : "FETCH" + }, + { + "direction" : "forward", + "indexBounds" : { + "a" : [ + "[MinKey, MaxKey]" + ], + "b" : [ + "[MinKey, MaxKey]" + ], + "d" : [ + "[MinKey, MaxKey]" + ] + }, + "indexName" : "a_1_b_1_d_1", + "isMultiKey" : true, + "isPartial" : false, + "isSparse" : false, + "isUnique" : false, + "keyPattern" : { + "a" : 1, + "b" : 1, + "d" : 1 + }, + "multiKeyPaths" : { + "a" : [ ], + "b" : [ ], + "d" : [ + "d" + ] + }, + "stage" : "IXSCAN" + } + ] + ], + "winningPlan" : [ + { + "stage" : "GROUP" + }, + { + "stage" : "PROJECTION_COVERED", + "transformBy" : { + "_id" : false, + "a" : true, + "b" : true, + "c" : true } }, { - "$group" : { - "_id" : "$a", - "accum" : { - "$top" : { - "output" : "$c", - "sortBy" : { - "a" : 1, - "b" : 1 - } - } - } - } + "direction" : "forward", + "indexBounds" : { + "a" : [ + "[MinKey, MaxKey]" + ], + "b" : [ + "[MinKey, MaxKey]" + ], + "c" : [ + "[MinKey, MaxKey]" + ] + }, + "indexName" : "a_1_b_1_c_1", + "isMultiKey" : false, + "isPartial" : false, + "isSparse" : false, + "isUnique" : false, + "keyPattern" : { + "a" : 1, + "b" : 1, + "c" : 1 + }, + "multiKeyPaths" : { + "a" : [ ], + "b" : [ ], + "c" : [ ] + }, + "stage" : "IXSCAN" } ] } @@ -3025,154 +3035,138 @@ { "_id" : 5, "accum" : 7 } ``` ### Summarized explain +Execution Engine: sbe ```json { - "stages" : [ - { - "$cursor" : { - "rejectedPlans" : [ - [ - { - "stage" : "PROJECTION_SIMPLE", - "transformBy" : { - "_id" : 0, - "a" : 1, - "b" : 1, - "c" : 1 - } - }, - { - "stage" : "FETCH" - }, - { - "direction" : "forward", - "indexBounds" : { - "a" : [ - "[MinKey, MaxKey]" - ], - "b" : [ - "[MinKey, MaxKey]" - ] - }, - "indexName" : "a_1_b_1", - "isMultiKey" : false, - "isPartial" : false, - "isSparse" : false, - "isUnique" : false, - "keyPattern" : { - "a" : 1, - "b" : 1 - }, - "multiKeyPaths" : { - "a" : [ ], - "b" : [ ] - }, - "stage" : "IXSCAN" - } + "rejectedPlans" : [ + [ + { + "stage" : "PROJECTION_SIMPLE", + "transformBy" : { + "_id" : 0, + "a" : 1, + "b" : 1, + "c" : 1 + } + }, + { + "stage" : "FETCH" + }, + { + "direction" : "forward", + "indexBounds" : { + "a" : [ + "[MinKey, MaxKey]" ], - [ - { - "stage" : "PROJECTION_SIMPLE", - "transformBy" : { - "_id" : 0, - "a" : 1, - "b" : 1, - "c" : 1 - } - }, - { - "stage" : "FETCH" - }, - { - "direction" : "forward", - "indexBounds" : { - "a" : [ - "[MinKey, MaxKey]" - ], - "b" : [ - "[MinKey, MaxKey]" - ], - "d" : [ - "[MinKey, MaxKey]" - ] - }, - "indexName" : "a_1_b_1_d_1", - "isMultiKey" : true, - "isPartial" : false, - "isSparse" : false, - "isUnique" : false, - "keyPattern" : { - "a" : 1, - "b" : 1, - "d" : 1 - }, - "multiKeyPaths" : { - "a" : [ ], - "b" : [ ], - "d" : [ - "d" - ] - }, - "stage" : "IXSCAN" - } + "b" : [ + "[MinKey, MaxKey]" ] - ], - "winningPlan" : [ - { - "stage" : "PROJECTION_COVERED", - "transformBy" : { - "_id" : 0, - "a" : 1, - "b" : 1, - "c" : 1 - } - }, - { - "direction" : "forward", - "indexBounds" : { - "a" : [ - "[MinKey, MaxKey]" - ], - "b" : [ - "[MinKey, MaxKey]" - ], - "c" : [ - "[MinKey, MaxKey]" - ] - }, - "indexName" : "a_1_b_1_c_1", - "isMultiKey" : false, - "isPartial" : false, - "isSparse" : false, - "isUnique" : false, - "keyPattern" : { - "a" : 1, - "b" : 1, - "c" : 1 - }, - "multiKeyPaths" : { - "a" : [ ], - "b" : [ ], - "c" : [ ] - }, - "stage" : "IXSCAN" - } - ] + }, + "indexName" : "a_1_b_1", + "isMultiKey" : false, + "isPartial" : false, + "isSparse" : false, + "isUnique" : false, + "keyPattern" : { + "a" : 1, + "b" : 1 + }, + "multiKeyPaths" : { + "a" : [ ], + "b" : [ ] + }, + "stage" : "IXSCAN" + } + ], + [ + { + "stage" : "PROJECTION_SIMPLE", + "transformBy" : { + "_id" : 0, + "a" : 1, + "b" : 1, + "c" : 1 + } + }, + { + "stage" : "FETCH" + }, + { + "direction" : "forward", + "indexBounds" : { + "a" : [ + "[MinKey, MaxKey]" + ], + "b" : [ + "[MinKey, MaxKey]" + ], + "d" : [ + "[MinKey, MaxKey]" + ] + }, + "indexName" : "a_1_b_1_d_1", + "isMultiKey" : true, + "isPartial" : false, + "isSparse" : false, + "isUnique" : false, + "keyPattern" : { + "a" : 1, + "b" : 1, + "d" : 1 + }, + "multiKeyPaths" : { + "a" : [ ], + "b" : [ ], + "d" : [ + "d" + ] + }, + "stage" : "IXSCAN" + } + ] + ], + "winningPlan" : [ + { + "stage" : "GROUP" + }, + { + "stage" : "PROJECTION_COVERED", + "transformBy" : { + "_id" : false, + "a" : true, + "b" : true, + "c" : true } }, { - "$group" : { - "_id" : "$a", - "accum" : { - "$bottom" : { - "output" : "$c", - "sortBy" : { - "a" : -1, - "b" : -1 - } - } - } - } + "direction" : "forward", + "indexBounds" : { + "a" : [ + "[MinKey, MaxKey]" + ], + "b" : [ + "[MinKey, MaxKey]" + ], + "c" : [ + "[MinKey, MaxKey]" + ] + }, + "indexName" : "a_1_b_1_c_1", + "isMultiKey" : false, + "isPartial" : false, + "isSparse" : false, + "isUnique" : false, + "keyPattern" : { + "a" : 1, + "b" : 1, + "c" : 1 + }, + "multiKeyPaths" : { + "a" : [ ], + "b" : [ ], + "c" : [ ] + }, + "stage" : "IXSCAN" } ] } @@ -3205,6 +3199,7 @@ { "_id" : [ 1, 2, 3 ], "accum" : 4 } ``` ### Summarized explain +Execution Engine: sbe ```json { "rejectedPlans" : [ @@ -3366,6 +3361,7 @@ { "_id" : [ 1, 2, 3 ], "accum" : 4 } ``` ### Summarized explain +Execution Engine: sbe ```json { "rejectedPlans" : [ ], diff --git a/jstests/query_golden/expected_output/distinct_index_eligiblity.md b/jstests/query_golden/expected_output/distinct_index_eligiblity.md index 65e3173f0b9..d162465563c 100644 --- a/jstests/query_golden/expected_output/distinct_index_eligiblity.md +++ b/jstests/query_golden/expected_output/distinct_index_eligiblity.md @@ -24,6 +24,7 @@ { "_id" : [ 1, 2, 3 ], "accum" : 5 } ``` ### Summarized explain +Execution Engine: sbe ```json { "rejectedPlans" : [ ], @@ -90,6 +91,7 @@ { "_id" : 1, "accum" : 5 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -172,6 +174,7 @@ { "_id" : [ 1, 2, 3 ], "accum" : 5 } ``` ### Summarized explain +Execution Engine: sbe ```json { "rejectedPlans" : [ ], @@ -215,6 +218,7 @@ { "_id" : 1, "accum" : [ 1, 2, 3 ] } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -418,7 +422,7 @@ "indexName" : "$**_1", "isMultiKey" : false, "isPartial" : false, - "isSparse" : false, + "isSparse" : true, "isUnique" : false, "keyPattern" : { "$_path" : 1, diff --git a/jstests/query_golden/expected_output/distinct_query_planner.md b/jstests/query_golden/expected_output/distinct_query_planner.md index 9d20fa925f9..a5ba7fa123d 100644 --- a/jstests/query_golden/expected_output/distinct_query_planner.md +++ b/jstests/query_golden/expected_output/distinct_query_planner.md @@ -25,6 +25,7 @@ { "_id" : 2, "accum" : 3 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -108,6 +109,7 @@ { "_id" : 2, "accum" : 3 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -191,41 +193,20 @@ { "_id" : 2, "accum" : 3 } ``` ### Summarized explain +Execution Engine: sbe ```json { - "stages" : [ + "rejectedPlans" : [ ], + "winningPlan" : [ { - "$cursor" : { - "rejectedPlans" : [ ], - "winningPlan" : [ - { - "stage" : "PROJECTION_SIMPLE", - "transformBy" : { - "_id" : 0, - "a" : 1, - "b" : 1 - } - }, - { - "direction" : "forward", - "stage" : "COLLSCAN" - } - ] - } + "stage" : "GROUP" }, { - "$group" : { - "_id" : "$a", - "accum" : { - "$top" : { - "output" : "$b", - "sortBy" : { - "a" : 1, - "b" : 1 - } - } - } - } + "direction" : "forward", + "filter" : { + + }, + "stage" : "COLLSCAN" } ] } @@ -265,60 +246,36 @@ { "_id" : 7, "accum" : 3 } ``` ### Summarized explain +Execution Engine: sbe ```json { - "stages" : [ + "rejectedPlans" : [ ], + "winningPlan" : [ { - "$cursor" : { - "rejectedPlans" : [ ], - "winningPlan" : [ - { - "stage" : "PROJECTION_SIMPLE", - "transformBy" : { - "_id" : 0, - "a" : 1, - "b" : 1 - } - }, - { - "stage" : "FETCH" - }, - { - "direction" : "forward", - "indexBounds" : { - "a" : [ - "(3.0, inf.0]" - ] - }, - "indexName" : "a_1", - "isMultiKey" : false, - "isPartial" : false, - "isSparse" : false, - "isUnique" : false, - "keyPattern" : { - "a" : 1 - }, - "multiKeyPaths" : { - "a" : [ ] - }, - "stage" : "IXSCAN" - } - ] - } + "stage" : "GROUP" }, { - "$group" : { - "_id" : "$a", - "accum" : { - "$top" : { - "output" : "$b", - "sortBy" : { - "a" : 1, - "b" : 1 - } - } - } - } + "stage" : "FETCH" + }, + { + "direction" : "forward", + "indexBounds" : { + "a" : [ + "(3.0, inf.0]" + ] + }, + "indexName" : "a_1", + "isMultiKey" : false, + "isPartial" : false, + "isSparse" : false, + "isUnique" : false, + "keyPattern" : { + "a" : 1 + }, + "multiKeyPaths" : { + "a" : [ ] + }, + "stage" : "IXSCAN" } ] } @@ -336,6 +293,7 @@ { "_id" : 2 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -397,31 +355,20 @@ { "_id" : 2 } ``` ### Summarized explain +Execution Engine: sbe ```json { - "stages" : [ + "rejectedPlans" : [ ], + "winningPlan" : [ { - "$cursor" : { - "rejectedPlans" : [ ], - "winningPlan" : [ - { - "stage" : "PROJECTION_SIMPLE", - "transformBy" : { - "_id" : 0, - "a" : 1 - } - }, - { - "direction" : "forward", - "stage" : "COLLSCAN" - } - ] - } + "stage" : "GROUP" }, { - "$group" : { - "_id" : "$a" - } + "direction" : "forward", + "filter" : { + + }, + "stage" : "COLLSCAN" } ] } diff --git a/jstests/query_golden/expected_output/distinct_scan.md b/jstests/query_golden/expected_output/distinct_scan.md index d7ffd48f1d9..32137353537 100644 --- a/jstests/query_golden/expected_output/distinct_scan.md +++ b/jstests/query_golden/expected_output/distinct_scan.md @@ -34,6 +34,7 @@ { "_id" : 7 } ``` ### Summarized explain +Execution Engine: sbe ```json { "rejectedPlans" : [ ], @@ -110,6 +111,7 @@ { "_id" : 7 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -227,6 +229,7 @@ { "_id" : 7, "firstField" : 1 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -312,6 +315,7 @@ { "_id" : 7, "firstField" : 1 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -422,6 +426,7 @@ { "_id" : 7, "firstField" : 1 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -585,6 +590,7 @@ { "_id" : 3 } ``` ### Summarized explain +Execution Engine: classic ```json { "stages" : [ @@ -732,50 +738,36 @@ { "_id" : 4 } ``` ### Summarized explain +Execution Engine: sbe ```json { - "stages" : [ + "rejectedPlans" : [ ], + "winningPlan" : [ { - "$cursor" : { - "rejectedPlans" : [ ], - "winningPlan" : [ - { - "stage" : "PROJECTION_SIMPLE", - "transformBy" : { - "_id" : 0, - "a" : 1 - } - }, - { - "stage" : "FETCH" - }, - { - "direction" : "forward", - "indexBounds" : { - "b" : [ - "[3.0, 3.0]" - ] - }, - "indexName" : "b_1", - "isMultiKey" : false, - "isPartial" : false, - "isSparse" : false, - "isUnique" : false, - "keyPattern" : { - "b" : 1 - }, - "multiKeyPaths" : { - "b" : [ ] - }, - "stage" : "IXSCAN" - } - ] - } + "stage" : "GROUP" }, { - "$group" : { - "_id" : "$a" - } + "stage" : "FETCH" + }, + { + "direction" : "forward", + "indexBounds" : { + "b" : [ + "[3.0, 3.0]" + ] + }, + "indexName" : "b_1", + "isMultiKey" : false, + "isPartial" : false, + "isSparse" : false, + "isUnique" : false, + "keyPattern" : { + "b" : 1 + }, + "multiKeyPaths" : { + "b" : [ ] + }, + "stage" : "IXSCAN" } ] } diff --git a/jstests/sharding/query/group_with_top_bottom_to_distinct_scan_with_orphans.js b/jstests/sharding/query/group_with_top_bottom_to_distinct_scan_with_orphans.js index 27b17c4ba4b..2fc7630a8f3 100644 --- a/jstests/sharding/query/group_with_top_bottom_to_distinct_scan_with_orphans.js +++ b/jstests/sharding/query/group_with_top_bottom_to_distinct_scan_with_orphans.js @@ -25,6 +25,6 @@ TestData.skipCheckOrphans = true; const st = new ShardingTest({shards: 2}); const db = prepareShardedCollectionWithOrphans(st); -runGroupWithTopBottomToDistinctScanTests(db, true); +runGroupWithTopBottomToDistinctScanTests(db); st.stop(); diff --git a/src/mongo/base/error_codes.yml b/src/mongo/base/error_codes.yml index f557544a590..fdae6b92a6c 100644 --- a/src/mongo/base/error_codes.yml +++ b/src/mongo/base/error_codes.yml @@ -811,6 +811,7 @@ error_codes: - {code: 431, name: ChunkMetadataInconsistency} - {code: 432, name: OfflineValidationFailedToComplete} - {code: 433, name: AdmissionQueueOverflow} + - {code: 434, name: NoDistinctScansForDistinctEligibleQuery} # Error codes 4000-8999 are reserved. diff --git a/src/mongo/db/pipeline/pipeline_d.cpp b/src/mongo/db/pipeline/pipeline_d.cpp index 4266c8c831d..29dc091f16b 100644 --- a/src/mongo/db/pipeline/pipeline_d.cpp +++ b/src/mongo/db/pipeline/pipeline_d.cpp @@ -1191,9 +1191,6 @@ tryPrepareDistinctExecutor(const intrusive_ptr& expCtx, // In the context of distinct multiplanning, if there are no indexes suitable for distinct // scans, we can omit the distinct part of the canonical query. For example, this will // remove the SBE ineligibilty criteria for queries that have a distinct component. - // - // TODO SERVER-93694: This initial pruning is just a temporary fix, since we still can end - // up with non-distinct scan solutions which otherwise could have been executed with SBE. auto plannerParams = std::make_unique(QueryPlannerParams::ArgsForDistinct{ cq->getExpCtx()->getOperationContext(), diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp index cda97c23a3b..66284b8b8ce 100644 --- a/src/mongo/db/query/get_executor.cpp +++ b/src/mongo/db/query/get_executor.cpp @@ -720,7 +720,16 @@ private: } auto cachedSolutionPair = retrievePlanFromCache(planCacheKey); - if (!cachedSolutionPair.has_value()) { + + const bool noCachedPlan = !cachedSolutionPair.has_value(); + const bool cachedPlanIsNotForClassic = !noCachedPlan && + cachedSolutionPair->first->decisionReadsOrWorks && + !std::holds_alternative( + cachedSolutionPair->first->decisionReadsOrWorks->data); + + // If cachedSolutionPair is empty or the stored entry uses NumReads instead of NumWorks, we + // cannot use it. + if (noCachedPlan || cachedPlanIsNotForClassic) { planCacheCounters.incrementClassicMissesCounter(); return nullptr; } @@ -1264,23 +1273,24 @@ StatusWith> getExecutorFind return sbeFull || shouldUseRegularSbe(opCtx, *canonicalQuery, sbeFull); }(); - if (useSbeEngine) { + canonicalQuery->setSbeCompatible(useSbeEngine); + if (!useSbeEngine) { + // There's a special case of the projection optimization being skipped when a query has + // any user-defined "let" variable and the query may be run with SBE. Here we make sure + // the projection is optimized for the classic engine. + canonicalQuery->optimizeProjection(); + } else if (!canonicalQuery->getDistinct()) { // Commit to using SBE by removing the pushed-down aggregation stages from the original // pipeline and by mutating the canonical query with search specific metadata. finalizePipelineStages(pipeline, unavailableMetadata, canonicalQuery.get()); - canonicalQuery->setSbeCompatible(true); - - } else { - // There's a special case of the projection optimization being skipped when a query has any - // user-defined "let" variable and the query may be run with SBE. Here we make sure the - // projection is optimized for the classic engine. - canonicalQuery->optimizeProjection(); - canonicalQuery->setSbeCompatible(false); } + auto makePlanner = [&](std::unique_ptr plannerParams) -> std::unique_ptr { - if (useSbeEngine) { + // If we have a distinct, we might get a better plan using classic and DISTINCT_SCAN than + // SBE without one. + if (useSbeEngine && !canonicalQuery->getDistinct()) { auto sbeYieldPolicy = PlanYieldPolicySBE::make(opCtx, yieldPolicy, collections, canonicalQuery->nss()); @@ -1323,27 +1333,40 @@ StatusWith> getExecutorFind auto planner = [&] { try { - // First try the single collection query parameters, as these would have been generated - // with query settings if present. - return makePlanner(std::move(paramsForSingleCollectionQuery)); - } catch (const ExceptionFor& exception) { - // The planner failed to generate a viable plan. Remove the query settings and retry if - // any are present. Otherwise just propagate the exception. - const auto& querySettings = canonicalQuery->getExpCtx()->getQuerySettings(); - const bool hasQuerySettings = querySettings.getIndexHints().has_value(); - if (!hasQuerySettings) { - throw; + try { + // First try the single collection query parameters, as these would have been + // generated with query settings if present. + return makePlanner(std::move(paramsForSingleCollectionQuery)); + } catch (const ExceptionFor& exception) { + // The planner failed to generate a viable plan. Remove the query settings and retry + // if any are present. Otherwise just propagate the exception. + const auto& querySettings = canonicalQuery->getExpCtx()->getQuerySettings(); + const bool hasQuerySettings = querySettings.getIndexHints().has_value(); + if (!hasQuerySettings) { + throw; + } + LOGV2_DEBUG( + 8524200, + 2, + "Encountered planning error while running with query settings. Retrying " + "without query settings.", + "query"_attr = canonicalQuery->toStringForErrorMsg(), + "querySettings"_attr = querySettings, + "reason"_attr = exception.reason(), + "code"_attr = exception.codeString()); + + plannerOptions |= QueryPlannerParams::IGNORE_QUERY_SETTINGS; + return makePlanner(makeQueryPlannerParams(plannerOptions)); } - LOGV2_DEBUG(8524200, - 2, - "Encountered planning error while running with query settings. Retrying " - "without query settings.", - "query"_attr = canonicalQuery->toStringForErrorMsg(), - "querySettings"_attr = querySettings, - "reason"_attr = exception.reason(), - "code"_attr = exception.codeString()); - return makePlanner( - makeQueryPlannerParams(plannerOptions | QueryPlannerParams::IGNORE_QUERY_SETTINGS)); + } catch (const ExceptionFor&) { + // The planner failed to generate a DISTINCT_SCAN for a distinct-like query. Replan + // using SBE. + tassert( + 936940, "Expected query to be SBE-compatible", canonicalQuery->isSbeCompatible()); + canonicalQuery->resetDistinct(); + // Stages still need to be finalized for SBE since classic was used previously. + finalizePipelineStages(pipeline, unavailableMetadata, canonicalQuery.get()); + return makePlanner(makeQueryPlannerParams(plannerOptions)); } }(); auto exec = planner->makeExecutor(std::move(canonicalQuery)); diff --git a/src/mongo/db/query/query_planner.cpp b/src/mongo/db/query/query_planner.cpp index 3dc300f25cf..c022ac66388 100644 --- a/src/mongo/db/query/query_planner.cpp +++ b/src/mongo/db/query/query_planner.cpp @@ -1614,6 +1614,18 @@ StatusWith>> QueryPlanner::plan( "No indexed plans available, and running with 'notablescan' 2"); } + // If CanonicalQuery is distinct-like and we haven't generated a plan that features + // a DISTINCT_SCAN, we should use SBE instead. + if (query.isSbeCompatible() && query.getDistinct()) { + const bool noDistinctScans = std::none_of(out.begin(), out.end(), [](const auto& soln) { + return soln->hasNode(STAGE_DISTINCT_SCAN); + }); + if (noDistinctScans) { + return Status(ErrorCodes::NoDistinctScansForDistinctEligibleQuery, + "No DISTINCT_SCAN plans available"); + } + } + return {std::move(out)}; } // QueryPlanner::plan diff --git a/src/mongo/db/query/query_planner_distinct_test.cpp b/src/mongo/db/query/query_planner_distinct_test.cpp index 183103b3465..e4fde536428 100644 --- a/src/mongo/db/query/query_planner_distinct_test.cpp +++ b/src/mongo/db/query/query_planner_distinct_test.cpp @@ -70,6 +70,12 @@ public: flipDistinctScanDirection)); auto statusWithMultiPlanSolns = QueryPlanner::plan(*cq, params); + if (statusWithMultiPlanSolns.getStatus().code() == + ErrorCodes::NoDistinctScansForDistinctEligibleQuery) { + cq->resetDistinct(); + statusWithMultiPlanSolns = QueryPlanner::plan(*cq, params); + } + ASSERT_OK(statusWithMultiPlanSolns.getStatus()); solns = std::move(statusWithMultiPlanSolns.getValue()); } diff --git a/src/mongo/db/query/query_utils.cpp b/src/mongo/db/query/query_utils.cpp index 8918e2e2313..702378aa4e5 100644 --- a/src/mongo/db/query/query_utils.cpp +++ b/src/mongo/db/query/query_utils.cpp @@ -101,11 +101,6 @@ bool isQuerySbeCompatible(const CollectionPtr* collection, const CanonicalQuery* return false; } - // In the context of distinct multiplanning, distinct-like queries are not eligible for SBE yet. - if (cq->getExpCtx()->isFeatureFlagShardFilteringDistinctScanEnabled() && cq->getDistinct()) { - return false; - } - const auto* proj = cq->getProj(); if (proj && (proj->requiresMatchDetails() || proj->containsElemMatch())) { return false;