-
Notifications
You must be signed in to change notification settings - Fork 175
Provide LMDB index recommendations in explain plans #5550
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Provide LMDB index recommendations in explain plans #5550
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements index recommendation functionality for LMDB database queries. The feature analyzes query patterns and suggests optimal indexes when the current index requires a full scan, helping users identify performance optimization opportunities.
Key Changes
- Added index name tracking through the
IndexReportingIteratorinterface implementation - Implemented recommendation logic that evaluates candidate indexes based on usage count, pattern score, and order deviation
- Created comprehensive test coverage for all index permutations and recommendation scenarios
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
LmdbExplainIndexRecommendationTest.java |
New test file validating index recommendation logic across all permutations and preference scenarios |
RecordIterator.java |
Added getIndexName() default method to support index name retrieval |
LmdbStatementIterator.java |
Implemented IndexReportingIterator interface to propagate index names |
LmdbRecordIterator.java |
Core implementation of index recommendation algorithm with candidate selection and tracking |
TripleSourceIterationWrapper.java |
Delegated getIndexName() to wrapped iterator when available |
StatementPatternQueryEvaluationStep.java |
Added explicit null handling when setting index names on statement patterns |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (contextBound) { | ||
| order.add('c'); | ||
| } |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition on line 307 checks contextBound again, but this code is inside the else block where contextBound could be true (lines 300-320). This creates a logic issue where the order of 'c' could be duplicated. The condition should only add 'c' if it hasn't been added already, or this appears to be redundant since line 317 already checks and adds 'c' if contextBound is true.
| if (contextBound) { | |
| order.add('c'); | |
| } | |
| // Removed redundant addition of 'c' to avoid duplication. |
| if (contextBound) { | ||
| order.add('c'); | ||
| } |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition if (contextBound) on line 307 is redundant because it's already inside a block where contextBound is guaranteed to be true (line 300). The check is unnecessary and reduces code clarity.
| if (contextBound) { | |
| order.add('c'); | |
| } | |
| order.add('c'); |
GitHub issue resolved: #5553
Briefly describe the changes proposed in this PR:
PR Author Checklist (see the contributor guidelines for more details):
mvn process-resourcesto format from the command line)