Jackie-Jiang commented on code in PR #20098:
URL: https://github.com/apache/pinot/pull/20098#discussion_r4418624397


##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountRawULLAggregationFunction.java:
##########
@@ -41,12 +42,24 @@ public SerializedULL extractFinalResult(@Nullable 
UltraLogLog intermediateResult
     return new SerializedULL(intermediateResult);
   }
 
+  @Override
   public AggregationFunctionType getType() {
-    return AggregationFunctionType.DISTINCTCOUNTULL;
+    return AggregationFunctionType.DISTINCTCOUNTRAWULL;

Review Comment:
   Good point. The PR description now calls out that unaliased single-stage 
queries return `distinctcountrawull(col)` instead of `distinctcountull(col)`, 
for clients that look up the column by name. The PR carries `release-notes` for 
it.



##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/SumPrecisionAggregationFunction.java:
##########
@@ -370,19 +370,21 @@ public ColumnDataType getFinalResultColumnType() {
   @Nullable
   @Override
   public BigDecimal extractFinalResult(@Nullable BigDecimal 
intermediateResult) {
-    if (intermediateResult == null) {
-      return null;
-    }
-    if (_precision == null) {
-      return intermediateResult;
-    }
-    BigDecimal result = intermediateResult.round(new MathContext(_precision, 
RoundingMode.HALF_EVEN));
-    return _scale == null ? result : result.setScale(_scale, 
RoundingMode.HALF_EVEN);
+    return intermediateResult != null ? round(intermediateResult) : null;
   }
 
   @Override
   public BigDecimal mergeFinalResult(BigDecimal finalResult1, BigDecimal 
finalResult2) {
-    return merge(finalResult1, finalResult2);
+    return round(merge(finalResult1, finalResult2));

Review Comment:
   Agreed. `mergeFinalResult` now has a Javadoc that states the caveat with 
this example, and the PR description no longer says the merge matches 
`extractFinalResult`.



-- 
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]

Reply via email to