From ebe88a4c33970eb1e9cf70c95ded9d3c51444630 Mon Sep 17 00:00:00 2001 From: AlexanderSaydakov Date: Mon, 22 Apr 2019 17:04:13 -0700 Subject: [PATCH 1/4] handle empty sketches --- .../quantiles/DoublesSketchToHistogramPostAggregator.java | 3 +++ .../quantiles/DoublesSketchToQuantilesPostAggregator.java | 3 +++ 2 files changed, 6 insertions(+) diff --git a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToHistogramPostAggregator.java b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToHistogramPostAggregator.java index 2ec10b595f77..ad3237ffebe2 100644 --- a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToHistogramPostAggregator.java +++ b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToHistogramPostAggregator.java @@ -56,6 +56,9 @@ public DoublesSketchToHistogramPostAggregator( public Object compute(final Map combinedAggregators) { final DoublesSketch sketch = (DoublesSketch) field.compute(combinedAggregators); + if (sketch.isEmpty()) { + return new double[0]; + } final double[] histogram = sketch.getPMF(splitPoints); for (int i = 0; i < histogram.length; i++) { histogram[i] *= sketch.getN(); diff --git a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToQuantilesPostAggregator.java b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToQuantilesPostAggregator.java index 60a7064011f0..0187480271bf 100644 --- a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToQuantilesPostAggregator.java +++ b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToQuantilesPostAggregator.java @@ -75,6 +75,9 @@ public double[] getFractions() public Object compute(final Map combinedAggregators) { final DoublesSketch sketch = (DoublesSketch) field.compute(combinedAggregators); + if (sketch.isEmpty()) { + return new double[0]; + } return sketch.getQuantiles(fractions); } From 099a7bfdd2f85058949949e18daab9bdfe90ec53 Mon Sep 17 00:00:00 2001 From: AlexanderSaydakov Date: Wed, 24 Apr 2019 13:01:47 -0700 Subject: [PATCH 2/4] return array of NaN in case of empty sketch --- ...oublesSketchToHistogramPostAggregator.java | 4 +- ...oublesSketchToQuantilesPostAggregator.java | 4 +- ...esSketchToHistogramPostAggregatorTest.java | 80 ++++++++++++++++++ ...esSketchToQuantilesPostAggregatorTest.java | 82 +++++++++++++++++++ 4 files changed, 168 insertions(+), 2 deletions(-) create mode 100644 extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToHistogramPostAggregatorTest.java create mode 100644 extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToQuantilesPostAggregatorTest.java diff --git a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToHistogramPostAggregator.java b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToHistogramPostAggregator.java index ad3237ffebe2..da8f3c28345f 100644 --- a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToHistogramPostAggregator.java +++ b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToHistogramPostAggregator.java @@ -57,7 +57,9 @@ public Object compute(final Map combinedAggregators) { final DoublesSketch sketch = (DoublesSketch) field.compute(combinedAggregators); if (sketch.isEmpty()) { - return new double[0]; + final double[] histogram = new double[splitPoints.length + 1]; + Arrays.fill(histogram, Double.NaN); + return histogram; } final double[] histogram = sketch.getPMF(splitPoints); for (int i = 0; i < histogram.length; i++) { diff --git a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToQuantilesPostAggregator.java b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToQuantilesPostAggregator.java index 0187480271bf..e5089cbd07fb 100644 --- a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToQuantilesPostAggregator.java +++ b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToQuantilesPostAggregator.java @@ -76,7 +76,9 @@ public Object compute(final Map combinedAggregators) { final DoublesSketch sketch = (DoublesSketch) field.compute(combinedAggregators); if (sketch.isEmpty()) { - return new double[0]; + final double[] quantiles = new double[fractions.length]; + Arrays.fill(quantiles, Double.NaN); + return quantiles; } return sketch.getQuantiles(fractions); } diff --git a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToHistogramPostAggregatorTest.java b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToHistogramPostAggregatorTest.java new file mode 100644 index 000000000000..de682c8e6d3f --- /dev/null +++ b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToHistogramPostAggregatorTest.java @@ -0,0 +1,80 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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.apache.druid.query.aggregation.datasketches.quantiles; + +import org.apache.druid.query.aggregation.Aggregator; +import org.apache.druid.query.aggregation.PostAggregator; +import org.apache.druid.query.aggregation.TestDoubleColumnSelectorImpl; +import org.apache.druid.query.aggregation.post.FieldAccessPostAggregator; +import org.junit.Assert; +import org.junit.Test; + +import java.util.HashMap; +import java.util.Map; + +public class DoublesSketchToHistogramPostAggregatorTest +{ + @Test + public void emptySketch() + { + final Aggregator agg = new DoublesSketchBuildAggregator(null, 8); + + final Map fields = new HashMap(); + fields.put("sketch", agg.get()); + + final PostAggregator postAgg = new DoublesSketchToHistogramPostAggregator( + "histogram", + new FieldAccessPostAggregator("field", "sketch"), + new double[] {3.5} + ); + + final double[] histogram = (double[]) postAgg.compute(fields); + Assert.assertEquals(2, histogram.length); + Assert.assertTrue(Double.isNaN(histogram[0])); + Assert.assertTrue(Double.isNaN(histogram[1])); + } + + @Test + public void normalCase() + { + final double[] values = new double[] {1, 2, 3, 4, 5, 6}; + final TestDoubleColumnSelectorImpl selector = new TestDoubleColumnSelectorImpl(values); + + final Aggregator agg = new DoublesSketchBuildAggregator(selector, 8); + for (int i = 0; i < values.length; i++) { + agg.aggregate(); + selector.increment(); + } + + final Map fields = new HashMap(); + fields.put("sketch", agg.get()); + + final PostAggregator postAgg = new DoublesSketchToHistogramPostAggregator( + "histogram", + new FieldAccessPostAggregator("field", "sketch"), + new double[] {3.5} // splits distribution in two buckets of equal mass + ); + + final double[] histogram = (double[]) postAgg.compute(fields); + Assert.assertEquals(2, histogram.length); + Assert.assertEquals(3.0, histogram[0], 0); + Assert.assertEquals(3.0, histogram[1], 0); + } +} diff --git a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToQuantilesPostAggregatorTest.java b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToQuantilesPostAggregatorTest.java new file mode 100644 index 000000000000..38fa1349411f --- /dev/null +++ b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToQuantilesPostAggregatorTest.java @@ -0,0 +1,82 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF 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.apache.druid.query.aggregation.datasketches.quantiles; + +import org.apache.druid.query.aggregation.Aggregator; +import org.apache.druid.query.aggregation.PostAggregator; +import org.apache.druid.query.aggregation.TestDoubleColumnSelectorImpl; +import org.apache.druid.query.aggregation.post.FieldAccessPostAggregator; +import org.junit.Assert; +import org.junit.Test; + +import java.util.HashMap; +import java.util.Map; + +public class DoublesSketchToQuantilesPostAggregatorTest +{ + @Test + public void emptySketch() + { + final Aggregator agg = new DoublesSketchBuildAggregator(null, 8); + + final Map fields = new HashMap(); + fields.put("sketch", agg.get()); + + final PostAggregator postAgg = new DoublesSketchToQuantilesPostAggregator( + "quantiles", + new FieldAccessPostAggregator("field", "sketch"), + new double[] {0, 0.5, 1} + ); + + final double[] quantiles = (double[]) postAgg.compute(fields); + Assert.assertEquals(3, quantiles.length); + Assert.assertTrue(Double.isNaN(quantiles[0])); + Assert.assertTrue(Double.isNaN(quantiles[1])); + Assert.assertTrue(Double.isNaN(quantiles[2])); + } + + @Test + public void normalCase() + { + final double[] values = new double[] {1, 2, 3, 4, 5}; + final TestDoubleColumnSelectorImpl selector = new TestDoubleColumnSelectorImpl(values); + + final Aggregator agg = new DoublesSketchBuildAggregator(selector, 8); + for (int i = 0; i < values.length; i++) { + agg.aggregate(); + selector.increment(); + } + + final Map fields = new HashMap(); + fields.put("sketch", agg.get()); + + final PostAggregator postAgg = new DoublesSketchToQuantilesPostAggregator( + "quantiles", + new FieldAccessPostAggregator("field", "sketch"), + new double[] {0, 0.5, 1} + ); + + final double[] quantiles = (double[]) postAgg.compute(fields); + Assert.assertEquals(3, quantiles.length); + Assert.assertEquals(1.0, quantiles[0], 0); + Assert.assertEquals(3.0, quantiles[1], 0); + Assert.assertEquals(5.0, quantiles[2], 0); + } +} From ff91d989dca73060b6e83636c6c46a93ca36aaa4 Mon Sep 17 00:00:00 2001 From: AlexanderSaydakov Date: Wed, 24 Apr 2019 14:57:25 -0700 Subject: [PATCH 3/4] noinspection ForLoopReplaceableByForEach in tests --- .../quantiles/DoublesSketchToHistogramPostAggregatorTest.java | 1 + .../quantiles/DoublesSketchToQuantilesPostAggregatorTest.java | 1 + 2 files changed, 2 insertions(+) diff --git a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToHistogramPostAggregatorTest.java b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToHistogramPostAggregatorTest.java index de682c8e6d3f..443ba70de9ef 100644 --- a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToHistogramPostAggregatorTest.java +++ b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToHistogramPostAggregatorTest.java @@ -58,6 +58,7 @@ public void normalCase() final TestDoubleColumnSelectorImpl selector = new TestDoubleColumnSelectorImpl(values); final Aggregator agg = new DoublesSketchBuildAggregator(selector, 8); + //noinspection ForLoopReplaceableByForEach for (int i = 0; i < values.length; i++) { agg.aggregate(); selector.increment(); diff --git a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToQuantilesPostAggregatorTest.java b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToQuantilesPostAggregatorTest.java index 38fa1349411f..cd19057ec95f 100644 --- a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToQuantilesPostAggregatorTest.java +++ b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToQuantilesPostAggregatorTest.java @@ -59,6 +59,7 @@ public void normalCase() final TestDoubleColumnSelectorImpl selector = new TestDoubleColumnSelectorImpl(values); final Aggregator agg = new DoublesSketchBuildAggregator(selector, 8); + //noinspection ForLoopReplaceableByForEach for (int i = 0; i < values.length; i++) { agg.aggregate(); selector.increment(); From 678190ea12fd94910b6738fb1ad591c7cb3c6ee2 Mon Sep 17 00:00:00 2001 From: AlexanderSaydakov Date: Wed, 24 Apr 2019 15:04:49 -0700 Subject: [PATCH 4/4] style fixes --- .../DoublesSketchToHistogramPostAggregatorTest.java | 9 ++++++--- .../DoublesSketchToQuantilesPostAggregatorTest.java | 9 ++++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToHistogramPostAggregatorTest.java b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToHistogramPostAggregatorTest.java index 443ba70de9ef..b5aeec985fb3 100644 --- a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToHistogramPostAggregatorTest.java +++ b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToHistogramPostAggregatorTest.java @@ -34,9 +34,10 @@ public class DoublesSketchToHistogramPostAggregatorTest @Test public void emptySketch() { - final Aggregator agg = new DoublesSketchBuildAggregator(null, 8); + final TestDoubleColumnSelectorImpl selector = new TestDoubleColumnSelectorImpl(null); + final Aggregator agg = new DoublesSketchBuildAggregator(selector, 8); - final Map fields = new HashMap(); + final Map fields = new HashMap<>(); fields.put("sketch", agg.get()); final PostAggregator postAgg = new DoublesSketchToHistogramPostAggregator( @@ -46,6 +47,7 @@ public void emptySketch() ); final double[] histogram = (double[]) postAgg.compute(fields); + Assert.assertNotNull(histogram); Assert.assertEquals(2, histogram.length); Assert.assertTrue(Double.isNaN(histogram[0])); Assert.assertTrue(Double.isNaN(histogram[1])); @@ -64,7 +66,7 @@ public void normalCase() selector.increment(); } - final Map fields = new HashMap(); + final Map fields = new HashMap<>(); fields.put("sketch", agg.get()); final PostAggregator postAgg = new DoublesSketchToHistogramPostAggregator( @@ -74,6 +76,7 @@ public void normalCase() ); final double[] histogram = (double[]) postAgg.compute(fields); + Assert.assertNotNull(histogram); Assert.assertEquals(2, histogram.length); Assert.assertEquals(3.0, histogram[0], 0); Assert.assertEquals(3.0, histogram[1], 0); diff --git a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToQuantilesPostAggregatorTest.java b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToQuantilesPostAggregatorTest.java index cd19057ec95f..7a4edab5e443 100644 --- a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToQuantilesPostAggregatorTest.java +++ b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/DoublesSketchToQuantilesPostAggregatorTest.java @@ -34,9 +34,10 @@ public class DoublesSketchToQuantilesPostAggregatorTest @Test public void emptySketch() { - final Aggregator agg = new DoublesSketchBuildAggregator(null, 8); + final TestDoubleColumnSelectorImpl selector = new TestDoubleColumnSelectorImpl(null); + final Aggregator agg = new DoublesSketchBuildAggregator(selector, 8); - final Map fields = new HashMap(); + final Map fields = new HashMap<>(); fields.put("sketch", agg.get()); final PostAggregator postAgg = new DoublesSketchToQuantilesPostAggregator( @@ -46,6 +47,7 @@ public void emptySketch() ); final double[] quantiles = (double[]) postAgg.compute(fields); + Assert.assertNotNull(quantiles); Assert.assertEquals(3, quantiles.length); Assert.assertTrue(Double.isNaN(quantiles[0])); Assert.assertTrue(Double.isNaN(quantiles[1])); @@ -65,7 +67,7 @@ public void normalCase() selector.increment(); } - final Map fields = new HashMap(); + final Map fields = new HashMap<>(); fields.put("sketch", agg.get()); final PostAggregator postAgg = new DoublesSketchToQuantilesPostAggregator( @@ -75,6 +77,7 @@ public void normalCase() ); final double[] quantiles = (double[]) postAgg.compute(fields); + Assert.assertNotNull(quantiles); Assert.assertEquals(3, quantiles.length); Assert.assertEquals(1.0, quantiles[0], 0); Assert.assertEquals(3.0, quantiles[1], 0);