-
Notifications
You must be signed in to change notification settings - Fork 321
Add further filtering of forced indexes on CRDB to discount other shapes #2437
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2437 +/- ##
==========================================
- Coverage 77.17% 77.15% -0.01%
==========================================
Files 422 422
Lines 51219 51228 +9
==========================================
Hits 39521 39521
- Misses 9193 9199 +6
- Partials 2505 2508 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
LGTM, appreciate the test coverage
subjectFieldDepth := 0 | ||
subjectFieldDepths := mapz.NewSet[int]() |
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.
I'm not entirely clear on the intent of using a map here - I can see that the semantic is that there should only be a single field depth, but I'm not clear on when there could be multiple, or why that would indicate that an index shouldn't be used.
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.
Its a set: we need to make sure all filters have the same depth
No description provided.