PHILO-HE commented on code in PR #10125:
URL:
https://github.com/apache/incubator-gluten/pull/10125#discussion_r2224633199
##########
gluten-flink/planner/src/main/java/org/apache/gluten/rexnode/functions/BaseRexCallConverters.java:
##########
@@ -97,9 +109,89 @@ public ValidationResult isSuitable(RexCall callNode,
RexConversionContext contex
@Override
public TypedExpr toTypedExpr(RexCall callNode, RexConversionContext context)
{
List<TypedExpr> params = getParams(callNode, context);
- // If types are different, align them
- List<TypedExpr> alignedParams =
TypeUtils.promoteTypeForArithmeticExpressions(params);
+
Type resultType = getResultType(callNode);
+ if (params.size() == 2) {
+ Type leftType = params.get(0).getReturnType();
+ Type rightType = params.get(1).getReturnType();
+
+ if (leftType instanceof DecimalType || rightType instanceof DecimalType)
{
+ List<TypedExpr> alignedParams =
TypeUtils.promoteTypeForArithmeticExpressions(params);
+
+ Type alignedLeftType = alignedParams.get(0).getReturnType();
+ Type alignedRightType = alignedParams.get(1).getReturnType();
+
+ if (alignedLeftType instanceof DecimalType && alignedRightType
instanceof DecimalType) {
+ Type veloxResultType =
+ calculateDecimalResultType(
+ (DecimalType) alignedLeftType, (DecimalType)
alignedRightType, functionName);
+ TypedExpr veloxExpr = new CallTypedExpr(veloxResultType,
alignedParams, functionName);
+
+ return new CallTypedExpr(resultType, List.of(veloxExpr), "cast");
+ }
+ }
+ }
+
+ List<TypedExpr> alignedParams =
TypeUtils.promoteTypeForArithmeticExpressions(params);
return new CallTypedExpr(resultType, alignedParams, functionName);
}
+
+ private Type calculateDecimalResultType(
+ DecimalType leftType, DecimalType rightType, String operation) {
+ int leftPrecision = leftType.getPrecision();
+ int leftScale = leftType.getScale();
+ int rightPrecision = rightType.getPrecision();
+ int rightScale = rightType.getScale();
+
+ int resultPrecision;
+ int resultScale;
+
+ // Decimal precision and scale computation follows Velox/Spark decimal
rules
+ // Reference:
+ //
https://github.com/facebookincubator/velox/blob/main/velox/docs/functions/spark/decimal.rst
+ switch (operation) {
+ case "plus":
+ case "add":
+ case "minus":
+ case "subtract":
+ // precision = max(p1-s1, p2-s2) + max(s1, s2) + 1
+ // scale = max(s1, s2)
+ resultScale = Math.max(leftScale, rightScale);
+ resultPrecision =
+ Math.max(leftPrecision - leftScale, rightPrecision - rightScale) +
resultScale + 1;
+ break;
+
+ case "multiply":
+ // precision = p1 + p2 + 1
+ // scale = s1 + s2
+ resultPrecision = leftPrecision + rightPrecision + 1;
+ resultScale = leftScale + rightScale;
+ break;
+
+ case "divide":
+ // precision = p1 - s1 + s2 + max(6, s1 + p2 + 1)
+ // scale = max(6, s1 + p2 + 1)
+ resultScale = Math.max(6, leftScale + rightPrecision + 1);
+ resultPrecision = leftPrecision - leftScale + rightScale + resultScale;
+ break;
+
+ default:
+ resultPrecision = Math.max(leftPrecision, rightPrecision);
+ resultScale = Math.max(leftScale, rightScale);
+ break;
+ }
+
+ // Cap precision at 38 (maximum decimal precision in Spark/Velox)
+ if (resultPrecision > 38) {
+ resultPrecision = 38;
+ resultScale = Math.min(resultScale, resultPrecision);
+ if ("divide".equals(operation.toLowerCase())) {
Review Comment:
Remove `toLowerCase` if unnecessary.
##########
gluten-flink/planner/src/main/java/org/apache/gluten/rexnode/TypeUtils.java:
##########
@@ -53,6 +54,52 @@ public static boolean isStringType(Type type) {
}
public static List<TypedExpr>
promoteTypeForArithmeticExpressions(List<TypedExpr> expressions) {
Review Comment:
Seems only two inputs are considered for type promotion in arithmetic
operations. If so, use left & right as input variables. And revise the
implementation accordingly.
--
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]