luis4a0 commented on code in PR #13103:
URL: https://github.com/apache/gluten/pull/13103#discussion_r4422543823
##########
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, agreed. I removed the decimal-specific `doExprValidate` policy from
Scala and now rely on the existing native Substrait validator as the normal
compatibility gate.
Commit:
https://github.com/apache/gluten/commit/bb36d430eae3f0a17503599fea15054d0891c141
The behavior is now:
- With native validation enabled (the default), scales outside `[-400, 400]`
fail native validation and the expression falls back to Spark.
- With native validation disabled for debugging, the plan remains native and
the registered decimal special form reports its construction-time scale error
at execution.
I also updated the internal configuration description to make that
debug-mode behavior explicit.
The direct validator regression now verifies that Scala does not filter or
eagerly evaluate invalid literals, nonliteral scales, foldable expressions, or
an ANSI-true throwing cast. Integration tests cover normal fallback versus
debug-mode native failure, while typed NULL and bounded foldable scales
continue to execute natively.
Validation passed: native 3/3, API 2/2, Spark 4.1 SQL 4/4 on JDK 17 and 21,
full Spark 4.1 reactor build, active Spotless/Scalastyle, license headers, and
packaged native-library hash checks. Fresh upstream CI is still required.
--
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]