github-actions[bot] commented on code in PR #61220:
URL: https://github.com/apache/doris/pull/61220#discussion_r2916627867
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/SumLiteralRewrite.java:
##########
@@ -204,10 +205,41 @@ private NamedExpression constructCount(SumInfo info,
Map<AggregateFunction, Name
// only support integer or float types
return null;
}
+ // Strip redundant widening integer cast introduced by type coercion.
+ // e.g. SUM(CAST(smallint_col AS INT) + 1) → after rewrite becomes
SUM(CAST(smallint_col AS INT)).
+ // Since SUM always returns BIGINT for any integer input,
CAST(smallint→int) is unnecessary
+ // and forces wider data reads. Strip it so we get SUM(smallint_col)
directly.
+ left = stripWideningIntegerCast(left);
SumInfo info = new SumInfo(left, ((Sum) func).isDistinct(), ((Sum)
func).isAlwaysNullable());
return Pair.of(namedExpression, Pair.of(info, (Literal) right));
}
+ /**
+ * Strip a widening integer cast that is redundant for SUM/COUNT.
+ * For example, CAST(smallint_col AS INT) → smallint_col.
+ *
+ * This is safe because:
+ * - SUM returns BIGINT for all integer inputs
(TINYINT/SMALLINT/INT/BIGINT),
+ * so widening the input before aggregation does not change the result.
+ * - COUNT just counts non-null values, unaffected by widening.
+ *
+ * Only implicit (type-coercion) casts between integer-like types are
stripped.
+ */
+ private static Expression stripWideningIntegerCast(Expression expr) {
+ if (!(expr instanceof Cast)) {
+ return expr;
+ }
+ Cast cast = (Cast) expr;
+ if (cast.isExplicitType()) {
+ return expr;
+ }
+ Expression inner = cast.child();
+ if (inner.getDataType().isIntegerLikeType() &&
cast.getDataType().isIntegerLikeType()) {
+ return inner;
Review Comment:
nit: The method is named `stripWideningIntegerCast` and the Javadoc says
"widening", but the condition `inner.getDataType().isIntegerLikeType() &&
cast.getDataType().isIntegerLikeType()` would also strip a hypothetical
**narrowing** implicit integer cast (e.g., `CAST(BIGINT AS INT)`).
In the current call context this is safe — binary arithmetic type coercion
only produces widening casts — but the method itself doesn't enforce the
widening invariant. Consider adding an explicit width check for robustness and
to match the documented intent:
```java
if (inner.getDataType().isIntegerLikeType() &&
cast.getDataType().isIntegerLikeType()
&& inner.getDataType().width() <= cast.getDataType().width()) {
return inner;
}
```
Or alternatively, add a comment noting that narrowing casts cannot reach
this path due to how `processBinaryArithmetic` works, so future readers won't
be confused by the mismatch between the name and the implementation.
--
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]
