luis4a0 commented on code in PR #13103:
URL: https://github.com/apache/gluten/pull/13103#discussion_r4311931659
##########
backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxValidatorApi.scala:
##########
@@ -38,12 +39,32 @@ import io.substrait.proto.SimpleExtensionDeclaration
import scala.collection.JavaConverters._
import scala.collection.mutable.ArrayBuffer
-class VeloxValidatorApi extends ValidatorApi {
+class VeloxValidatorApi extends ValidatorApi with Logging {
import VeloxValidatorApi._
- /** For velox backend, key validation is on native side. */
- override def doExprValidate(substraitExprName: String, expr: Expression):
Boolean =
- true
+ /**
+ * Enforce decimal ROUND scale compatibility on the JVM independently of
optional native plan
+ * validation.
+ */
+ override def doExprValidate(substraitExprName: String, expr: Expression):
Boolean = expr match {
+ case round: Round if round.child.dataType.isInstanceOf[DecimalType] =>
Review Comment:
@rui-mo, the bounds overlap, but these checks are not equivalent under the
supported validation settings.
`spark.gluten.sql.enable.native.validation=false` skips the JNI/Substrait plan
validator. The Spark-expression eligibility hook still runs while constructing
the candidate Substrait plan:
- Mandatory expression eligibility:
https://github.com/apache/gluten/blob/d8b0c398116423e54832df0bdc52e6df40f9ea95/gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala#L964-L978
- Optional native plan validation:
https://github.com/apache/gluten/blob/d8b0c398116423e54832df0bdc52e6df40f9ea95/gluten-substrait/src/main/scala/org/apache/gluten/execution/WholeStageTransformer.scala#L91-L99
Without the JVM eligibility guard, that configuration could offload an
unsupported scale and fail in native execution instead of retaining Spark. For
example, Spark 4.1.1 successfully evaluates decimal ROUND at scales `401` and
`-401`, while our documented native domain rejects them.
This uses the existing `ValidatorApi.doExprValidate` path, not an additional
full validation pass. A rejected expression becomes a failed offload candidate
through the normal validation/fallback path. I am retaining the guard to
preserve this behavior independently of the optional native-validation flag.
`GlutenDecimalRoundSuite` explicitly exercises this configuration and checks
both the fallback plan and Spark-compatible results:
https://github.com/apache/gluten/blob/d8b0c398116423e54832df0bdc52e6df40f9ea95/gluten-ut/common/src/test/scala/org/apache/spark/sql/GlutenDecimalRoundSuiteBase.scala#L124-L162
--
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]
