add TextMatchFilterOptimizer to maximally push down text_match filters to Lucene
#12339
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #12339 +/- ##
============================================
- Coverage 61.66% 61.62% -0.04%
Complexity 207 207
============================================
Files 2421 2422 +1
Lines 131852 131974 +122
Branches 20345 20372 +27
============================================
+ Hits 81303 81331 +28
- Misses 44582 44666 +84
- Partials 5967 5977 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
@siddharthteotia I'd appreciate your thoughts/review |
chenboat
reviewed
Jan 30, 2024
pinot-core/src/test/java/org/apache/pinot/core/query/optimizer/QueryOptimizerTest.java
Show resolved
Hide resolved
chenboat
reviewed
Jan 30, 2024
| "SELECT * FROM testTable WHERE TEXT_MATCH(string1, 'foo1 AND bar1') AND TEXT_MATCH(string2, 'foo2 AND bar2')"); | ||
| testCannotOptimizeQuery("SELECT * FROM testTable WHERE TEXT_MATCH(string1, 'foo') OR TEXT_MATCH(string2, 'bar')"); | ||
| testCannotOptimizeQuery( | ||
| "SELECT * FROM testTable WHERE int = 1 AND TEXT_MATCH(string, 'foo') OR TEXT_MATCH(string, 'bar')"); |
There was a problem hiding this comment.
why this one can not be optimized? the columns are the same "string"?
There was a problem hiding this comment.
int = 1 AND text_match(x) OR text_match(y) wouldn't be equivalent to int = 1 AND text_match(x OR y)
chenboat
approved these changes
Jan 30, 2024
suyashpatel98
pushed a commit
to suyashpatel98/pinot
that referenced
this pull request
Feb 28, 2024
…ers to Lucene (apache#12339) * add TextMatchFilterOptimizer * fix equivalence for all not
|
this looks awesome. is it possible to optimize regexp_like(x) OR regexp_like(y) ? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation:
Query performance against the Lucene index suffers when chaining multiple
text_matchpredicates together. Our users often programmatically generate their queries, which exacerbates the issue as 10s/100s oftext_matchpredicates can be included in a single query.Because of this, users are required to understand Pinot's Lucene implementation details for them to compose an efficient query. To remove this requirement, this PR adds a
TextMatchFilterOptmizerthat performs the optimization automatically.Summary:
This functionality is best understood through the unit testcases. In short:
text_matchoperands when possible, without affecting query accuracytext_matchfilters are inversed, then the NOT expression remains in PinotOpen question:
There is one edge case (that I can think of) where this optimization can hurt performance: if there are a number of
text_match OR text_match OR text_matchetc, early termination whenlimitis reached might take longer since the entire mergedtext_matchquery must now complete. For this reason, it might be prudent to put this behind a query option (or rather a query option to disable it, since I believe it makes more sense to enable by default).Ideally, the
LuceneDocIdCollectorcould early terminate (but doesn't currently have the required context).Testing: unit tests (query performance separately verified via running the optimized vs unoptimized queries). Sample optimization improvements:
tags:
feature,performance(?)