This is an automated email from the ASF dual-hosted git repository.
maytasm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new f80c02d Fix HyperUniquesAggregatorFactory.estimateCardinality null
handling to respect output type (#10063)
f80c02d is described below
commit f80c02da02351cbf5c430f293b6c20418321ef47
Author: Maytas Monsereenusorn <[email protected]>
AuthorDate: Tue Jun 23 15:54:37 2020 -1000
Fix HyperUniquesAggregatorFactory.estimateCardinality null handling to
respect output type (#10063)
* fix return type from HyperUniquesAggregator/HyperUniquesVectorAggregator
* address comments
* address comments
---
.../hyperloglog/HyperUniquesAggregatorFactory.java | 10 +++------
.../apache/druid/query/SchemaEvolutionTest.java | 2 +-
.../HyperUniquesAggregatorFactoryTest.java | 25 ++++++++++++++++++++++
.../timeseries/TimeseriesQueryRunnerTest.java | 4 ++--
.../druid/query/topn/TopNQueryRunnerTest.java | 6 +++---
5 files changed, 34 insertions(+), 13 deletions(-)
diff --git
a/processing/src/main/java/org/apache/druid/query/aggregation/hyperloglog/HyperUniquesAggregatorFactory.java
b/processing/src/main/java/org/apache/druid/query/aggregation/hyperloglog/HyperUniquesAggregatorFactory.java
index ceb82d5..e0b7686 100644
---
a/processing/src/main/java/org/apache/druid/query/aggregation/hyperloglog/HyperUniquesAggregatorFactory.java
+++
b/processing/src/main/java/org/apache/druid/query/aggregation/hyperloglog/HyperUniquesAggregatorFactory.java
@@ -58,17 +58,13 @@ public class HyperUniquesAggregatorFactory extends
AggregatorFactory
{
public static Object estimateCardinality(@Nullable Object object, boolean
round)
{
- if (object == null) {
- return 0;
- }
-
final HyperLogLogCollector collector = (HyperLogLogCollector) object;
- // Avoid ternary, it causes estimateCardinalityRound to be cast to double.
+ // Avoid ternary for round check as it causes estimateCardinalityRound to
be cast to double.
if (round) {
- return collector.estimateCardinalityRound();
+ return collector == null ? 0L : collector.estimateCardinalityRound();
} else {
- return collector.estimateCardinality();
+ return collector == null ? 0d : collector.estimateCardinality();
}
}
diff --git
a/processing/src/test/java/org/apache/druid/query/SchemaEvolutionTest.java
b/processing/src/test/java/org/apache/druid/query/SchemaEvolutionTest.java
index 7e829e0..ce17b38 100644
--- a/processing/src/test/java/org/apache/druid/query/SchemaEvolutionTest.java
+++ b/processing/src/test/java/org/apache/druid/query/SchemaEvolutionTest.java
@@ -241,7 +241,7 @@ public class SchemaEvolutionTest
// index1 has no "uniques" column
Assert.assertEquals(
- timeseriesResult(ImmutableMap.of("uniques", 0)),
+ timeseriesResult(ImmutableMap.of("uniques", 0d)),
runQuery(query, factory, ImmutableList.of(index1))
);
diff --git
a/processing/src/test/java/org/apache/druid/query/aggregation/hyperloglog/HyperUniquesAggregatorFactoryTest.java
b/processing/src/test/java/org/apache/druid/query/aggregation/hyperloglog/HyperUniquesAggregatorFactoryTest.java
index f915192..421a457 100644
---
a/processing/src/test/java/org/apache/druid/query/aggregation/hyperloglog/HyperUniquesAggregatorFactoryTest.java
+++
b/processing/src/test/java/org/apache/druid/query/aggregation/hyperloglog/HyperUniquesAggregatorFactoryTest.java
@@ -30,6 +30,7 @@ import org.apache.druid.segment.TestHelper;
import org.junit.Assert;
import org.junit.Test;
+import java.nio.ByteBuffer;
import java.util.Comparator;
import java.util.Random;
@@ -174,6 +175,30 @@ public class HyperUniquesAggregatorFactoryTest
}
@Test
+ public void testEstimateCardinalityForZeroCardinality()
+ {
+ HyperLogLogCollector emptyHyperLogLogCollector =
HyperUniquesBufferAggregator.doGet(
+
ByteBuffer.allocate(HyperLogLogCollector.getLatestNumBytesForDenseStorage()),
+ 0
+ );
+
+ Assert.assertEquals(0L,
HyperUniquesAggregatorFactory.estimateCardinality(null, true));
+ Assert.assertEquals(0d,
HyperUniquesAggregatorFactory.estimateCardinality(null, false));
+
+ Assert.assertEquals(0L,
HyperUniquesAggregatorFactory.estimateCardinality(emptyHyperLogLogCollector,
true));
+ Assert.assertEquals(0d,
HyperUniquesAggregatorFactory.estimateCardinality(emptyHyperLogLogCollector,
false));
+
+ Assert.assertEquals(
+
HyperUniquesAggregatorFactory.estimateCardinality(emptyHyperLogLogCollector,
true).getClass(),
+ HyperUniquesAggregatorFactory.estimateCardinality(null,
true).getClass()
+ );
+ Assert.assertEquals(
+
HyperUniquesAggregatorFactory.estimateCardinality(emptyHyperLogLogCollector,
false).getClass(),
+ HyperUniquesAggregatorFactory.estimateCardinality(null,
false).getClass()
+ );
+ }
+
+ @Test
public void testSerde() throws Exception
{
final HyperUniquesAggregatorFactory factory = new
HyperUniquesAggregatorFactory(
diff --git
a/processing/src/test/java/org/apache/druid/query/timeseries/TimeseriesQueryRunnerTest.java
b/processing/src/test/java/org/apache/druid/query/timeseries/TimeseriesQueryRunnerTest.java
index 5d5af01..99989c0 100644
---
a/processing/src/test/java/org/apache/druid/query/timeseries/TimeseriesQueryRunnerTest.java
+++
b/processing/src/test/java/org/apache/druid/query/timeseries/TimeseriesQueryRunnerTest.java
@@ -2568,7 +2568,7 @@ public class TimeseriesQueryRunnerTest extends
InitializedNullHandlingTest
);
Assert.assertEquals(
0.0D,
- NullHandling.sqlCompatible() ? (Double) result[4] : (Integer)
result[4],
+ (Double) result[4],
0.02
);
Assert.assertEquals(
@@ -2581,7 +2581,7 @@ public class TimeseriesQueryRunnerTest extends
InitializedNullHandlingTest
result[3]
);
Assert.assertEquals(
- (Integer) result[4],
+ (Double) result[4],
0.0,
0.02
);
diff --git
a/processing/src/test/java/org/apache/druid/query/topn/TopNQueryRunnerTest.java
b/processing/src/test/java/org/apache/druid/query/topn/TopNQueryRunnerTest.java
index 3f5b41d..f98160a 100644
---
a/processing/src/test/java/org/apache/druid/query/topn/TopNQueryRunnerTest.java
+++
b/processing/src/test/java/org/apache/druid/query/topn/TopNQueryRunnerTest.java
@@ -647,15 +647,15 @@ public class TopNQueryRunnerTest extends
InitializedNullHandlingTest
Arrays.<Map<String, Object>>asList(
ImmutableMap.<String, Object>builder()
.put("market", "spot")
- .put("uniques", 0)
+ .put("uniques", 0d)
.build(),
ImmutableMap.<String, Object>builder()
.put("market", "total_market")
- .put("uniques", 0)
+ .put("uniques", 0d)
.build(),
ImmutableMap.<String, Object>builder()
.put("market", "upfront")
- .put("uniques", 0)
+ .put("uniques", 0d)
.build()
)
)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
