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

Unify histogram implementations #9446

Merged
merged 1 commit into from Jan 28, 2015
Merged

Unify histogram implementations #9446

merged 1 commit into from Jan 28, 2015

Conversation

colings86
Copy link
Contributor

This change makes InternalHistogram the only InternalAggregation used by the Histogram Aggregator. There is still a separate Bucket implementation and Factory implementation. All buckets are created through the factory passed into the InternalHistogram meaning and the correct factory implementation is serialised as part of the aggregation to make sure the correct bucket types are always generate.

This is needed by the Transformers (namely the derivative transformer) to allow it to generate buckets of the right type without having to know what the underlying bucket implementation is.

import java.util.List;
import java.util.Map;

/**
*
*/
public class InternalDateHistogram extends InternalHistogram<InternalDateHistogram.Bucket> {
public class InternalDateHistogram {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it still used?

Copy link
Contributor

Choose a reason for hiding this comment

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

oh I see, this class is just a namespace for buckets now

@colings86
Copy link
Contributor Author

@jpountz pushed a new commit from your comments

@jpountz
Copy link
Contributor

jpountz commented Jan 28, 2015

LGTM

This change makes InternalHistogram the only InternalAggregation used by the Histogram Aggregator. There is still a separate Bucket implementation and Factory implementation. All buckets are created through the factory passed into the InternalHistogram meaning and the correct factory implementation is serialised as part of the aggregation to make sure the correct bucket types are always generate.

This is needed by the Transformers (namely the derivative transformer) to allow it to generate buckets of the right type without having to know what the underlying bucket implementation is.
@colings86 colings86 merged commit 29c24d7 into elastic:master Jan 28, 2015
@colings86 colings86 deleted the feature/unifiedHistoImpl branch January 28, 2015 11:33
@clintongormley clintongormley changed the title Aggregations: Unify histogram implementations Unify histogram implementations Jun 6, 2015
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.

None yet

3 participants