Skip to content

feat(query-insights): demote score for wasteful single-field bitmap indexes#623

Merged
tnaum-ms merged 12 commits into
nextfrom
dev/tnaum/bitmap-index-demotions
May 8, 2026
Merged

feat(query-insights): demote score for wasteful single-field bitmap indexes#623
tnaum-ms merged 12 commits into
nextfrom
dev/tnaum/bitmap-index-demotions

Conversation

@tnaum-ms
Copy link
Copy Markdown
Collaborator

@tnaum-ms tnaum-ms commented May 4, 2026

Summary

Adds a score demotion rule to the Query Insights static analysis for single-field bitmap indexes that return a large fraction of the collection.

Previously, a query using a bitmap index with high selectivity (≥20% of documents returned) could still receive an Excellent performance rating because the index was technically efficient — docsExamined ≈ nReturned. This was misleading: bitmap indexes on low-cardinality fields carry ongoing write and storage overhead that outweighs the marginal read benefit when the index returns many documents.

What changed

  • Demotion rule: When isBitmap: true, the index is single-field, and selectivity ≥ 20%, the bitmap badge type is set to negative and the performance score is demoted one level (e.g. ExcellentGood).
  • Compound indexes are excluded: The penalty only applies to single-field indexes. A compound index that includes a bitmap field on a low-cardinality key still provides meaningful selectivity via its other keys.
  • Spec updated: query-insights-static-analysis.md documents the demotion table and design decision.
  • Tests: 4 new test cases added to ExplainPlanAnalyzer.addIndexStrategyAdvisories.test.ts; all 10 bitmap tests pass.

Non-DocumentDB safety

isBitmap is a DocumentDB-specific explain plan field. Standard MongoDB explain plans never include it, so this rule is never triggered for non-DocumentDB clusters.

…dexes

When a bitmap index is single-field AND returns >= 20% of the collection,
the badge becomes negative and the performance score is demoted one level.
Compound indexes are excluded (bitmap prefix + selective second key is valid).

This ensures queries like find({isFamilyFriendly: true}) on a 55/45 boolean
field get 'Good' instead of 'Excellent' — signaling the index is wasteful.
@tnaum-ms tnaum-ms requested a review from a team as a code owner May 4, 2026 14:35
Copilot AI review requested due to automatic review settings May 4, 2026 14:35
@tnaum-ms tnaum-ms added this to the 0.8.0 milestone May 4, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new Query Insights static-analysis rule to penalize wasteful single-field bitmap indexes that return a large fraction of a collection, improving the accuracy of performance ratings for DocumentDB-specific explain plans.

Changes:

  • Demotes performance score by one level and marks the bitmap_index badge as negative when a bitmap index is single-field and coverage is ≥ 20%.
  • Updates the Query Insights static analysis spec to document the demotion behavior and rationale (compound indexes excluded).
  • Adds new Jest test cases covering the demotion and non-demotion scenarios.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
src/documentdb/queryInsights/query-insights-static-analysis.md Documents the bitmap demotion rule and updates the post-score adjustment list.
src/documentdb/queryInsights/ExplainPlanAnalyzer.ts Implements single-field bitmap detection via indexUsage[].scanKeys and applies badge/score demotion.
src/documentdb/queryInsights/ExplainPlanAnalyzer.addIndexStrategyAdvisories.test.ts Adds/extends bitmap advisory tests to validate demotion behavior and compound-index exclusion.

