-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Refactor Metric aggregations #4332
Labels
Comments
russcam
added a commit
that referenced
this issue
Feb 6, 2020
This commit adds support for the string_stats aggregation introduced in Elasticsearch 7.6.0. It is a metric aggregation but does not implement IMetricAggregation because the type of Missing field is a string and not a double value. Missing is implemented as object as the hierarchy of metric aggregations will be changed as part of #4332, and Missing will be an object. The documentation for StringStats indicates that the distribution is returned in descending probability order, but are modelled as a JSON object. Following internal discussion, this is modelled as a dictionary on the response as it is considered this modelling will not diminish functionality. Closes #4369
russcam
added a commit
that referenced
this issue
Feb 10, 2020
This commit adds support for the string_stats aggregation introduced in Elasticsearch 7.6.0. It is a metric aggregation but does not implement IMetricAggregation because the type of Missing field is a string and not a double value. Missing is implemented as object as the hierarchy of metric aggregations will be changed as part of #4332, and Missing will be an object. The documentation for StringStats indicates that the distribution is returned in descending probability order, but are modelled as a JSON object. Following internal discussion, this is modelled as a dictionary on the response as it is considered this modelling will not diminish functionality. Closes #4369
russcam
added a commit
that referenced
this issue
Feb 10, 2020
This commit adds support for the string_stats aggregation introduced in Elasticsearch 7.6.0. It is a metric aggregation but does not implement IMetricAggregation because the type of Missing field is a string and not a double value. Missing is implemented as object as the hierarchy of metric aggregations will be changed as part of #4332, and Missing will be an object. The documentation for StringStats indicates that the distribution is returned in descending probability order, but are modelled as a JSON object. Following internal discussion, this is modelled as a dictionary on the response as it is considered this modelling will not diminish functionality. Closes #4369 (cherry picked from commit cc45fb1)
Code-gen should catch this for v8. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
As part of #4331, it looks like not all
IMetricAggregation
implementations support scripting i.e. theScript
propertyelasticsearch-net/src/Nest/Aggregations/Metric/MetricAggregation.cs
Line 17 in 4adf18a
Going through the aggregation builder types in Elasticsearch, whether an aggregation supports scripting, formatting, etc. is controlled by declaring the supported fields on the parser. For example, for
MinAggregationBuilder
https://github.com/elastic/elasticsearch/blob/17358b5af7b34a2b9a15846ba672f5a04001fcd5/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MinAggregationBuilder.java#L48
where the
bool
values arescriptable
,formattable
andtimezoneAware
, respectively.Only the
IMetricAggregation
that support scripting should allow aScript
property to be set. Going through the 7.x Java source, this is the support found(timezone aware is not a column because it doesn't look to apply to metric aggs)
(metric aggregations omitted from the list do not support either script or format)
Possible implementation
Derive an
IScriptableMetricAggregation
fromIMetricAggregation
, andIFormattableMetricAggregation
fromIScriptableMetricAggregation
NOTE:
Missing
should also be changed toobject
The text was updated successfully, but these errors were encountered: