Fix date rounding for date math parsing #90458
Merged
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
pgomulka
added
v7.17.7
v8.5.1
>bug
:Core/Infra/Core
|
Hi @pgomulka, I've created a changelog YAML for you. |
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
…o date_range_defaulting_fix
romseygeek
reviewed
Sep 29, 2022
| range: | ||
| date: | ||
| gte: 1500 #1500-01-01T00:00:00 | ||
| lte: 1500 #1500-01-01T23:59:59 |
There was a problem hiding this comment.
I don't think this is right? If you're asking for things in the range 1500 to 1500, you expect to find stuff on the 31st December 1500, but this is rounding to midnight on the 1st January
There was a problem hiding this comment.
but did we had that behaviour in the past? It might be a bug on its own, but I thought that this is how it always worked
There was a problem hiding this comment.
Testing on a 7x install it seems you're right, this has always been broken. But it's still a bug. Is it simple to fix here or should we merge this PR as-is and work on that separately?
There was a problem hiding this comment.
agree this is a bug (or a missing feature at least). I think this would require some more work. It would make 2 cases for rounding (up and down) I am happy to raise an issue where we could gather all the use cases
|
@elasticmachine update branch |
|
I wonder if we should try to make this fix in the 8.4.3 release too? Is it too late? |
I think it is too late now. |
pgomulka
added a commit
to pgomulka/elasticsearch
that referenced
this pull request
Oct 4, 2022
in elastic#89693 the rounding logic was only applied when a field was present on a pattern. This is incorrect as for dates like "2020" we want to default to "2020-01-01T23:59:59.999..." when rounding is enabled. This commit always applies monthOfYear or dayofMonth defaulting (when rounding enabled) except when the dayOfYear is set closes elastic#90187 (cherry picked from commit 3f3a95e)
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
pgomulka
added a commit
to pgomulka/elasticsearch
that referenced
this pull request
Oct 4, 2022
in elastic#89693 the rounding logic was only applied when a field was present on a pattern. This is incorrect as for dates like "2020" we want to default to "2020-01-01T23:59:59.999..." when rounding is enabled. This commit always applies monthOfYear or dayofMonth defaulting (when rounding enabled) except when the dayOfYear is set closes elastic#90187 (cherry picked from commit 3f3a95e) # Conflicts: # server/src/test/java/org/elasticsearch/common/time/DateFormattersTests.java
pgomulka
added a commit
that referenced
this pull request
Oct 4, 2022
in #89693 the rounding logic was only applied when a field was present on a pattern. This is incorrect as for dates like "2020" we want to default to "2020-01-01T23:59:59.999..." when rounding is enabled. This commit always applies monthOfYear or dayofMonth defaulting (when rounding enabled) except when the dayOfYear is set closes #90187 backports #90458
pgomulka
added a commit
that referenced
this pull request
Oct 4, 2022
* Fix date rounding for date math parsing (#90458) in #89693 the rounding logic was only applied when a field was present on a pattern. This is incorrect as for dates like "2020" we want to default to "2020-01-01T23:59:59.999..." when rounding is enabled. This commit always applies monthOfYear or dayofMonth defaulting (when rounding enabled) except when the dayOfYear is set closes #90187 backport(#90458) (cherry picked from commit 3f3a95e)
pgomulka
added a commit
to pgomulka/elasticsearch
that referenced
this pull request
Oct 4, 2022
the test should be enabled as the fix is applied in 7.x branch relates elastic#90458
pgomulka
added a commit
to pgomulka/elasticsearch
that referenced
this pull request
Oct 4, 2022
the elastic#90458 has been backported to all branches so the bwc testing can be enable for this
pgomulka
added a commit
to pgomulka/elasticsearch
that referenced
this pull request
Oct 5, 2022
the elastic#90458 has been backported to all branches so the bwc testing can be enable for this tests were incorrectly relying on sort order. Added sort to make it deterministic closes elastic#90668
pgomulka
added a commit
that referenced
this pull request
Oct 5, 2022
pgomulka
added a commit
to pgomulka/elasticsearch
that referenced
this pull request
Oct 5, 2022
the elastic#90458 has been backported to all branches so the bwc testing can be enable for this tests were incorrectly relying on sort order. Added sort to make it deterministic closes elastic#90668
pgomulka
added a commit
to pgomulka/elasticsearch
that referenced
this pull request
Oct 5, 2022
the elastic#90458 has been backported to all branches so the bwc testing can be enable for this tests were incorrectly relying on sort order. Added sort to make it deterministic closes elastic#90668
pgomulka
added a commit
that referenced
this pull request
Oct 6, 2022
pgomulka
added a commit
that referenced
this pull request
Oct 6, 2022
pgomulka
added a commit
to pgomulka/elasticsearch
that referenced
this pull request
Oct 13, 2022
…tic#90633) in elastic#89693 the rounding logic was only applied when a field was present on a pattern. This is incorrect as for dates like "2020" we want to default to "2020-01-01T23:59:59.999..." when rounding is enabled. This commit always applies monthOfYear or dayofMonth defaulting (when rounding enabled) except when the dayOfYear is set closes elastic#90187 backports elastic#90458
pgomulka
added a commit
that referenced
this pull request
Oct 13, 2022
in #89693 the rounding logic was only applied when a field was present on a pattern. This is incorrect as for dates like "2020" we want to default to "2020-01-01T23:59:59.999..." when rounding is enabled. This commit always applies monthOfYear or dayofMonth defaulting (when rounding enabled) except when the dayOfYear is set closes #90187 backports #90458
pgomulka
added a commit
to pgomulka/elasticsearch
that referenced
this pull request
Oct 31, 2022
…elastic#90690) the elastic#90458 has been backported to all branches so the bwc testing can be enable for this tests were incorrectly relying on sort order. Added sort to make it deterministic closes elastic#90668
pgomulka
added a commit
that referenced
this pull request
Oct 31, 2022
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.
in #89693 the rounding logic was only applied when a field was present on a pattern. This is incorrect as for dates like "2020" we want to default to "2020-01-01T23:59:59.999..." when rounding is enabled.
This commit always applies
monthOfYearordayofMonthdefaulting (when rounding enabled) except when thedayOfYearis setcloses #90187