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

Fix round up of date range without rounding #43303

Merged
merged 3 commits into from
Jun 20, 2019

Conversation

jimczi
Copy link
Contributor

@jimczi jimczi commented Jun 17, 2019

Today when searching for an exclusive range the java date math parser rounds up the value
with the granularity of the operation. So when searching for values that are greater than
now-2M the parser rounds up the operation to now-1M.

This behavior was introduced when we migrated to java date but it looks like a bug since the joda math parser rounds up values but only when a rounding is used. So now/M is rounded to now-1ms (minus 1ms to get the largest inclusive value) in the joda parser if the result should be exclusive but no rounding is applied if the input is a simple operation like now-1M.

This change restores the joda behavior in order to have a consistent parsing in all versions.

Closes #43277

Today when searching for an exclusive range the java date math parser rounds up the value
with the granularity of the operation. So when searching for values that are greater than
"now-2M" the parser rounds up the operation to "now-1M". This behavior was introduced when
we migrated to java date but it looks like a bug since the joda math parser rounds up values
but only when a rounding is used. So "now/M" is rounded to "now-1ms" (minus 1ms to get the largest inclusive value)
in the joda parser if the result should be exclusive but no rounding is applied if the input
is a simple operation like "now-1M". This change restores the joda behavior in order to have
a consistent parsing in all versions.

Closes elastic#43277
@jimczi jimczi added >bug :Search/Search Search-related issues that do not fall into other categories v8.0.0 v7.3.0 v7.2.1 labels Jun 17, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

Copy link
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, good catch!

@jimczi jimczi merged commit 215fc4c into elastic:master Jun 20, 2019
@jimczi jimczi deleted the bug/date_math_rounding branch June 20, 2019 20:01
jimczi added a commit that referenced this pull request Jun 20, 2019
Today when searching for an exclusive range the java date math parser rounds up the value
with the granularity of the operation. So when searching for values that are greater than
"now-2M" the parser rounds up the operation to "now-1M". This behavior was introduced when
we migrated to java date but it looks like a bug since the joda math parser rounds up values
but only when a rounding is used. So "now/M" is rounded to "now-1ms" (minus 1ms to get the largest inclusive value)
in the joda parser if the result should be exclusive but no rounding is applied if the input
is a simple operation like "now-1M". This change restores the joda behavior in order to have
a consistent parsing in all versions.

Closes #43277
jimczi added a commit that referenced this pull request Jun 20, 2019
Today when searching for an exclusive range the java date math parser rounds up the value
with the granularity of the operation. So when searching for values that are greater than
"now-2M" the parser rounds up the operation to "now-1M". This behavior was introduced when
we migrated to java date but it looks like a bug since the joda math parser rounds up values
but only when a rounding is used. So "now/M" is rounded to "now-1ms" (minus 1ms to get the largest inclusive value)
in the joda parser if the result should be exclusive but no rounding is applied if the input
is a simple operation like "now-1M". This change restores the joda behavior in order to have
a consistent parsing in all versions.

Closes #43277
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Search/Search Search-related issues that do not fall into other categories v7.2.1 v7.3.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'gt' gives unexpected results.
4 participants