Jackie-Jiang commented on code in PR #18873:
URL: https://github.com/apache/pinot/pull/18873#discussion_r3797862250
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionUtils.java:
##########
@@ -799,10 +800,12 @@ private static HyperLogLogPlus
getDistinctValueHLLPlus(Dictionary dictionary, in
return hllPlus;
}
- private static HyperLogLog getDistinctCountHLLResult(Dictionary dictionary,
+ private static HyperLogLog getDistinctCountHLLResult(DataSource dataSource,
DistinctCountHLLAggregationFunction function, String explainPlanName) {
- if (dictionary.getValueType() == FieldSpec.DataType.BYTES) {
- // Treat BYTES value as serialized HyperLogLog
+ Dictionary dictionary = Objects.requireNonNull(dataSource.getDictionary());
Review Comment:
(minor) Not introduced in this PR, but to reduce overhead, we can change it
to:
```suggestion
Dictionary dictionary =dataSource.getDictionary();
assert dictionary != null;
```
Same for other explicit null check
##########
pinot-core/src/main/java/org/apache/pinot/core/query/distinct/table/BytesDistinctTable.java:
##########
@@ -290,9 +291,10 @@ private ResultTable toResultTableWithOrderBy() {
return new ResultTable(_dataSchema, rows);
}
- private static void addRows(ByteArray[] values, int length, List<Object[]>
rows) {
+ private void addRows(ByteArray[] values, int length, List<Object[]> rows) {
+ ColumnDataType columnDataType = _dataSchema.getColumnDataType(0);
Review Comment:
To reduce overhead, make it explicit type check and avoid per value switch
on `columnDataType`. Similar to how `IntDistinctTable` and `LongDistinctTable`
is handled
##########
pinot-core/src/main/java/org/apache/pinot/core/query/reduce/GroupByDataTableReducer.java:
##########
@@ -531,6 +531,8 @@ private Object getConvertedKey(DataTable dataTable,
ColumnDataType columnDataTyp
return dataTable.getString(rowId, colId);
case BYTES:
return dataTable.getBytes(rowId, colId).getBytes();
+ case UUID:
+ return columnDataType.convert(dataTable.getBytes(rowId, colId));
Review Comment:
Avoid extra switch under `convert`. Directly use `UuitUtils.toUUID()`
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