Comment thread src/documentdb/queryInsights/ExplainPlanAnalyzer.ts Outdated
Comment thread src/documentdb/queryInsights/ExplainPlanAnalyzer.ts Outdated
Comment thread src/documentdb/queryInsights/ExplainPlanAnalyzer.ts
Comment thread src/documentdb/queryInsights/query-insights-static-analysis.md Outdated
tnaum-ms added 4 commits May 4, 2026 14:57
Adds Math.min(..., 1) to the bitmap demotion coverage calculation,
matching the earlier coverage-badge logic. Prevents displaying >100%
when totalCollectionDocs is an underestimate.
Updates the static-analysis spec and the JSDoc on
addIndexStrategyAdvisories() to reflect that bitmap index
and severe multikey expansion advisories can demote scores.
Changes isSingleField from .some() to a strict check requiring exactly
one indexUsage entry with exactly one scanKey. Mixed entries are
conservatively treated as compound to avoid false demotions.

Adds a test for the mixed-entry edge case.
@tnaum-ms
Copy link
Copy Markdown
Collaborator Author

tnaum-ms commented May 8, 2026

Review fix #3: isSingleField can misclassify mixed indexUsage entries

Fixed in 12127d3 — changed isSingleField from .some() to a strict check requiring exactly one indexUsage entry with exactly one scanKey. If DocumentDB returns multiple indexUsage entries with mixed key counts, the index is conservatively treated as compound to avoid false demotions.

Added a test (does not demote score when indexUsage has mixed single/multi-key entries) to lock down this edge case.

On plans with multiple IXSCAN stages (e.g., OR, index intersection),
findStageInPlan could return different index scans for the planner
and execution trees. Now correlates by indexName so the bitmap flag
and scanKeys are read from the same index.

Adds a test for the mismatched-indexName edge case.
@tnaum-ms
Copy link
Copy Markdown
Collaborator Author

tnaum-ms commented May 8, 2026

Review fix #4: Planner IXSCAN and execution-stats IXSCAN are not correlated

Fixed in bb5a427 — the bitmap check now correlates the planner IXSCAN and the execution-stats IXSCAN by indexName. If the first IXSCAN found in each tree refers to a different index (e.g., OR plans, index intersection), the single-field detection falls back to neutral instead of potentially inspecting a different index's scanKeys.

Added a test (does not demote when planner and exec IXSCAN have different index names) to cover this edge case.

tnaum-ms added 3 commits May 8, 2026 16:10
Moves the duplicated scoreOrder/demotion pattern into a shared
private static method and a module-level SCORE_ORDER constant.
Both bitmap and multikey demotion blocks now use the helper.
makeExplainResult now accepts an nReturned option so the raw
executionStats.nReturned stays in sync with the analysis object.
All tests that override analysis.nReturned now pass the same
value to the fixture.
Adds a test confirming that bitmap and severe multikey demotions
stack independently (e.g., excellent → fair when both fire).
Documents this in the static-analysis spec.
@tnaum-ms
Copy link
Copy Markdown
Collaborator Author

tnaum-ms commented May 8, 2026

Review fix #7: Double-demotion behavior is not specified

Fixed in a229c1e — added a test (demotes score twice when both bitmap and severe multikey thresholds are met) confirming that advisory demotions are cumulative: if both bitmap_index and severe_multikey_expansion fire, the score is reduced by two levels (e.g., Excellent → Fair).

Also documented this in the static-analysis spec under a new "Cumulative demotions" note.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 8, 2026

✅ Code Quality Checks

Check Status How to fix
Localization (l10n) ✅ Passed
ESLint ✅ Passed
Prettier formatting ✅ Passed

This comment is updated automatically on each push.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 8, 2026

📦 Build Size Report

Metric Base (next) PR Delta
VSIX (vscode-documentdb-0.8.0-beta.vsix) 6.79 MB 6.79 MB ⬆️ +0 KB (+0.0%)
Webview bundle (views.js) 5.79 MB 5.79 MB ✅ 0 KB (0.0%)

Download artifact · updated automatically on each push.

@tnaum-ms tnaum-ms merged commit f836f00 into next May 8, 2026
8 checks passed
@tnaum-ms tnaum-ms deleted the dev/tnaum/bitmap-index-demotions branch May 8, 2026 18:19
@tnaum-ms tnaum-ms mentioned this pull request May 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

3 participants