Add hard_bounds support for histogram field-based histograms (elastic…

…#64246) hard_bounds should now support histogram fields, previously hard bounds on histogram fields were ignored. Closes elastic#62124
imotov · Oct 28, 2020 · c401cff · c401cff
1 parent aabf1c4
commit c401cff
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 9 deletions.
diff --git a/...g/elasticsearch/search/aggregations/bucket/histogram/NumericHistogramAggregatorTests.java b/...g/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/...csearch/xpack/analytics/aggregations/bucket/histogram/HistoBackedHistogramAggregator.java b/...csearch/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/...ch/xpack/analytics/aggregations/bucket/histogram/HistoBackedHistogramAggregatorTests.java b/...ch/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
      */