Jackie-Jiang commented on code in PR #8769:
URL: https://github.com/apache/pinot/pull/8769#discussion_r881915146
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/MaxAggregationFunction.java:
##########
@@ -92,6 +93,15 @@ public void aggregate(int length, AggregationResultHolder
aggregationResultHolde
aggregationResultHolder.setValue(Math.max(max,
aggregationResultHolder.getDoubleResult()));
break;
}
+ case BIG_DECIMAL: {
+ BigDecimal[] values = blockValSet.getBigDecimalValuesSV();
+ BigDecimal max = values[0];
+ for (int i = 0; i < length & i < values.length; i++) {
+ max = values[i].max(max);
+ }
+ aggregationResultHolder.setValue(Math.max(max.doubleValue(),
aggregationResultHolder.getDoubleResult()));
Review Comment:
Suggest adding a comment (or TODO) stating even though the source data has
BIG_DECIMAL type, we still only support double precision. Same for other
functions
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/mutable/MutableSegmentImpl.java:
##########
@@ -582,6 +583,9 @@ private void addNewRow(int docId, GenericRow row) {
case DOUBLE:
forwardIndex.setDouble(docId, ((Number) value).doubleValue());
break;
+ case BIG_DECIMAL:
Review Comment:
Let's not add this case. Currently we don't support aggregated metric on
`BIG_DECIMAL` type (no support for overriding var-length mutable dictionary
entries)
--
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]
