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

Add hasValue() aggregation inspection helpers #36020

Merged
merged 18 commits into from
Jan 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.search.aggregations.matrix.stats;

/**
* Counterpart to {@link org.elasticsearch.search.aggregations.support.AggregationInspectionHelper}, providing
* helpers for some aggs in the MatrixStats package
*/
public class MatrixAggregationInspectionHelper {
polyfractal marked this conversation as resolved.
Show resolved Hide resolved
public static boolean hasValue(InternalMatrixStats agg) {
return agg.getResults() != null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,12 @@ public void testNoData() throws Exception {
.fields(Collections.singletonList("field"));
InternalMatrixStats stats = search(searcher, new MatchAllDocsQuery(), aggBuilder, ft);
assertNull(stats.getStats());
assertFalse(MatrixAggregationInspectionHelper.hasValue(stats));
}
}
}

@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/37587")
public void testTwoFields() throws Exception {
String fieldA = "a";
MappedFieldType ftA = new NumberFieldMapper.NumberFieldType(NumberFieldMapper.NumberType.DOUBLE);
Expand Down Expand Up @@ -87,8 +89,9 @@ public void testTwoFields() throws Exception {
IndexSearcher searcher = new IndexSearcher(reader);
MatrixStatsAggregationBuilder aggBuilder = new MatrixStatsAggregationBuilder("my_agg")
.fields(Arrays.asList(fieldA, fieldB));
InternalMatrixStats stats = search(searcher, new MatchAllDocsQuery(), aggBuilder, ftA, ftB);
InternalMatrixStats stats = searchAndReduce(searcher, new MatchAllDocsQuery(), aggBuilder, ftA, ftB);
multiPassStats.assertNearlyEqual(new MatrixStatsResults(stats.getStats()));
assertTrue(MatrixAggregationInspectionHelper.hasValue(stats));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.join.aggregations;

/**
* Counterpart to {@link org.elasticsearch.search.aggregations.support.AggregationInspectionHelper}, providing
* helpers for some aggs in the Join package
*/
public class JoinAggregationInspectionHelper {
polyfractal marked this conversation as resolved.
Show resolved Hide resolved

public static boolean hasValue(InternalParent agg) {
return agg.getDocCount() > 0;
}

public static boolean hasValue(InternalChildren agg) {
return agg.getDocCount() > 0;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ public void testNoDocs() throws IOException {
assertEquals(0, childrenToParent.getDocCount());
assertNotNull("Aggregations: " + childrenToParent.getAggregations().asMap(), parentAggregation);
assertEquals(Double.POSITIVE_INFINITY, ((InternalMin) parentAggregation).getValue(), Double.MIN_VALUE);
assertFalse(JoinAggregationInspectionHelper.hasValue(childrenToParent));
});
indexReader.close();
directory.close();
Expand Down Expand Up @@ -119,6 +120,7 @@ public void testParentChild() throws IOException {
parent.getAggregations().asMap(),
expectedTotalParents, parent.getDocCount());
assertEquals(expectedMinValue, ((InternalMin) parent.getAggregations().get("in_parent")).getValue(), Double.MIN_VALUE);
assertTrue(JoinAggregationInspectionHelper.hasValue(parent));
});

// verify for each children
Expand Down Expand Up @@ -170,6 +172,7 @@ public void testParentChildTerms() throws IOException {
// verify a terms-aggregation inside the parent-aggregation
testCaseTerms(new MatchAllDocsQuery(), indexSearcher, parent -> {
assertNotNull(parent);
assertTrue(JoinAggregationInspectionHelper.hasValue(parent));
LongTerms valueTerms = parent.getAggregations().get("value_terms");
assertNotNull(valueTerms);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ public void testParentChild() throws IOException {
expectedMinValue = Math.min(expectedMinValue, expectedValues.v2());
}
assertEquals(expectedTotalChildren, child.getDocCount());
assertTrue(JoinAggregationInspectionHelper.hasValue(child));
assertEquals(expectedMinValue, ((InternalMin) child.getAggregations().get("in_child")).getValue(), Double.MIN_VALUE);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ InternalBucket next() {
}
}

static class InternalBucket extends InternalMultiBucketAggregation.InternalBucket
public static class InternalBucket extends InternalMultiBucketAggregation.InternalBucket
implements CompositeAggregation.Bucket, KeyComparable<InternalBucket> {

private final CompositeKey key;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,4 @@ public String getWriteableName() {
protected InternalSingleBucketAggregation newAggregation(String name, long docCount, InternalAggregations subAggregations) {
return new InternalFilter(name, docCount, subAggregations, pipelineAggregators(), getMetaData());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
import java.util.List;
import java.util.Objects;

class GeoGridBucket extends InternalMultiBucketAggregation.InternalBucket implements GeoHashGrid.Bucket, Comparable<GeoGridBucket> {
public class GeoGridBucket extends InternalMultiBucketAggregation.InternalBucket implements GeoHashGrid.Bucket, Comparable<GeoGridBucket> {

protected long geohashAsLong;
protected long docCount;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,4 @@ protected InternalSingleBucketAggregation newAggregation(String name, long docCo
InternalAggregations subAggregations) {
return new InternalSampler(name, docCount, subAggregations, pipelineAggregators(), metaData);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ public long getEstimatedMemoryFootprint() {
return state.getEstimatedFootprintInBytes();
}

DoubleHistogram getState() {
return state;
}

@Override
public AbstractInternalHDRPercentiles doReduce(List<InternalAggregation> aggregations, ReduceContext reduceContext) {
DoubleHistogram merged = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ public long getEstimatedMemoryFootprint() {
return state.byteSize();
}

TDigestState getState() {
return state;
}

@Override
public AbstractInternalTDigestPercentiles doReduce(List<InternalAggregation> aggregations, ReduceContext reduceContext) {
TDigestState merged = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ public long getValue() {
return counts == null ? 0 : counts.cardinality(0);
}

HyperLogLogPlusPlus getCounts() {
return counts;
}

@Override
public InternalAggregation doReduce(List<InternalAggregation> aggregations, ReduceContext reduceContext) {
InternalCardinality reduced = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public String getWriteableName() {
return MedianAbsoluteDeviationAggregationBuilder.NAME;
}

public TDigestState getValuesSketch() {
TDigestState getValuesSketch() {
return valuesSketch;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ public Object aggregation() {
return aggregation.get(0);
}

List<Object> getAggregation() {
return aggregation;
}

@Override
public InternalAggregation doReduce(List<InternalAggregation> aggregations, ReduceContext reduceContext) {
List<Object> aggregationObjects = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.search.aggregations.metrics;

import org.elasticsearch.search.aggregations.pipeline.InternalDerivative;

/**
* Counterpart to {@link org.elasticsearch.search.aggregations.support.AggregationInspectionHelper}, providing
* helpers for some aggs that have package-private getters. AggregationInspectionHelper delegates to these
* helpers when needed, and consumers should prefer to use AggregationInspectionHelper instead of these
* helpers.
*/
public class MetricInspectionHelper {

public static boolean hasValue(InternalAvg agg) {
return agg.getCount() > 0;
}

public static boolean hasValue(InternalCardinality agg) {
return agg.getCounts() != null;
}

public static boolean hasValue(InternalHDRPercentileRanks agg) {
return agg.getState().getTotalCount() > 0;
}

public static boolean hasValue(InternalHDRPercentiles agg) {
return agg.getState().getTotalCount() > 0;
}

public static boolean hasValue(InternalMedianAbsoluteDeviation agg) {
return agg.getValuesSketch().size() > 0;
}

public static boolean hasValue(InternalScriptedMetric agg) {
// TODO better way to know if the scripted metric received documents?
// Could check for null too, but a script might return null on purpose...
return agg.getAggregation().size() > 0 ;
}

public static boolean hasValue(InternalTDigestPercentileRanks agg) {
return agg.getState().size() > 0;
}

public static boolean hasValue(InternalTDigestPercentiles agg) {
return agg.getState().size() > 0;
}

public static boolean hasValue(InternalTopHits agg) {
return (agg.getHits().getTotalHits().value == 0
&& Double.isNaN(agg.getHits().getMaxScore())
&& Double.isNaN(agg.getTopDocs().maxScore)) == false;
}

public static boolean hasValue(InternalWeightedAvg agg) {
return (agg.getSum() == 0.0 && agg.getWeight() == 0L) == false;
}

public static boolean hasValue(InternalDerivative agg) {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,10 @@ public InternalAggregation reduce(InternalAggregation aggregation, ReduceContext
} else {
final List<InternalAggregation> aggs = StreamSupport.stream(bucket.getAggregations().spliterator(), false).map(
(p) -> (InternalAggregation) p).collect(Collectors.toList());
aggs.add(new InternalSimpleValue(name(), returned.doubleValue(), formatter, new ArrayList<>(), metaData()));

InternalSimpleValue simpleValue = new InternalSimpleValue(name(), returned.doubleValue(),
formatter, new ArrayList<>(), metaData());
aggs.add(simpleValue);
InternalMultiBucketAggregation.InternalBucket newBucket = originalAgg.createBucket(new InternalAggregations(aggs),
bucket);
newBuckets.add(newBucket);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,7 @@ public InternalAggregation reduce(InternalAggregation aggregation, ReduceContext
sum += thisBucketValue;
}

List<InternalAggregation> aggs = StreamSupport
.stream(bucket.getAggregations().spliterator(), false)
List<InternalAggregation> aggs = StreamSupport.stream(bucket.getAggregations().spliterator(), false)
.map((p) -> (InternalAggregation) p)
.collect(Collectors.toList());
aggs.add(new InternalSimpleValue(name(), sum, formatter, new ArrayList<>(), metaData()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public InternalAggregation reduce(InternalAggregation aggregation, ReduceContext
// since we only change newBucket if we can add to it
Bucket newBucket = bucket;

if (!(thisBucketValue == null || thisBucketValue.equals(Double.NaN))) {
if ((thisBucketValue == null || thisBucketValue.equals(Double.NaN)) == false) {

// Some models (e.g. HoltWinters) have certain preconditions that must be met
if (model.hasValue(values.size())) {
Expand All @@ -126,7 +126,7 @@ public InternalAggregation reduce(InternalAggregation aggregation, ReduceContext
List<InternalAggregation> aggs = StreamSupport.stream(bucket.getAggregations().spliterator(), false)
.map((p) -> (InternalAggregation) p)
.collect(Collectors.toList());
aggs.add(new InternalSimpleValue(name(), movavg, formatter, new ArrayList<PipelineAggregator>(), metaData()));
aggs.add(new InternalSimpleValue(name(), movavg, formatter, new ArrayList<>(), metaData()));
newBucket = factory.createBucket(factory.getKey(bucket), bucket.getDocCount(), new InternalAggregations(aggs));
}

Expand All @@ -153,10 +153,10 @@ public InternalAggregation reduce(InternalAggregation aggregation, ReduceContext
Bucket bucket = newBuckets.get(lastValidPosition + i + 1);

// Get the existing aggs in the bucket so we don't clobber data
aggs = StreamSupport.stream(bucket.getAggregations().spliterator(), false).map((p) -> {
return (InternalAggregation) p;
}).collect(Collectors.toList());
aggs.add(new InternalSimpleValue(name(), predictions[i], formatter, new ArrayList<PipelineAggregator>(), metaData()));
aggs = StreamSupport.stream(bucket.getAggregations().spliterator(), false)
.map((p) -> (InternalAggregation) p)
.collect(Collectors.toList());
aggs.add(new InternalSimpleValue(name(), predictions[i], formatter, new ArrayList<>(), metaData()));

Bucket newBucket = factory.createBucket(newKey, bucket.getDocCount(), new InternalAggregations(aggs));

Expand All @@ -166,7 +166,7 @@ public InternalAggregation reduce(InternalAggregation aggregation, ReduceContext
} else {
// Not seen before, create fresh
aggs = new ArrayList<>();
aggs.add(new InternalSimpleValue(name(), predictions[i], formatter, new ArrayList<PipelineAggregator>(), metaData()));
aggs.add(new InternalSimpleValue(name(), predictions[i], formatter, new ArrayList<>(), metaData()));

Bucket newBucket = factory.createBucket(newKey, 0, new InternalAggregations(aggs));

Expand Down
Loading