gianm commented on a change in pull request #8023: Add round support for DS-HLL
URL: https://github.com/apache/incubator-druid/pull/8023#discussion_r300506591
##########
File path:
extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/hll/sql/HllSketchSqlAggregator.java
##########
@@ -135,7 +135,13 @@ public Aggregation toDruidAggregation(
final String aggregatorName = finalizeAggregations ?
Calcites.makePrefixedName(name, "a") : name;
if (columnArg.isDirectColumnAccess() &&
rowSignature.getColumnType(columnArg.getDirectColumn()) == ValueType.COMPLEX) {
- aggregatorFactory = new HllSketchMergeAggregatorFactory(aggregatorName,
columnArg.getDirectColumn(), logK, tgtHllType);
+ aggregatorFactory = new HllSketchMergeAggregatorFactory(
+ aggregatorName,
+ columnArg.getDirectColumn(),
+ logK,
+ tgtHllType,
+ false
Review comment:
`round` should be `true` here -- the `APPROX_COUNT_DISTINCT_DS_HLL` has a
return type of `SqlTypeName.BIGINT`, so rounding off the result is more correct
than what was happening before.
You should be able to notice a difference with a test case like this one in
`HllSketchSqlAggregatorTest`,
```
SELECT
dim2,
APPROX_COUNT_DISTINCT_DS_HLL(m1)
FROM druid.foo
GROUP BY dim2
HAVING APPROX_COUNT_DISTINCT_DS_HLL(m1) = 2
```
It should come up empty before patching, because the
HllSketchMergeAggregatorFactory underlying APPROX_COUNT_DISTINCT_DS_HLL never
actually returns a whole number `2`, it's returning some sort of 2-ish floating
point number. After patching it should return, I think, one row.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
