-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
DocValueFormat implementation for date range fields #47472
DocValueFormat implementation for date range fields #47472
Conversation
Pinging @elastic/es-analytics-geo (:Analytics/Aggregations) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a comment about the doc tests but otherwise the changes LGTM once CI is happy :)
@@ -136,35 +137,35 @@ calculated over the ranges of all matching documents. | |||
"november_data" : { | |||
"buckets" : [ | |||
{ | |||
"key" : 1572220800000, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the doc tests are failing because date_histo will show both the key and key_as_string. E.g.
{
"key_as_string": "2015-01-01",
"key": 1420070400000,
"doc_count": 3
},
range fields didn't have doc_values before 7.4.1. They were added in elastic#47472.
range fields didn't have doc_values before 7.4.1. They were added in #47472.
Resolves #47323
This PR adds support for resolving
DocValueFormat
for date ranges. Other range types are not supported, which is consistent with how ranges work elsewhere in the system. Prior to this change, attempting to use a format string in theDateHistogram
over a range field generated an exception.There's a little bit of risk with this change, since
docValueFormat
gets used in a lot of places (see #47469 ).