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] Fix auto date histogram rounding assertion bug #17168

Closed
wants to merge 1 commit into from

Conversation

opensearch-trigger-bot[bot]
Copy link
Contributor

Backport de59264 from #17023.

* Add comments explanations for auto date histo increaseRoundingIfNeeded.

Signed-off-by: Finn Carroll <[email protected]>

* Add testFilterRewriteWithTZRoundingRangeAssert() to reproduce auto date histo assertion bug per #16932

Signed-off-by: Finn Carroll <[email protected]>

* Fix #16932. Ensure optimized path can only increase preparedRounding of agg.

Signed-off-by: Finn Carroll <[email protected]>

* Spotless apply

Signed-off-by: Finn Carroll <[email protected]>

* Fast fail filter rewrite opt in data histo aggs for non UTC timezones

Signed-off-by: Finn Carroll <[email protected]>

* Remove redundant UTC check from getInterval().

Signed-off-by: Finn Carroll <[email protected]>

* Save a call to prepareRounding if roundingIdx is unchanged.

Signed-off-by: Finn Carroll <[email protected]>

* Spotless apply

Signed-off-by: Finn Carroll <[email protected]>

* Changelog

Signed-off-by: Finn Carroll <[email protected]>

* Add ZoneId getter for date histo filter rewrite canOptimize check.

Signed-off-by: Finn Carroll <[email protected]>

* Spotless apply

Signed-off-by: Finn Carroll <[email protected]>

* Disable ff optimzation for composite agg in canOptimize.

Signed-off-by: Finn Carroll <[email protected]>

* Spotless apply

Signed-off-by: Finn Carroll <[email protected]>

* Handle utc timezone check

Signed-off-by: bowenlan-amzn <[email protected]>

* Remove redundant timeZone getter.

Signed-off-by: Finn Carroll <[email protected]>

* Simplify ff prepared rounding check.

Signed-off-by: Finn Carroll <[email protected]>

---------

Signed-off-by: Finn Carroll <[email protected]>
Signed-off-by: bowenlan-amzn <[email protected]>
Co-authored-by: bowenlan-amzn <[email protected]>
(cherry picked from commit de59264)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@jainankitk
Copy link
Collaborator

As suspected, the backport is failing detect breaking changes due to the abstract method addition. @finnegancarroll - Can we make the change in 2.x for backward compatibility?

Copy link
Contributor

❕ Gradle check result for edd743d: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

Copy link

codecov bot commented Jan 28, 2025

Codecov Report

Attention: Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.

Project coverage is 71.93%. Comparing base (1c7f719) to head (edd743d).
Report is 5 commits behind head on 2.x.

Files with missing lines Patch % Lines
...ain/java/org/opensearch/search/DocValueFormat.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                2.x   #17168      +/-   ##
============================================
+ Coverage     71.89%   71.93%   +0.03%     
- Complexity    65852    65859       +7     
============================================
  Files          5332     5332              
  Lines        306587   306592       +5     
  Branches      44720    44721       +1     
============================================
+ Hits         220429   220554     +125     
+ Misses        67754    67662      -92     
+ Partials      18404    18376      -28     

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

@finnegancarroll
Copy link
Contributor

Manual backport here: #17175
This duplicate can be closed.

@jainankitk jainankitk closed this Jan 29, 2025
@jainankitk jainankitk deleted the backport/backport-17023-to-2.x branch January 29, 2025 17:29
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.

2 participants