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

Remove legacy settings for restricting date rounding #8598

Closed
rjernst opened this issue Nov 21, 2014 · 0 comments · Fixed by #8889
Closed

Remove legacy settings for restricting date rounding #8598

rjernst opened this issue Nov 21, 2014 · 0 comments · Fixed by #8889
Labels
:Core/Infra/Settings Settings infrastructure and APIs v2.0.0-beta1

Comments

@rjernst
Copy link
Member

rjernst commented Nov 21, 2014

The setting mapping.date.round_ceil affects how date ranges are parsed, but only on the upper end of the range, and when the end is inclusive. It causes a slightly different date parsing function to be used, which will first try to parse as a date, and then try as a timestamp if the year is very large. If we want to support this timestamp fallback, it should happen for all date parsing, regardless of which end of the range it came from, or whether the end is inclusive.

Also, the setting index.mapping.date.parse_upper_inclusive appears to be from before 1.0, and is no longer documented. This setting is currently parsed and the value used as the default for mapping.date.round_ceil. Both of these settings should be removed to ensure date ranges are parsed symmetrically.

@rjernst rjernst added the :Core/Infra/Settings Settings infrastructure and APIs label Nov 21, 2014
rjernst added a commit to rjernst/elasticsearch that referenced this issue Dec 15, 2014
The setting `mapping.date.round_ceil` (and the undocumented setting
`index.mapping.date.parse_upper_inclusive`) affect how date ranges using
`lte` are parsed.  In elastic#8556 the semantics of date rounding were
solidified, eliminating the need to have different parsing functions
whether the date is inclusive or exclusive.

This change removes these legacy settings and improves the tests
for the date math parser (now at 100% coverage!). It also removes the
unnecessary function `DateMathParser.parseTimeZone` for which
the existing `DateTimeZone.forID` handles all use cases.

Any user previously using these settings can refer to the changed
semantics and change their query accordingly. This is a breaking change
because even dates without datemath previously used the different
parsing functions depending on context.

closes elastic#8598
closes elastic#8889
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Settings Settings infrastructure and APIs v2.0.0-beta1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants