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

Numeric metric aggregations are now formattable #9032

Merged
merged 1 commit into from Jan 9, 2015
Merged

Numeric metric aggregations are now formattable #9032

merged 1 commit into from Jan 9, 2015

Conversation

colings86
Copy link
Contributor

You can now specify format in the request definition for most numeric metric aggregations. The exceptions are Percentile_Ranks, Cardinality and Value_Count as the response type of these can be different from the field type so the formatter won't work.

Closes #6812


public static interface MultiValue extends NumericMetricsAggregation {

String getValueAsString(String name);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels weird to have getValueAsString but not value here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I did think that. The problem is that we don't expose value(String) as a public method on the Java API but each aggregation exposes its own methods (e.g the stats aggregation exposes max(), min(), etc.) I did think about keeping this internal and adding xAsString() methods to each relevant multi value aggregation. Do you think that would be better?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think it would be more consistent?

@jpountz
Copy link
Contributor

jpountz commented Dec 22, 2014

LGTM, just left 2 minor comments

@colings86
Copy link
Contributor Author

@jpountz I updated this with your comments. Could you re-review?

@jpountz
Copy link
Contributor

jpountz commented Jan 9, 2015

LGTM

You can now specify `format` in the request definition for most numeric metric aggregations. The exceptions are Percentile_Ranks, Cardinality and Value_Count as the response type of these can be different from the field type so the formatter won't work.

Closes #6812
@colings86 colings86 merged commit 91e00c6 into elastic:master Jan 9, 2015
@colings86 colings86 deleted the feature/formattableMetrics branch January 9, 2015 17:01
olivere added a commit to olivere/elastic that referenced this pull request Feb 11, 2015
ES 1.5.0+ gets a format parameter for numeric metric aggregations (see
elastic/elasticsearch#9032).

Closes #38
@im-denisenko im-denisenko mentioned this pull request Apr 6, 2015
16 tasks
@clintongormley clintongormley changed the title Aggregations: Numeric metric aggregations are now formattable Numeric metric aggregations are now formattable Jun 6, 2015
olivere added a commit to olivere/elastic that referenced this pull request Jul 11, 2015
ES 1.5.0+ gets a format parameter for numeric metric aggregations (see
elastic/elasticsearch#9032).

Closes #38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show proper date format in response on max aggregation over timestamp
3 participants