Fix HyperUniquesAggregatorFactory.estimateCardinality null handling to respect output type (#10063)

* fix return type from HyperUniquesAggregator/HyperUniquesVectorAggregator

* address comments

* address comments
This commit is contained in:
Maytas Monsereenusorn 2020-06-23 15:54:37 -10:00 committed by GitHub
parent 978b494b46
commit f80c02da02
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 34 additions and 13 deletions

View File

@ -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();
}
}

View File

@ -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))
);

View File

@ -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;
@ -173,6 +174,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
{

View File

@ -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
);

View File

@ -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()
)
)