Skip to content
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

[Backport 2.x] Optimize global ordinal includes/excludes for prefix matching #15324

Merged
merged 1 commit into from
Aug 20, 2024

Conversation

opensearch-trigger-bot[bot]
Copy link
Contributor

Backport 13163ab from #14371.

* Optimize global ordinal includes/excludes for prefix matching

If an aggregration specifies includes or excludes based on a regular
expression, and the regular expression has a finite expansion followed
by .*, then we can optimize the global ordinal filter.

Specifically, in this case, we can expand the matching prefixes, then
include/exclude the range of global ordinals that start with each
prefix.

Signed-off-by: Michael Froh <froh@amazon.com>

* Add unit test

Signed-off-by: Michael Froh <froh@amazon.com>

* Add changelog entry

Signed-off-by: Michael Froh <froh@amazon.com>

* Improve test coverage

Updated the unit test to be functionally equivalent, but it covers
more of the regex logic.

Signed-off-by: Michael Froh <froh@amazon.com>

* Improve test coverage

Signed-off-by: Michael Froh <froh@amazon.com>

* Fix bug in exclude-only case with no doc values in segment

Signed-off-by: Michael Froh <froh@amazon.com>

* Address comments from @mch2

Signed-off-by: Michael Froh <froh@amazon.com>

---------

Signed-off-by: Michael Froh <froh@amazon.com>
(cherry picked from commit 13163ab)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Copy link
Contributor

✅ Gradle check result for a99e985: SUCCESS

Copy link

codecov bot commented Aug 20, 2024

Codecov Report

Attention: Patch coverage is 77.88462% with 23 lines in your changes missing coverage. Please review.

Project coverage is 71.61%. Comparing base (0b0f1e1) to head (a99e985).
Report is 1 commits behind head on 2.x.

Files Patch % Lines
...arch/aggregations/bucket/terms/IncludeExclude.java 77.88% 8 Missing and 15 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                2.x   #15324      +/-   ##
============================================
+ Coverage     71.59%   71.61%   +0.02%     
- Complexity    62916    62969      +53     
============================================
  Files          5157     5157              
  Lines        295230   295334     +104     
  Branches      42971    43000      +29     
============================================
+ Hits         211368   211507     +139     
+ Misses        66150    66066      -84     
- Partials      17712    17761      +49     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mch2 mch2 merged commit e2bb0ab into 2.x Aug 20, 2024
58 checks passed
@mch2 mch2 deleted the backport/backport-14371-to-2.x branch August 20, 2024 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant