From 81fa972a498695fbe05015f5eabc3222dd576845 Mon Sep 17 00:00:00 2001 From: rich7420 Date: Fri, 11 Sep 2026 20:25:09 +0800 Subject: [PATCH 1/2] test: cover lpad and rpad routing configurations --- .../comet/CometStringExpressionSuite.scala | 35 ++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/spark/src/test/scala/org/apache/comet/CometStringExpressionSuite.scala b/spark/src/test/scala/org/apache/comet/CometStringExpressionSuite.scala index c58403487ac..b54d628ddb0 100644 --- a/spark/src/test/scala/org/apache/comet/CometStringExpressionSuite.scala +++ b/spark/src/test/scala/org/apache/comet/CometStringExpressionSuite.scala @@ -46,8 +46,8 @@ class CometStringExpressionSuite extends CometTestBase with CometCodegenAssertio testStringPadding("rpad") } - for ((function, expressionName) <- Seq("lpad" -> "StringLPad", "rpad" -> "StringRPad")) { - test(s"$function dispatches unsupported argument shapes (issue #5579)") { + for (function <- Seq("lpad", "rpad")) { + test(s"$function routing for unsupported argument shapes") { val data: Seq[(String, Option[Int], String)] = Seq( ("hi", Some(5), "xy"), ("hello", Some(3), "x"), @@ -61,15 +61,18 @@ class CometStringExpressionSuite extends CometTestBase with CometCodegenAssertio withSQLConf( SQLConf.OPTIMIZER_EXCLUDED_RULES.key -> "org.apache.spark.sql.catalyst.optimizer.ConstantFolding") { - for (allowIncompatible <- Seq("false", "true")) { - withSQLConf( - CometConf.getExprAllowIncompatConfigKey(expressionName) -> allowIncompatible) { + for (codegenEnabled <- Seq("false", "true")) { + withSQLConf(CometConf.COMET_SCALA_UDF_CODEGEN_ENABLED.key -> codegenEnabled) { for (query <- Seq( s"SELECT $function(_1, _2, _3) FROM tbl", s"SELECT $function('hi', _2, 'xy') FROM tbl", s"SELECT $function('hi', 5, 'xy') FROM tbl")) { - assertCodegenRan { - checkSparkAnswerAndOperator(query) + if (codegenEnabled.toBoolean) { + checkSparkAnswerAndImpl(query, native = Seq.empty, dispatched = Seq(function)) + } else { + checkSparkAnswerAndFallbackReason( + query, + s"$function: ${CometConf.COMET_SCALA_UDF_CODEGEN_ENABLED.key}=false") } } } @@ -78,16 +81,16 @@ class CometStringExpressionSuite extends CometTestBase with CometCodegenAssertio } } - test(s"$function keeps supported argument shapes native") { + test(s"$function routing for supported argument shapes") { withParquetTable(Seq(("hi", 5), ("hello", 3), ("", 0)), "tbl") { - for (query <- Seq( - s"SELECT $function(_1, _2) FROM tbl", - s"SELECT $function(_1, _2, 'xy') FROM tbl")) { - CometScalaUDFCodegen.resetStats() - checkSparkAnswerAndOperator(query) - assert( - CometScalaUDFCodegen.stats().totalLookups == 0, - s"expected native execution for $query") + for (codegenEnabled <- Seq("false", "true")) { + withSQLConf(CometConf.COMET_SCALA_UDF_CODEGEN_ENABLED.key -> codegenEnabled) { + for (query <- Seq( + s"SELECT $function(_1, _2) FROM tbl", + s"SELECT $function(_1, _2, 'xy') FROM tbl")) { + checkSparkAnswerAndImpl(query, native = Seq(function), dispatched = Seq.empty) + } + } } } } From c16150908a607d81dc9570cbbdc9df68fd7fb20d Mon Sep 17 00:00:00 2001 From: rich7420 Date: Tue, 15 Sep 2026 12:08:44 +0800 Subject: [PATCH 2/2] test: move padding routing coverage into SQL fixtures --- .../expressions/string/string_lpad.sql | 28 +++++++---- .../string/string_lpad_fallback.sql | 15 +++--- .../expressions/string/string_rpad.sql | 28 +++++++---- .../string/string_rpad_fallback.sql | 15 +++--- .../comet/CometStringExpressionSuite.scala | 50 ------------------- 5 files changed, 54 insertions(+), 82 deletions(-) diff --git a/spark/src/test/resources/sql-tests/expressions/string/string_lpad.sql b/spark/src/test/resources/sql-tests/expressions/string/string_lpad.sql index 27b38bbda58..145aa2a42b9 100644 --- a/spark/src/test/resources/sql-tests/expressions/string/string_lpad.sql +++ b/spark/src/test/resources/sql-tests/expressions/string/string_lpad.sql @@ -15,39 +15,47 @@ -- specific language governing permissions and limitations -- under the License. +-- Config: spark.comet.exec.scalaUDF.codegen.enabled=true + statement CREATE TABLE test_lpad(s string, len int, pad string) USING parquet statement -INSERT INTO test_lpad VALUES ('hi', 5, 'x'), ('hello', 3, 'x'), ('hi', 5, 'xy'), ('', 3, 'a'), (NULL, 5, 'x'), ('hi', 0, 'x'), ('hi', -1, 'x'), ('hi', NULL, 'x'), (NULL, NULL, 'x'), ('hi', 5, NULL), ('hi', 5, ''), ('', 3, ''), ('hi', -100, 'x'), (NULL, NULL, NULL), ('"hi', 7, '"x'), ('café', 7, '中文'), ('é', 5, '🙂') +INSERT INTO test_lpad VALUES ('hi', 5, 'x'), ('hello', 3, 'x'), ('hi', 5, 'xy'), ('', 3, 'a'), ('', 0, 'x'), (NULL, 5, 'x'), ('hi', 0, 'x'), ('hi', -1, 'x'), ('hi', NULL, 'x'), (NULL, NULL, 'x'), ('hi', 5, NULL), ('hi', 5, ''), ('', 3, ''), ('hi', -100, 'x'), (NULL, NULL, NULL), ('"hi', 7, '"x'), ('café', 7, '中文'), ('é', 5, '🙂') -- Column padding runs through the codegen dispatcher (issue #5579). -query +query expect_dispatch(lpad) SELECT lpad(s, len, pad) FROM test_lpad -query +query expect_dispatch(lpad) SELECT lpad(s, 5, pad) FROM test_lpad -query +query expect_dispatch(lpad) SELECT lpad('hi', len, pad) FROM test_lpad -query +query expect_dispatch(lpad) SELECT lpad('hi', len, 'xy') FROM test_lpad -query +query expect_dispatch(lpad) SELECT lpad('hi', len) FROM test_lpad -query +query expect_native(lpad) SELECT lpad(s, len) FROM test_lpad -- column + column + literal -query +query expect_native(lpad) SELECT lpad(s, len, 'x') FROM test_lpad +query expect_native(lpad) +SELECT lpad(s, len, 'xy') FROM test_lpad + -- column + literal + literal -query +query expect_native(lpad) SELECT lpad(s, 5, 'x') FROM test_lpad -- literal + literal + literal -query +query expect_dispatch(lpad) SELECT lpad('hi', 5, 'x'), lpad('hello', 3, 'x'), lpad('', 3, 'a'), lpad(NULL, 5, 'x') + +query expect_dispatch(lpad) +SELECT lpad('hi', 5, 'xy') FROM test_lpad diff --git a/spark/src/test/resources/sql-tests/expressions/string/string_lpad_fallback.sql b/spark/src/test/resources/sql-tests/expressions/string/string_lpad_fallback.sql index 32c476379d9..76020ae374d 100644 --- a/spark/src/test/resources/sql-tests/expressions/string/string_lpad_fallback.sql +++ b/spark/src/test/resources/sql-tests/expressions/string/string_lpad_fallback.sql @@ -21,20 +21,23 @@ statement CREATE TABLE test_lpad_fallback(s string, len int, pad string) USING parquet statement -INSERT INTO test_lpad_fallback VALUES ('hi', 5, 'xy'), ('hello', 3, 'x'), (NULL, NULL, NULL) +INSERT INTO test_lpad_fallback VALUES ('hi', 5, 'xy'), ('hello', 3, 'x'), ('', 3, 'a'), ('', 0, 'x'), ('hi', 5, ''), (NULL, 5, 'x'), ('hi', NULL, 'x'), ('hi', 5, NULL), (NULL, NULL, NULL) -query expect_fallback(spark.comet.exec.scalaUDF.codegen.enabled) +query expect_fallback(lpad: spark.comet.exec.scalaUDF.codegen.enabled=false) SELECT lpad(s, len, pad) FROM test_lpad_fallback -query expect_fallback(spark.comet.exec.scalaUDF.codegen.enabled) +query expect_fallback(lpad: spark.comet.exec.scalaUDF.codegen.enabled=false) SELECT lpad('hi', len, 'xy') FROM test_lpad_fallback -query expect_fallback(spark.comet.exec.scalaUDF.codegen.enabled) +query expect_fallback(lpad: spark.comet.exec.scalaUDF.codegen.enabled=false) SELECT lpad('hi', 5, 'xy') +query expect_fallback(lpad: spark.comet.exec.scalaUDF.codegen.enabled=false) +SELECT lpad('hi', 5, 'xy') FROM test_lpad_fallback + -- The native argument shapes do not require the dispatcher. -query +query expect_native(lpad) SELECT lpad(s, len, 'xy') FROM test_lpad_fallback -query +query expect_native(lpad) SELECT lpad(s, len) FROM test_lpad_fallback diff --git a/spark/src/test/resources/sql-tests/expressions/string/string_rpad.sql b/spark/src/test/resources/sql-tests/expressions/string/string_rpad.sql index a5f99512739..6f243867361 100644 --- a/spark/src/test/resources/sql-tests/expressions/string/string_rpad.sql +++ b/spark/src/test/resources/sql-tests/expressions/string/string_rpad.sql @@ -15,39 +15,47 @@ -- specific language governing permissions and limitations -- under the License. +-- Config: spark.comet.exec.scalaUDF.codegen.enabled=true + statement CREATE TABLE test_rpad(s string, len int, pad string) USING parquet statement -INSERT INTO test_rpad VALUES ('hi', 5, 'x'), ('hello', 3, 'x'), ('hi', 5, 'xy'), ('', 3, 'a'), (NULL, 5, 'x'), ('hi', 0, 'x'), ('hi', -1, 'x'), ('hi', NULL, 'x'), (NULL, NULL, 'x'), ('hi', 5, NULL), ('hi', 5, ''), ('', 3, ''), ('hi', -100, 'x'), (NULL, NULL, NULL), ('"hi', 7, '"x'), ('café', 7, '中文'), ('é', 5, '🙂') +INSERT INTO test_rpad VALUES ('hi', 5, 'x'), ('hello', 3, 'x'), ('hi', 5, 'xy'), ('', 3, 'a'), ('', 0, 'x'), (NULL, 5, 'x'), ('hi', 0, 'x'), ('hi', -1, 'x'), ('hi', NULL, 'x'), (NULL, NULL, 'x'), ('hi', 5, NULL), ('hi', 5, ''), ('', 3, ''), ('hi', -100, 'x'), (NULL, NULL, NULL), ('"hi', 7, '"x'), ('café', 7, '中文'), ('é', 5, '🙂') -- Column padding runs through the codegen dispatcher (issue #5579). -query +query expect_dispatch(rpad) SELECT rpad(s, len, pad) FROM test_rpad -query +query expect_dispatch(rpad) SELECT rpad(s, 5, pad) FROM test_rpad -query +query expect_dispatch(rpad) SELECT rpad('hi', len, pad) FROM test_rpad -query +query expect_dispatch(rpad) SELECT rpad('hi', len, 'xy') FROM test_rpad -query +query expect_dispatch(rpad) SELECT rpad('hi', len) FROM test_rpad -query +query expect_native(rpad) SELECT rpad(s, len) FROM test_rpad -- column + column + literal -query +query expect_native(rpad) SELECT rpad(s, len, 'x') FROM test_rpad +query expect_native(rpad) +SELECT rpad(s, len, 'xy') FROM test_rpad + -- column + literal + literal -query +query expect_native(rpad) SELECT rpad(s, 5, 'x') FROM test_rpad -- literal + literal + literal -query +query expect_dispatch(rpad) SELECT rpad('hi', 5, 'x'), rpad('hello', 3, 'x'), rpad('', 3, 'a'), rpad(NULL, 5, 'x') + +query expect_dispatch(rpad) +SELECT rpad('hi', 5, 'xy') FROM test_rpad diff --git a/spark/src/test/resources/sql-tests/expressions/string/string_rpad_fallback.sql b/spark/src/test/resources/sql-tests/expressions/string/string_rpad_fallback.sql index 8ac5bb36ba4..c2c37b7970d 100644 --- a/spark/src/test/resources/sql-tests/expressions/string/string_rpad_fallback.sql +++ b/spark/src/test/resources/sql-tests/expressions/string/string_rpad_fallback.sql @@ -21,20 +21,23 @@ statement CREATE TABLE test_rpad_fallback(s string, len int, pad string) USING parquet statement -INSERT INTO test_rpad_fallback VALUES ('hi', 5, 'xy'), ('hello', 3, 'x'), (NULL, NULL, NULL) +INSERT INTO test_rpad_fallback VALUES ('hi', 5, 'xy'), ('hello', 3, 'x'), ('', 3, 'a'), ('', 0, 'x'), ('hi', 5, ''), (NULL, 5, 'x'), ('hi', NULL, 'x'), ('hi', 5, NULL), (NULL, NULL, NULL) -query expect_fallback(spark.comet.exec.scalaUDF.codegen.enabled) +query expect_fallback(rpad: spark.comet.exec.scalaUDF.codegen.enabled=false) SELECT rpad(s, len, pad) FROM test_rpad_fallback -query expect_fallback(spark.comet.exec.scalaUDF.codegen.enabled) +query expect_fallback(rpad: spark.comet.exec.scalaUDF.codegen.enabled=false) SELECT rpad('hi', len, 'xy') FROM test_rpad_fallback -query expect_fallback(spark.comet.exec.scalaUDF.codegen.enabled) +query expect_fallback(rpad: spark.comet.exec.scalaUDF.codegen.enabled=false) SELECT rpad('hi', 5, 'xy') +query expect_fallback(rpad: spark.comet.exec.scalaUDF.codegen.enabled=false) +SELECT rpad('hi', 5, 'xy') FROM test_rpad_fallback + -- The native argument shapes do not require the dispatcher. -query +query expect_native(rpad) SELECT rpad(s, len, 'xy') FROM test_rpad_fallback -query +query expect_native(rpad) SELECT rpad(s, len) FROM test_rpad_fallback diff --git a/spark/src/test/scala/org/apache/comet/CometStringExpressionSuite.scala b/spark/src/test/scala/org/apache/comet/CometStringExpressionSuite.scala index b54d628ddb0..f5d594d1afd 100644 --- a/spark/src/test/scala/org/apache/comet/CometStringExpressionSuite.scala +++ b/spark/src/test/scala/org/apache/comet/CometStringExpressionSuite.scala @@ -46,56 +46,6 @@ class CometStringExpressionSuite extends CometTestBase with CometCodegenAssertio testStringPadding("rpad") } - for (function <- Seq("lpad", "rpad")) { - test(s"$function routing for unsupported argument shapes") { - val data: Seq[(String, Option[Int], String)] = Seq( - ("hi", Some(5), "xy"), - ("hello", Some(3), "x"), - ("", Some(3), "a"), - ("hi", Some(5), ""), - (null, Some(5), "x"), - ("hi", None, "x"), - ("hi", Some(5), null), - (null, None, null)) - withParquetTable(data, "tbl") { - withSQLConf( - SQLConf.OPTIMIZER_EXCLUDED_RULES.key -> - "org.apache.spark.sql.catalyst.optimizer.ConstantFolding") { - for (codegenEnabled <- Seq("false", "true")) { - withSQLConf(CometConf.COMET_SCALA_UDF_CODEGEN_ENABLED.key -> codegenEnabled) { - for (query <- Seq( - s"SELECT $function(_1, _2, _3) FROM tbl", - s"SELECT $function('hi', _2, 'xy') FROM tbl", - s"SELECT $function('hi', 5, 'xy') FROM tbl")) { - if (codegenEnabled.toBoolean) { - checkSparkAnswerAndImpl(query, native = Seq.empty, dispatched = Seq(function)) - } else { - checkSparkAnswerAndFallbackReason( - query, - s"$function: ${CometConf.COMET_SCALA_UDF_CODEGEN_ENABLED.key}=false") - } - } - } - } - } - } - } - - test(s"$function routing for supported argument shapes") { - withParquetTable(Seq(("hi", 5), ("hello", 3), ("", 0)), "tbl") { - for (codegenEnabled <- Seq("false", "true")) { - withSQLConf(CometConf.COMET_SCALA_UDF_CODEGEN_ENABLED.key -> codegenEnabled) { - for (query <- Seq( - s"SELECT $function(_1, _2) FROM tbl", - s"SELECT $function(_1, _2, 'xy') FROM tbl")) { - checkSparkAnswerAndImpl(query, native = Seq(function), dispatched = Seq.empty) - } - } - } - } - } - } - test("lpad/rpad with NULL length") { // FuzzDataGenerator never generates NULL integers (#5389), so build the rows explicitly. // Spark's StringLPad/StringRPad are null-intolerant: a NULL length yields a NULL row.