From 55f5797dced8a6d4ba3f3c0c2368d6073a95aed7 Mon Sep 17 00:00:00 2001 From: venti <1308199824@qq.com> Date: Fri, 29 May 2026 21:45:02 +0800 Subject: [PATCH 1/2] @ fix(query-insights): traverse shard branches in planner-tree helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `findStageInPlan()` and `collectPlannerStages()` only traversed inputStage and inputStages — they missed stages nested under shards structures in sharded cluster explain plans. This meant planner-stage signals like isBitmap (bitmap index) and low-cardinality index detection could be silently missed. - Extend findStageInPlan() to recurse through shards arrays (matching traverseStage / checkStageForSort behavior). - Extend collectPlannerStages() similarly. - Add two sharded test cases: IXSCAN with isBitmap under a SHARD_MERGE > shards > SINGLE_SHARD path, and a mixed-case where planner is sharded but exec stats are flat. Closes #620 @ --- ...nalyzer.addIndexStrategyAdvisories.test.ts | 110 ++++++++++++++++++ .../queryInsights/ExplainPlanAnalyzer.ts | 10 ++ .../queryInsights/StagePropertyExtractor.ts | 6 + 3 files changed, 126 insertions(+) diff --git a/src/documentdb/queryInsights/ExplainPlanAnalyzer.addIndexStrategyAdvisories.test.ts b/src/documentdb/queryInsights/ExplainPlanAnalyzer.addIndexStrategyAdvisories.test.ts index 1750b8e1b..2f37c115a 100644 --- a/src/documentdb/queryInsights/ExplainPlanAnalyzer.addIndexStrategyAdvisories.test.ts +++ b/src/documentdb/queryInsights/ExplainPlanAnalyzer.addIndexStrategyAdvisories.test.ts @@ -288,6 +288,116 @@ describe('ExplainPlanAnalyzer.addIndexStrategyAdvisories', () => { }); }); + it('detects isBitmap under a shard branch in the winning plan', () => { + // Simulates a sharded cluster where the IXSCAN with isBitmap lives + // under a SHARD_MERGE > shards > FETCH > IXSCAN path. + // Before the fix, findStageInPlan() did not recurse into shards. + const analysis = makeAnalysis(); + const explainResult: Document = { + queryPlanner: { + winningPlan: { + stage: 'SHARD_MERGE', + shards: [ + { + stage: 'SINGLE_SHARD', + inputStage: { + stage: 'FETCH', + inputStage: { + stage: 'IXSCAN', + indexName: 'bitmapIndex_1', + isBitmap: true, + }, + }, + }, + ], + }, + }, + executionStats: { + nReturned: 100, + executionTimeMillis: 13, + totalDocsExamined: 100, + totalKeysExamined: 100, + executionStages: { + stage: 'SHARD_MERGE', + shards: [ + { + stage: 'SINGLE_SHARD', + inputStage: { + stage: 'FETCH', + inputStage: { + stage: 'IXSCAN', + indexName: 'bitmapIndex_1', + indexUsage: [ + { + scanKeys: [ + 'key 1: [(isInequality: false, estimatedEntryCount: 100)]', + ], + }, + ], + }, + }, + }, + ], + }, + }, + }; + + ExplainPlanAnalyzer.addIndexStrategyAdvisories(analysis, 1000, explainResult); + + const ids = getDiagnosticIds(analysis.performanceRating.diagnostics); + expect(ids).toContain('bitmap_index'); + }); + + it('detects IXSCAN in planner shards even when exec stats use flat structure', () => { + // Edge case: planner is sharded, execution stats may not be + const analysis = makeAnalysis(); + const explainResult: Document = { + queryPlanner: { + winningPlan: { + stage: 'SHARD_MERGE', + shards: [ + { + stage: 'SINGLE_SHARD', + inputStage: { + stage: 'FETCH', + inputStage: { + stage: 'IXSCAN', + indexName: 'bitmapIndex_1', + isBitmap: true, + }, + }, + }, + ], + }, + }, + executionStats: { + nReturned: 100, + executionTimeMillis: 13, + totalDocsExamined: 100, + totalKeysExamined: 100, + executionStages: { + stage: 'FETCH', + inputStage: { + stage: 'IXSCAN', + indexName: 'bitmapIndex_1', + indexUsage: [ + { + scanKeys: [ + 'key 1: [(isInequality: false, estimatedEntryCount: 100)]', + ], + }, + ], + }, + }, + }, + }; + + ExplainPlanAnalyzer.addIndexStrategyAdvisories(analysis, 1000, explainResult); + + const ids = getDiagnosticIds(analysis.performanceRating.diagnostics); + expect(ids).toContain('bitmap_index'); + }); + describe('cumulative advisory demotions', () => { it('demotes score twice when both bitmap and severe multikey thresholds are met', () => { // Single-field bitmap with 50% coverage + 25× multikey expansion diff --git a/src/documentdb/queryInsights/ExplainPlanAnalyzer.ts b/src/documentdb/queryInsights/ExplainPlanAnalyzer.ts index d07015188..d3e85ca17 100644 --- a/src/documentdb/queryInsights/ExplainPlanAnalyzer.ts +++ b/src/documentdb/queryInsights/ExplainPlanAnalyzer.ts @@ -770,6 +770,16 @@ export class ExplainPlanAnalyzer { } } + // Traverse shard branches (sharded clusters) + if (plan.shards && Array.isArray(plan.shards)) { + for (const shard of plan.shards) { + const found = this.findStageInPlan(shard as Document, stageName); + if (found) { + return found; + } + } + } + return undefined; } diff --git a/src/documentdb/queryInsights/StagePropertyExtractor.ts b/src/documentdb/queryInsights/StagePropertyExtractor.ts index d77509a57..dbb53eb31 100644 --- a/src/documentdb/queryInsights/StagePropertyExtractor.ts +++ b/src/documentdb/queryInsights/StagePropertyExtractor.ts @@ -58,6 +58,7 @@ export class StagePropertyExtractor { /** * Collects all stages from the queryPlanner winning plan tree into a flat array. + * Traverses single inputStage, multiple inputStages, and shard branches (sharded clusters). */ private static collectPlannerStages(stage: Document, accumulator: Document[]): void { if (!stage || !stage.stage) { @@ -74,6 +75,11 @@ export class StagePropertyExtractor { this.collectPlannerStages(child, accumulator); }); } + if (stage.shards && Array.isArray(stage.shards)) { + for (const shard of stage.shards) { + this.collectPlannerStages(shard as Document, accumulator); + } + } } /** From 26de0e4e891b4a8a2158cf6c5dfdd47683ba3a97 Mon Sep 17 00:00:00 2001 From: venti <1308199824@qq.com> Date: Tue, 2 Jun 2026 12:32:42 +0800 Subject: [PATCH 2/2] fix: address Copilot review feedback for shard traversal in query-insights --- ...Analyzer.addIndexStrategyAdvisories.test.ts | 7 ++++++- .../queryInsights/ExplainPlanAnalyzer.ts | 9 ++++++--- .../queryInsights/StagePropertyExtractor.ts | 18 ++++++++++++------ 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/documentdb/queryInsights/ExplainPlanAnalyzer.addIndexStrategyAdvisories.test.ts b/src/documentdb/queryInsights/ExplainPlanAnalyzer.addIndexStrategyAdvisories.test.ts index 2f37c115a..2c36b0ab3 100644 --- a/src/documentdb/queryInsights/ExplainPlanAnalyzer.addIndexStrategyAdvisories.test.ts +++ b/src/documentdb/queryInsights/ExplainPlanAnalyzer.addIndexStrategyAdvisories.test.ts @@ -349,7 +349,12 @@ describe('ExplainPlanAnalyzer.addIndexStrategyAdvisories', () => { }); it('detects IXSCAN in planner shards even when exec stats use flat structure', () => { - // Edge case: planner is sharded, execution stats may not be + // Verifies that bitmap index detection works when the queryPlanner + // includes shard info (SHARD_MERGE > SINGLE_SHARD > IXSCAN) but + // executionStats use a simplified flat structure (FETCH > IXSCAN) + // without shard wrappers. This is important because production + // sharded clusters may return plan trees in either format, and the + // bitmap detector must be able to traverse both shapes correctly. const analysis = makeAnalysis(); const explainResult: Document = { queryPlanner: { diff --git a/src/documentdb/queryInsights/ExplainPlanAnalyzer.ts b/src/documentdb/queryInsights/ExplainPlanAnalyzer.ts index d3e85ca17..411454eb6 100644 --- a/src/documentdb/queryInsights/ExplainPlanAnalyzer.ts +++ b/src/documentdb/queryInsights/ExplainPlanAnalyzer.ts @@ -771,9 +771,12 @@ export class ExplainPlanAnalyzer { } // Traverse shard branches (sharded clusters) - if (plan.shards && Array.isArray(plan.shards)) { - for (const shard of plan.shards) { - const found = this.findStageInPlan(shard as Document, stageName); + if (plan.shards) { + const shardEntries = Array.isArray(plan.shards) ? plan.shards : Object.values(plan.shards); + for (const shardEntry of shardEntries) { + const shard = shardEntry as Document; + const planRoot = shard.winningPlan || shard.executionStages || shard.inputStage || shard; + const found = this.findStageInPlan(planRoot, stageName); if (found) { return found; } diff --git a/src/documentdb/queryInsights/StagePropertyExtractor.ts b/src/documentdb/queryInsights/StagePropertyExtractor.ts index dbb53eb31..86bdf3721 100644 --- a/src/documentdb/queryInsights/StagePropertyExtractor.ts +++ b/src/documentdb/queryInsights/StagePropertyExtractor.ts @@ -75,9 +75,12 @@ export class StagePropertyExtractor { this.collectPlannerStages(child, accumulator); }); } - if (stage.shards && Array.isArray(stage.shards)) { - for (const shard of stage.shards) { - this.collectPlannerStages(shard as Document, accumulator); + if (stage.shards) { + const shardEntries = Array.isArray(stage.shards) ? stage.shards : Object.values(stage.shards); + for (const shardEntry of shardEntries) { + const shard = shardEntry as Document; + const planRoot = shard.winningPlan || shard; + this.collectPlannerStages(planRoot, accumulator); } } } @@ -111,9 +114,12 @@ export class StagePropertyExtractor { }); } if (stage.shards) { - (stage.shards as Document[]).forEach((shard: Document) => { - this.traverseStages(shard, accumulator); - }); + const shardEntries = Array.isArray(stage.shards) ? stage.shards : Object.values(stage.shards); + for (const shardEntry of shardEntries) { + const shard = shardEntry as Document; + const planRoot = shard.executionStages || shard.inputStage || shard; + this.traverseStages(planRoot, accumulator); + } } }