Fix date rounding for date math parsing by pgomulka · Pull Request #90458 · elastic/elasticsearch

pgomulka · 2022-09-28T13:27:30Z

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

elasticsearchmachine · 2022-09-29T12:35:54Z

Hi @pgomulka, I've created a changelog YAML for you.

elasticsearchmachine · 2022-09-29T12:36:29Z

Pinging @elastic/es-core-infra (Team:Core/Infra)

…o date_range_defaulting_fix

romseygeek · 2022-09-29T13:30:43Z

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search/500_date_range.yml

+            range:
+              date:
+                gte: 1500 #1500-01-01T00:00:00
+                lte: 1500 #1500-01-01T23:59:59


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

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

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?

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

#90605

pgomulka · 2022-09-29T18:27:42Z

@elasticmachine update branch

grcevski

LGTM! Thanks!

grcevski · 2022-09-29T19:51:08Z

I wonder if we should try to make this fix in the 8.4.3 release too? Is it too late?

pgomulka · 2022-10-03T07:13:16Z

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.

romseygeek

LGTM

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)

pgomulka · 2022-10-04T09:06:46Z

💚 All backports created successfully

Status	Branch	Result
✅	8.5
✅	7.17

Questions ?

Please refer to the Backport tool documentation

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

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

* 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)

the test should be enabled as the fix is applied in 7.x branch relates elastic#90458

the elastic#90458 has been backported to all branches so the bwc testing can be enable for this

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

the #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 #90668

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

…90681) (#90690) the #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 #90668

… (#90689) the #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 #90668

…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

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

…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

… (#91201) the #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 close #90783

not spanning on full date

b5444c5

elasticsearchmachine added the v8.6.0 label Sep 28, 2022

comment

f5c0fe9

eirikboy mentioned this pull request Sep 29, 2022

Match query on date property with format 'uuuu' (year) acts as a range query for years below 1971 #90187

Closed

fix test

9eff947

pgomulka added v7.17.7 v8.5.1 >bug :Core/Infra/Core Core issues without another label labels Sep 29, 2022

Update docs/changelog/90458.yaml

a47ab03

pgomulka marked this pull request as ready for review September 29, 2022 12:36

pgomulka requested review from grcevski and romseygeek September 29, 2022 12:36

elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Sep 29, 2022

pgomulka added 2 commits September 29, 2022 14:55

precommit

5b5bd1c

Merge remote-tracking branch 'pgomulka/date_range_defaulting_fix' int…

3f9e7d9

…o date_range_defaulting_fix

romseygeek reviewed Sep 29, 2022

View reviewed changes

fix tests

103a00d

Merge branch 'main' into date_range_defaulting_fix

c8be4fb

grcevski approved these changes Sep 29, 2022

View reviewed changes

romseygeek approved these changes Oct 4, 2022

View reviewed changes

pgomulka merged commit 3f3a95e into elastic:main Oct 4, 2022

pgomulka mentioned this pull request Oct 4, 2022

[8.5] Fix date rounding for date math parsing (#90458) #90633

Merged

pgomulka mentioned this pull request Oct 4, 2022

[7.17] Fix date rounding for date math parsing (#90458) #90635

Merged

pgomulka added a commit to pgomulka/elasticsearch that referenced this pull request Oct 4, 2022

Enable 500_date_range test

50e8c8b

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

Enable search/500_date_range test for bwc testing

6e969af

the elastic#90458 has been backported to all branches so the bwc testing can be enable for this

pgomulka mentioned this pull request Oct 4, 2022

Enable search/500_date_range test for bwc testing #90644

Closed

cbuescher mentioned this pull request Oct 5, 2022

[CI] SmokeTestMultiNodeClientYamlTestSuiteIT test {yaml=search/500_date_range/test match query gte and lte with year uuuu} failing #90668

Closed

pgomulka mentioned this pull request Oct 5, 2022

[Testing] Enable bwc and fix sorting for 500_date_range #90681

Merged

pgomulka mentioned this pull request Oct 13, 2022

[8.4] Fix date rounding for date math parsing (#90458) #90633 #90882

Merged

pgomulka mentioned this pull request Oct 31, 2022

[8.4][Testing] Fix sorting for 500_date_range backport(#90681) (#90690) #91201

Merged

csoulios added v8.5.0 and removed v8.5.1 labels Nov 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix date rounding for date math parsing#90458

Fix date rounding for date math parsing#90458
pgomulka merged 8 commits intoelastic:mainfrom
pgomulka:date_range_defaulting_fix

pgomulka commented Sep 28, 2022 •

edited

Loading

Uh oh!

elasticsearchmachine commented Sep 29, 2022

Uh oh!

elasticsearchmachine commented Sep 29, 2022

Uh oh!

romseygeek Sep 29, 2022

Uh oh!

pgomulka Sep 29, 2022

Uh oh!

romseygeek Oct 3, 2022

Uh oh!

pgomulka Oct 3, 2022 •

edited

Loading

Uh oh!

pgomulka commented Sep 29, 2022

Uh oh!

grcevski left a comment

Uh oh!

grcevski commented Sep 29, 2022

Uh oh!

pgomulka commented Oct 3, 2022

Uh oh!

romseygeek left a comment

Uh oh!

pgomulka commented Oct 4, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

pgomulka commented Sep 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented Sep 29, 2022

Uh oh!

elasticsearchmachine commented Sep 29, 2022

Uh oh!

romseygeek Sep 29, 2022

Choose a reason for hiding this comment

Uh oh!

pgomulka Sep 29, 2022

Choose a reason for hiding this comment

Uh oh!

romseygeek Oct 3, 2022

Choose a reason for hiding this comment

Uh oh!

pgomulka Oct 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pgomulka commented Sep 29, 2022

Uh oh!

grcevski left a comment

Choose a reason for hiding this comment

Uh oh!

grcevski commented Sep 29, 2022

Uh oh!

pgomulka commented Oct 3, 2022

Uh oh!

romseygeek left a comment

Choose a reason for hiding this comment

Uh oh!

pgomulka commented Oct 4, 2022

💚 All backports created successfully

Questions ?

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

pgomulka commented Sep 28, 2022 •

edited

Loading

pgomulka Oct 3, 2022 •

edited

Loading