-
Notifications
You must be signed in to change notification settings - Fork 25.2k
Range Field Histogram Aggregation #41545
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
Changes from 20 commits
5c09a48
6d986f1
b9f906e
4ad0a0c
0b8c814
40a60eb
164c40e
7d93c56
10fee2e
4cfcebe
c9d36a7
28a1cfc
d962c1e
d93139c
b13b0ea
9af8074
40d3190
9b7448a
37a50ad
e90a192
2ddb99d
076cdad
ea2f9cc
0b3208d
0933933
f016c01
b48efdc
db9df49
b63c395
0e5a901
78bce8d
fa5db26
0183469
ac3e717
56b0641
854cc86
4da53b8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -108,6 +108,11 @@ public List<RangeFieldMapper.Range> decodeRanges(BytesRef bytes) { | |
throw new UnsupportedOperationException(); | ||
} | ||
|
||
@Override | ||
public Double doubleValue (Object endpointValue) { | ||
throw new UnsupportedOperationException("IP ranges cannot be safely converted to doubles"); | ||
} | ||
|
||
@Override | ||
public Query dvRangeQuery(String field, BinaryDocValuesRangeQuery.QueryType queryType, Object from, Object to, boolean includeFrom, | ||
boolean includeTo) { | ||
|
@@ -208,6 +213,11 @@ public List<RangeFieldMapper.Range> decodeRanges(BytesRef bytes) { | |
return LONG.decodeRanges(bytes); | ||
} | ||
|
||
@Override | ||
public Double doubleValue (Object endpointValue) { | ||
return LONG.doubleValue(endpointValue); | ||
} | ||
|
||
@Override | ||
public Query dvRangeQuery(String field, BinaryDocValuesRangeQuery.QueryType queryType, Object from, Object to, boolean includeFrom, | ||
boolean includeTo) { | ||
|
@@ -274,6 +284,12 @@ public List<RangeFieldMapper.Range> decodeRanges(BytesRef bytes) { | |
return BinaryRangeUtil.decodeFloatRanges(bytes); | ||
} | ||
|
||
@Override | ||
public Double doubleValue(Object endpointValue) { | ||
assert endpointValue instanceof Float; | ||
return ((Float) endpointValue).doubleValue(); | ||
} | ||
|
||
@Override | ||
public Query dvRangeQuery(String field, BinaryDocValuesRangeQuery.QueryType queryType, Object from, Object to, boolean includeFrom, | ||
boolean includeTo) { | ||
|
@@ -339,6 +355,12 @@ public List<RangeFieldMapper.Range> decodeRanges(BytesRef bytes) { | |
return BinaryRangeUtil.decodeDoubleRanges(bytes); | ||
} | ||
|
||
@Override | ||
public Double doubleValue(Object endpointValue) { | ||
assert endpointValue instanceof Double; | ||
return (Double)endpointValue; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style nit: should have space between cast and variable |
||
} | ||
|
||
@Override | ||
public Query dvRangeQuery(String field, BinaryDocValuesRangeQuery.QueryType queryType, Object from, Object to, boolean includeFrom, | ||
boolean includeTo) { | ||
|
@@ -407,6 +429,11 @@ public List<RangeFieldMapper.Range> decodeRanges(BytesRef bytes) { | |
return LONG.decodeRanges(bytes); | ||
} | ||
|
||
@Override | ||
public Double doubleValue (Object endpointValue) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style nit: space between method name and arguments |
||
return LONG.doubleValue(endpointValue); | ||
} | ||
|
||
@Override | ||
public Query dvRangeQuery(String field, BinaryDocValuesRangeQuery.QueryType queryType, Object from, Object to, boolean includeFrom, | ||
boolean includeTo) { | ||
|
@@ -461,6 +488,12 @@ public List<RangeFieldMapper.Range> decodeRanges(BytesRef bytes) { | |
return BinaryRangeUtil.decodeLongRanges(bytes); | ||
} | ||
|
||
@Override | ||
public Double doubleValue(Object endpointValue) { | ||
assert endpointValue instanceof Long; | ||
return ((Long) endpointValue).doubleValue(); | ||
} | ||
|
||
@Override | ||
public Query dvRangeQuery(String field, BinaryDocValuesRangeQuery.QueryType queryType, Object from, Object to, boolean includeFrom, | ||
boolean includeTo) { | ||
|
@@ -621,6 +654,19 @@ public Query rangeQuery(String field, boolean hasDocValues, Object from, Object | |
public abstract BytesRef encodeRanges(Set<RangeFieldMapper.Range> ranges) throws IOException; | ||
public abstract List<RangeFieldMapper.Range> decodeRanges(BytesRef bytes); | ||
|
||
/** | ||
* Given the Range.to or Range.from Object value from a Range instance, converts that value into a Double. Before converting, it | ||
* asserts that the object is of the expected type. Operation is not supported on IP ranges (because of loss of precision) | ||
* | ||
* @param endpointValue Object value for Range.to or Range.from | ||
* @return endpointValue as a Double | ||
*/ | ||
public abstract Double doubleValue(Object endpointValue); | ||
|
||
public boolean isNumeric() { | ||
return numberType != null; | ||
} | ||
|
||
public abstract Query dvRangeQuery(String field, BinaryDocValuesRangeQuery.QueryType queryType, Object from, Object to, | ||
boolean includeFrom, boolean includeTo); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,10 +22,9 @@ | |
import org.elasticsearch.search.aggregations.Aggregator; | ||
import org.elasticsearch.search.aggregations.AggregatorFactories; | ||
import org.elasticsearch.search.aggregations.AggregatorFactory; | ||
import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; | ||
import org.elasticsearch.search.aggregations.BucketOrder; | ||
import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; | ||
import org.elasticsearch.search.aggregations.support.ValuesSource; | ||
import org.elasticsearch.search.aggregations.support.ValuesSource.Numeric; | ||
import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory; | ||
import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; | ||
import org.elasticsearch.search.internal.SearchContext; | ||
|
@@ -34,15 +33,19 @@ | |
import java.util.List; | ||
import java.util.Map; | ||
|
||
public final class HistogramAggregatorFactory extends ValuesSourceAggregatorFactory<ValuesSource.Numeric, HistogramAggregatorFactory> { | ||
/** | ||
* Constructs the per-shard aggregator instance for histogram aggregation. Selects the numeric or range field implementation based on the | ||
* field type. | ||
*/ | ||
public final class HistogramAggregatorFactory extends ValuesSourceAggregatorFactory<ValuesSource, HistogramAggregatorFactory> { | ||
|
||
private final double interval, offset; | ||
private final BucketOrder order; | ||
private final boolean keyed; | ||
private final long minDocCount; | ||
private final double minBound, maxBound; | ||
|
||
public HistogramAggregatorFactory(String name, ValuesSourceConfig<Numeric> config, double interval, double offset, | ||
public HistogramAggregatorFactory(String name, ValuesSourceConfig<ValuesSource> config, double interval, double offset, | ||
BucketOrder order, boolean keyed, long minDocCount, double minBound, double maxBound, | ||
SearchContext context, AggregatorFactory<?> parent, | ||
AggregatorFactories.Builder subFactoriesBuilder, Map<String, Object> metaData) throws IOException { | ||
|
@@ -61,24 +64,35 @@ public long minDocCount() { | |
} | ||
|
||
@Override | ||
protected Aggregator doCreateInternal(ValuesSource.Numeric valuesSource, Aggregator parent, boolean collectsFromSingleBucket, | ||
protected Aggregator doCreateInternal(ValuesSource valuesSource, Aggregator parent, boolean collectsFromSingleBucket, | ||
List<PipelineAggregator> pipelineAggregators, Map<String, Object> metaData) throws IOException { | ||
if (collectsFromSingleBucket == false) { | ||
return asMultiBucketAggregator(this, context, parent); | ||
} | ||
return createAggregator(valuesSource, parent, pipelineAggregators, metaData); | ||
} | ||
|
||
private Aggregator createAggregator(ValuesSource.Numeric valuesSource, Aggregator parent, List<PipelineAggregator> pipelineAggregators, | ||
Map<String, Object> metaData) throws IOException { | ||
|
||
return new HistogramAggregator(name, factories, interval, offset, order, keyed, minDocCount, minBound, maxBound, valuesSource, | ||
config.format(), context, parent, pipelineAggregators, metaData); | ||
if (valuesSource instanceof ValuesSource.Numeric) { | ||
return new NumericHistogramAggregator(name, factories, interval, offset, order, keyed, minDocCount, minBound, maxBound, | ||
(ValuesSource.Numeric) valuesSource, config.format(), context, parent, pipelineAggregators, metaData); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style nit: |
||
else if (valuesSource instanceof ValuesSource.Bytes.FieldData.RangeFieldData) { | ||
ValuesSource.Bytes.FieldData.RangeFieldData rangeValueSource = (ValuesSource.Bytes.FieldData.RangeFieldData) valuesSource; | ||
if (rangeValueSource.rangeType().isNumeric() == false) { | ||
throw new IllegalArgumentException("Expected numeric range type but found non-numeric range [" | ||
+ rangeValueSource.rangeType().name + "]"); | ||
} | ||
return new RangeHistogramAggregator(name, factories, interval, offset, order, keyed, minDocCount, minBound, maxBound, | ||
(ValuesSource.Bytes.FieldData.RangeFieldData) valuesSource, config.format(), context, parent, pipelineAggregators, | ||
metaData); | ||
} | ||
else { | ||
throw new IllegalArgumentException("Expected one of [Numeric, RangeFieldData] values source, found [" | ||
+ valuesSource.toString() + "]"); | ||
} | ||
} | ||
|
||
@Override | ||
protected Aggregator createUnmapped(Aggregator parent, List<PipelineAggregator> pipelineAggregators, Map<String, Object> metaData) | ||
throws IOException { | ||
return createAggregator(null, parent, pipelineAggregators, metaData); | ||
return new NumericHistogramAggregator(name, factories, interval, offset, order, keyed, minDocCount, minBound, maxBound, | ||
null, config.format(), context, parent, pipelineAggregators, metaData); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm almost positive this is the wrong thing to do here, but I'm not sure what the right thing to do is. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, so this is a bit of a split personality in the framework right now. Setting the valuesSource to null will use a no-op collector in the histo's Terms agg (and a few others) instead have an "unmapped" version of the agg ( I'm not sure if there is a preference or why it's a split-personality. I've pinged @colings86 to see if he knows the history here :) |
||
} | ||
} |
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.
This is currently only being used as a flag, but the overhead between storing a boolean and storing an enum reference isn't high, so it seemed worth leaving ourselves access to the more robust data.