xiangfu0 commented on code in PR #19988:
URL: https://github.com/apache/pinot/pull/19988#discussion_r4365666771
##########
pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/rules/PinotAggregateTypeArgumentRewriteRule.java:
##########
@@ -143,6 +170,12 @@ private static String getDataTypeArgument(AggregateCall
aggCall, RelNode input)
case DOUBLE:
return "DOUBLE";
case DECIMAL:
+ // The planner types an array of DECIMAL with scale 0 as INT_ARRAY or
LONG_ARRAY, see
+ // RelToPlanNodeConverter.resolveDecimal
+ if (name.equals(AggregationFunctionType.ARRAYAGG.name()) &&
dataType.getScale() == 0
+ && dataType.getPrecision() <= 38) {
+ return dataType.getPrecision() <= 10 ? "INT" : "LONG";
Review Comment:
Added in 13baff61e4: a plan case in `AggregatePlans.json` pins `'INT'` for
`DECIMAL(5, 0)`, `'LONG'` for `DECIMAL(20, 0)` and `'BIG_DECIMAL'` for
`DECIMAL(39, 0)`, matching `resolveDecimal`. `UDFAggregates.json` also runs
`DECIMAL(10, 0)` and `DECIMAL(20, 0)` end to end; with `'BIG_DECIMAL'` those
failed in `TypeUtils.convert`.
##########
pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/rules/PinotAggregateTypeArgumentRewriteRule.java:
##########
@@ -143,6 +170,12 @@ private static String getDataTypeArgument(AggregateCall
aggCall, RelNode input)
case DOUBLE:
return "DOUBLE";
case DECIMAL:
+ // The planner types an array of DECIMAL with scale 0 as INT_ARRAY or
LONG_ARRAY, see
+ // RelToPlanNodeConverter.resolveDecimal
+ if (name.equals(AggregationFunctionType.ARRAYAGG.name()) &&
dataType.getScale() == 0
+ && dataType.getPrecision() <= 38) {
+ return dataType.getPrecision() <= 10 ? "INT" : "LONG";
Review Comment:
Added in 13baff61e4: a plan case in `AggregatePlans.json` pins `'INT'` for
`DECIMAL(5, 0)`, `'LONG'` for `DECIMAL(20, 0)` and `'BIG_DECIMAL'` for
`DECIMAL(39, 0)`, matching `resolveDecimal`. `UDFAggregates.json` also runs
`DECIMAL(10, 0)` and `DECIMAL(20, 0)` end to end; with `'BIG_DECIMAL'` those
failed in `TypeUtils.convert`.
##########
pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/rules/PinotAggregateTypeArgumentRewriteRule.java:
##########
@@ -116,21 +124,40 @@ public void onMatch(RelOptRuleCall call) {
aggRel.copy(aggRel.getTraitSet(), newInput, aggRel.getGroupSet(),
aggRel.getGroupSets(), aggCalls));
}
- /// Returns the data type argument to append, or `null` when the call
already has one or is not such a call. Throws
- /// for a data argument without an explicit form, such as an array from a
multi-value column.
+ /// Returns the data type argument to add, or `null` when the call already
has one or is not such a call. Throws for
+ /// a data argument without an explicit form, such as an array from a
multi-value column in FIRST_WITH_TIME. The
+ /// mapped types match the single-stage `AggregateFunctionRewriteOptimizer`.
Review Comment:
Reworded in 13baff61e4. The Javadoc now says the mapping matches the
single-stage optimizer except for an ARRAY_AGG of a DECIMAL with scale 0, which
only an expression can produce, since a BIG_DECIMAL column is `DECIMAL(2000,
1000)`.
##########
pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/rules/PinotAggregateTypeArgumentRewriteRule.java:
##########
@@ -116,21 +124,40 @@ public void onMatch(RelOptRuleCall call) {
aggRel.copy(aggRel.getTraitSet(), newInput, aggRel.getGroupSet(),
aggRel.getGroupSets(), aggCalls));
}
- /// Returns the data type argument to append, or `null` when the call
already has one or is not such a call. Throws
- /// for a data argument without an explicit form, such as an array from a
multi-value column.
+ /// Returns the data type argument to add, or `null` when the call already
has one or is not such a call. Throws for
+ /// a data argument without an explicit form, such as an array from a
multi-value column in FIRST_WITH_TIME. The
+ /// mapped types match the single-stage `AggregateFunctionRewriteOptimizer`.
Review Comment:
Reworded in 13baff61e4. The Javadoc now says the mapping matches the
single-stage optimizer except for an ARRAY_AGG of a DECIMAL with scale 0, which
only an expression can produce, since a BIG_DECIMAL column is `DECIMAL(2000,
1000)`.
--
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]
