-
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
Field stats: added index_constraint option #11259
Field stats: added index_constraint option #11259
Conversation
I'm personally a bit confused by the API. For instance the documentation says Instead, maybe the API should apply to the min/max values instead of the field themselves to be clearer about what it does? Eg. curl -XPOST 'http://localhost:9200/_field_stats?level=indices' -d '{
"fields" : {
"_timestamp.max" : {
"gte" : "2014-01-01T00:00:00.000Z"
},
"_timestamp.min" : {
"lt" : "2015-01-01T00:00:00.000Z"
}
}
}' |
My above comment does not necessarily mean I think it's a bug, but I wanted to raise the corner case so that our API gurus like @clintongormley could help figure out if the API is right. |
@jpountz @clintongormley The field stats filtering doesn't really behave like a filter on docs in an index. It is rather more of an overlapping mechanism / range matching. Lets say an index contains logs from So maybe we should name and document it differently? |
+1 I would like it better if the API made it clearer this performs range overlapping - the current one feels more like it will return indices that have a value in the specified range, which is not what it does. |
I like @jpountz 's suggestion a lot - makes things much clearer |
I like to make a small change to the format @jpountz is suggesting: curl -XPOST 'http://localhost:9200/_field_stats?level=indices' -d '{
"fields" : {
"_timestamp" : {
"max" : {
"gte" : "2014-01-01T00:00:00.000Z"
},
"min" : {
"lt" : "2015-01-01T00:00:00.000Z"
}
}
}
}' By having on top level |
See #11187 (comment) |
78745f3
to
7db5d58
Compare
I've updated this PR based on the comments in #11187. Also the title and description have been updated to match index constraints instead of field stats filter. |
case START_ARRAY: | ||
if ("fields".equals(fieldName)) { | ||
while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { | ||
if (token.isValue()) { |
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.
does this mean we will silently ignore other tokens?
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.
yes... instead lets fail if there is another token.
@rjernst Thanks for looking at this, I've updated the PR. |
d1fc3e9
to
718290a
Compare
If there is no more feedback then I like to merge this PR in the next 48 hours. |
if (token.isValue()) { | ||
fields.add(parser.text()); | ||
} else { | ||
throw new IllegalArgumentException("Unknown token [" + token + "]"); |
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.
Should it be "unexpected" rather than "unknown"?
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.
yes, it should! I'll change it
I left some comments, otherwise LGTM. The one I'm most concerned about is what to do if a request has both a body and a |
On 30 June 2015 at 14:36, Adrien Grand notifications@github.com wrote:
good point, we should just fail the request. |
a0843ff
to
dd91753
Compare
Field stats index constraints allows to omit all field stats for indices that don't match with the constraint. An index constraint can exclude indices' field stats based on the `min_value` and `max_value` statistic. This option is only useful if the `level` option is set to `indices`. For example index constraints can be useful to find out the min and max value of a particular property of your data in a time based scenario. The following request only returns field stats for the `answer_count` property for indices holding questions created in the year 2014: curl -XPOST 'http://localhost:9200/_field_stats?level=indices' -d '{ "fields" : ["answer_count"] <1> "index_constraints" : { <2> "creation_date" : { <3> "min_value" : { <4> "gte" : "2014-01-01T00:00:00.000Z", }, "max_value" : { "lt" : "2015-01-01T00:00:00.000Z" } } } }' Closes elastic#11187
dd91753
to
ef9d70b
Compare
Field stats index constraints allows to omit all field stats for indices that don't match with the index constraint. An index constraint can exclude indices' field stats based on a field's
min_value
andmax_value
statistics.For example index constraints can be useful to find out the min and max value of a particular property of your data in a time based scenario. The following request only returns field stats for the
answer_count
property for indices holding questions created in the year 2014:PR for #11187