fixes error with custom interval in datetime aggregation

[ppisljar]

fixes error with custom interval in datetime aggregation with values like 2w

closes #9184

[thomasneirynck]

jenkins, test this

[thomasneirynck]

LGTM:

Likely due to elastic/elasticsearch#19102 in the first place.

[thomasneirynck]

add some explanation here, since this is somewhat of a magic number.

[thomasneirynck]

this probably needs rebasing as well.

[ppisljar]

jenkins, test this

[tbragin]

Functional LGTM

[kobelb]

The docs state, that the Date Histogram aggregation uses the time-units for the interval in addition to: year, quarter, month, week, day, hour, minute, second

However, it appears that the following undocumented values are also valid for the interval: 1y, 1q, 1M, 1w, 1d, 1h, 1m, 1s per https://github.com/elastic/elasticsearch/blob/master/core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregationBuilder.java#L64. So, that's why 1w works, and 2w doesn't when querying Elasticsearch. 1w is a hard-coded and undocumented value that takes effect before falling back to the 'time-units'.

[kobelb]

It might be helpful to document why we're limiting the "large units" from being anything but 1, as it's using undocumented values that on first glance appear to be invalid.

Or, it might be prudent to not rely on the undocumented values.

[ppisljar]

@epixa @spalger @Bargs what are your opinions, what direction should we take this ?

• go further with this PR, update documentation
• dont go further with this PR, change behaviour to not accept undocumented time arguments, handle error correctly
• dont go further with this PR, update documentation to describe all working time arguments, handle error correctly ?
• something else ?

[Bargs]

Supporting those undocumented values seems like it could only lead to confusion for the user, particularly because they look so similar to the "time-units" Brandon mentioned. I wouldn't rely on them personally.

However I'd also check with the ES team to see what's up with these mystery values. Perhaps they could add week support to time-units and start throwing errors for these undocumented values.

Alternatively, and this might be the best approach but the most work, you could define a set of valid interval values in kibana, and then translate them into some valid, lowest common denominator unit for ES. e.g. 2 weeks becomes 1209600000 milliseconds

[ppisljar]

this is not the right approach, so closing this.