Copilot commented on code in PR #12775:
URL: https://github.com/apache/gluten/pull/12775#discussion_r3783306465
##########
backends-velox/src/test/scala/org/apache/gluten/execution/VeloxStringFunctionsSuite.scala:
##########
@@ -678,4 +679,87 @@ class VeloxStringFunctionsSuite extends
VeloxWholeStageTransformerSuite {
s"select l_orderkey, unbase64(base64(l_comment)) " +
s"from $LINEITEM_TABLE limit
$LENGTH")(checkGlutenPlan[ProjectExecTransformer])
}
+
+ // format_number integration tests
+
+ private def formatNumberInProject(
+ p: org.apache.spark.sql.execution.SparkPlan): Boolean =
+ p.expressions.exists(_.exists(_.isInstanceOf[FormatNumber]))
+
+ // Asserts format_number WAS offloaded to Velox: a ProjectExecTransformer
must carry the
+ // FormatNumber expression (proving native offload, not merely that some
unrelated
+ // transformer exists in the plan).
+ private def assertFormatNumberOffloaded(df: org.apache.spark.sql.DataFrame):
Unit = {
+ val plan = stripAQEPlan(df.queryExecution.executedPlan)
+ assert(
+ collectWithSubqueries(plan) {
+ case p: ProjectExecTransformer if formatNumberInProject(p) => p
+ }.nonEmpty,
+ "format_number should be offloaded to a ProjectExecTransformer")
+ }
+
+ // Asserts format_number was NOT offloaded to Velox: no
ProjectExecTransformer may carry a
+ // FormatNumber expression, and a vanilla Spark ProjectExec must (proving
genuine fallback
+ // rather than the expression merely being absent from the projection).
+ private def assertFormatNumberFallsBack(df: org.apache.spark.sql.DataFrame):
Unit = {
+ val plan = stripAQEPlan(df.queryExecution.executedPlan)
+ assert(
+ collectWithSubqueries(plan) {
+ case p: ProjectExecTransformer if formatNumberInProject(p) => p
+ }.isEmpty,
+ "format_number must not be offloaded to a ProjectExecTransformer")
+ assert(
+ collectWithSubqueries(plan) { case p: ProjectExec if
formatNumberInProject(p) => p }.nonEmpty,
+ "format_number should execute in a vanilla Spark ProjectExec")
+ }
+
+ test("format_number executes natively for integer input") {
+ runQueryAndCompare(
+ s"select l_orderkey, format_number(l_orderkey, 2) " +
+ s"from $LINEITEM_TABLE limit $LENGTH")(assertFormatNumberOffloaded)
+ }
+
+ test("format_number executes natively for double input") {
+ runQueryAndCompare(
+ s"select l_orderkey, format_number(CAST(l_orderkey AS DOUBLE), 4) " +
+ s"from $LINEITEM_TABLE limit $LENGTH")(assertFormatNumberOffloaded)
+ }
+
Review Comment:
The PR description states native offload applies to Byte/Short as well, but
the new integration tests only cover int/bigint/float/double. Adding explicit
tinyint/smallint cases would help catch type-lowering issues (especially around
casts) and better match the claimed support matrix.
--
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]
