From af98730148103f98ef15f3822210b98e721d3fe1 Mon Sep 17 00:00:00 2001 From: Igor Motov Date: Wed, 28 Oct 2020 19:24:27 -0400 Subject: [PATCH] Add hard_bounds support for histogram field-based histograms (#64246) (#64312) hard_bounds should now support histogram fields, previously hard bounds on histogram fields were ignored. Closes #62124 --- .../NumericHistogramAggregatorTests.java | 23 +++++++++++++++++++ .../HistoBackedHistogramAggregator.java | 20 ++++++++-------- .../HistoBackedHistogramAggregatorTests.java | 23 +++++++++++++++++++ 3 files changed, 57 insertions(+), 9 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/histogram/NumericHistogramAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/histogram/NumericHistogramAggregatorTests.java index 65a1ce1372f09..26988c26f61ee 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/histogram/NumericHistogramAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/histogram/NumericHistogramAggregatorTests.java @@ -428,6 +428,29 @@ public void testExtendedBounds() throws Exception { } } + public void testHardBounds() throws Exception { + try (Directory dir = newDirectory(); RandomIndexWriter w = new RandomIndexWriter(random(), dir)) { + for (double value : new double[] { 3.2, -5, -4.5, 4.3 }) { + Document doc = new Document(); + doc.add(new SortedNumericDocValuesField("field", NumericUtils.doubleToSortableLong(value))); + w.addDocument(doc); + } + + HistogramAggregationBuilder aggBuilder = new HistogramAggregationBuilder("my_agg").field("field") + .interval(5) + .hardBounds(new DoubleBounds(0.0, 10.0)); + MappedFieldType fieldType = new NumberFieldMapper.NumberFieldType("field", NumberFieldMapper.NumberType.DOUBLE); + try (IndexReader reader = w.getReader()) { + IndexSearcher searcher = new IndexSearcher(reader); + InternalHistogram histogram = searchAndReduce(searcher, new MatchAllDocsQuery(), aggBuilder, fieldType); + assertEquals(1, histogram.getBuckets().size()); + assertEquals(0d, histogram.getBuckets().get(0).getKey()); + assertEquals(2, histogram.getBuckets().get(0).getDocCount()); + assertTrue(AggregationInspectionHelper.hasValue(histogram)); + } + } + } + public void testAsSubAgg() throws IOException { AggregationBuilder request = new HistogramAggregationBuilder("outer").field("outer").interval(5).subAggregation( new HistogramAggregationBuilder("inner").field("inner").interval(5).subAggregation( diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/bucket/histogram/HistoBackedHistogramAggregator.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/bucket/histogram/HistoBackedHistogramAggregator.java index 1f97ebcfd0645..04479e057bae8 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/bucket/histogram/HistoBackedHistogramAggregator.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/bucket/histogram/HistoBackedHistogramAggregator.java @@ -76,16 +76,18 @@ public void collect(int doc, long owningBucketOrd) throws IOException { double key = Math.floor((value - offset) / interval); assert key >= previousKey; - long bucketOrd = bucketOrds.add(owningBucketOrd, Double.doubleToLongBits(key)); - if (bucketOrd < 0) { // already seen - bucketOrd = -1 - bucketOrd; - collectExistingBucket(sub, doc, bucketOrd); - } else { - collectBucket(sub, doc, bucketOrd); + if (hardBounds == null || hardBounds.contain(key * interval)) { + long bucketOrd = bucketOrds.add(owningBucketOrd, Double.doubleToLongBits(key)); + if (bucketOrd < 0) { // already seen + bucketOrd = -1 - bucketOrd; + collectExistingBucket(sub, doc, bucketOrd); + } else { + collectBucket(sub, doc, bucketOrd); + } + // We have added the document already. We should increment doc_count by count - 1 + // so that we have added it count times. + incrementBucketDocCount(bucketOrd, count - 1); } - // We have added the document already. We should increment doc_count by count - 1 - // so that we have added it count times. - incrementBucketDocCount(bucketOrd, count - 1); previousKey = key; } } diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/bucket/histogram/HistoBackedHistogramAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/bucket/histogram/HistoBackedHistogramAggregatorTests.java index 437d483c70407..54ca065190b90 100644 --- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/bucket/histogram/HistoBackedHistogramAggregatorTests.java +++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/bucket/histogram/HistoBackedHistogramAggregatorTests.java @@ -22,6 +22,7 @@ import org.elasticsearch.plugins.SearchPlugin; import org.elasticsearch.search.aggregations.AggregationBuilder; import org.elasticsearch.search.aggregations.AggregatorTestCase; +import org.elasticsearch.search.aggregations.bucket.histogram.DoubleBounds; import org.elasticsearch.search.aggregations.bucket.histogram.HistogramAggregationBuilder; import org.elasticsearch.search.aggregations.bucket.histogram.InternalHistogram; import org.elasticsearch.search.aggregations.metrics.TopHitsAggregationBuilder; @@ -164,6 +165,28 @@ public void testExtendedBounds() throws Exception { } } + public void testHardBounds() throws Exception { + try (Directory dir = newDirectory(); RandomIndexWriter w = new RandomIndexWriter(random(), dir)) { + + w.addDocument(singleton(histogramFieldDocValues(FIELD_NAME, new double[] { -4.5, 4.3 }))); + w.addDocument(singleton(histogramFieldDocValues(FIELD_NAME, new double[] { -5, 3.2 }))); + w.addDocument(singleton(histogramFieldDocValues(FIELD_NAME, new double[] { 1.0, 2.2 }))); + w.addDocument(singleton(histogramFieldDocValues(FIELD_NAME, new double[] { -6.0, 12.2 }))); + + HistogramAggregationBuilder aggBuilder = new HistogramAggregationBuilder("my_agg").field(FIELD_NAME) + .interval(5) + .hardBounds(new DoubleBounds(0.0, 5.0)); + try (IndexReader reader = w.getReader()) { + IndexSearcher searcher = new IndexSearcher(reader); + InternalHistogram histogram = searchAndReduce(searcher, new MatchAllDocsQuery(), aggBuilder, defaultFieldType(FIELD_NAME)); + assertEquals(1, histogram.getBuckets().size()); + assertEquals(0d, histogram.getBuckets().get(0).getKey()); + assertEquals(4, histogram.getBuckets().get(0).getDocCount()); + assertTrue(AggregationInspectionHelper.hasValue(histogram)); + } + } + } + /** * Test that sub-aggregations are not supported */