jinchengchenghh commented on code in PR #5621:
URL: https://github.com/apache/incubator-gluten/pull/5621#discussion_r1593241871
##########
gluten-core/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala:
##########
@@ -510,42 +493,71 @@ object ExpressionConverter extends SQLConfHelper with
Logging {
expr.children.map(
replaceWithExpressionTransformerInternal(_, attributeSeq,
expressionsMap)),
expr)
- case b: BinaryArithmetic if DecimalArithmeticUtil.isDecimalArithmetic(b)
=>
- // PrecisionLoss=true: velox support / ch not support
- // PrecisionLoss=false: velox not support / ch support
- // TODO ch support PrecisionLoss=true
- if (!BackendsApiManager.getSettings.allowDecimalArithmetic) {
- throw new GlutenNotSupportException(
- s"Not support ${SQLConf.DECIMAL_OPERATIONS_ALLOW_PREC_LOSS.key} " +
- s"${conf.decimalOperationsAllowPrecisionLoss} mode")
- }
- val rescaleBinary = if
(BackendsApiManager.getSettings.rescaleDecimalLiteral) {
- DecimalArithmeticUtil.rescaleLiteral(b)
- } else {
- b
- }
- val (left, right) = DecimalArithmeticUtil.rescaleCastForDecimal(
- DecimalArithmeticUtil.removeCastForDecimal(rescaleBinary.left),
- DecimalArithmeticUtil.removeCastForDecimal(rescaleBinary.right))
- val leftChild = replaceWithExpressionTransformerInternal(left,
attributeSeq, expressionsMap)
- val rightChild =
- replaceWithExpressionTransformerInternal(right, attributeSeq,
expressionsMap)
- val resultType = DecimalArithmeticUtil.getResultTypeForOperation(
- DecimalArithmeticUtil.getOperationType(b),
- DecimalArithmeticUtil
- .getResultType(leftChild)
- .getOrElse(left.dataType.asInstanceOf[DecimalType]),
- DecimalArithmeticUtil
- .getResultType(rightChild)
- .getOrElse(right.dataType.asInstanceOf[DecimalType])
- )
+ case CheckOverflow(b: BinaryArithmetic, decimalType, _)
+ if DecimalArithmeticUtil.isDecimalArithmetic(b) &&
Review Comment:
Can you change the condition to
`BackendsApiManager.getSettings.dontTransformCheckOverflow &&
DecimalArithmeticUtil.isDecimalArithmetic(b)`, the first condition is easier to
check
--
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]
