sam-1112 commented on code in PR #5415:
URL: https://github.com/apache/datafusion-comet/pull/5415#discussion_r3974818646
##########
spark/src/test/scala/org/apache/comet/CometRegExpJvmSuite.scala:
##########
@@ -171,6 +178,502 @@ class CometRegExpJvmSuite extends CometTestBase with
AdaptiveSparkPlanHelper {
}
}
+ private def withRLikeExplain(f: => Unit): Unit = {
+ withSQLConf(
+ CometConf.COMET_SCALA_UDF_CODEGEN_ENABLED.key -> "true",
+ CometConf.COMET_EXPLAIN_CODEGEN_ENABLED.key -> "true",
+ CometConf.COMET_EXTENDED_EXPLAIN_FORMAT.key ->
+ CometConf.COMET_EXTENDED_EXPLAIN_FORMAT_VERBOSE)(f)
+ }
+
+ private def explainOf(df: org.apache.spark.sql.DataFrame): String =
+ new
ExtendedExplainInfo().generateExtendedInfo(df.queryExecution.executedPlan)
+
+ private def assertSparkRegexError(query: String): Unit = {
+ def collectError(): Throwable =
+ intercept[Throwable](sql(query).collect())
+
+ def chain(ex: Throwable): List[Throwable] =
+ Iterator.iterate(ex)(_.getCause).takeWhile(_ != null).toList
+
+ var sparkEx: Throwable = null
+ withSQLConf(CometConf.COMET_ENABLED.key -> "false") {
+ sparkEx = collectError()
+ }
+ val cometEx = collectError()
+ val sparkMsgs = chain(sparkEx).flatMap(e =>
Option(e.getMessage)).mkString("\n")
+ val cometMsgs = chain(cometEx).flatMap(e =>
Option(e.getMessage)).mkString("\n")
+ assert(
+ sparkMsgs.toLowerCase.contains("unclosed") ||
sparkMsgs.contains("PatternSyntax") ||
+ sparkMsgs.toLowerCase.contains("regex"),
+ s"Spark error did not look like a regex syntax error: $sparkMsgs")
+ assert(
+ cometMsgs.toLowerCase.contains("unclosed") ||
cometMsgs.contains("PatternSyntax") ||
+ cometMsgs.toLowerCase.contains("regex"),
+ s"Comet error did not look like a regex syntax error: $cometMsgs")
+ val sparkTypes = chain(sparkEx).map(_.getClass.getName)
+ val cometTypes = chain(cometEx).map(_.getClass.getName)
+ assert(
+ sparkTypes.exists(cometTypes.contains),
+ s"Comet exception types $cometTypes did not share a type with Spark
$sparkTypes")
+ }
+
+ test("rlike: safe literal pattern takes the native path by default") {
+ withRLikeExplain {
+ withSubjects("abc123", "xyz", null, "abc") {
+ val df = sql("SELECT s, s rlike 'abc[0-9]+' FROM t")
+ checkSparkAnswerAndOperator(df)
+ val explain = explainOf(df)
+ assert(
+ !explain.contains("JVM codegen dispatcher: rlike"),
+ s"expected native path for in-subset pattern, got:\n$explain")
+ }
+ }
+ }
+
+ test("rlike: Rust class set operations stay on the dispatcher") {
+ withRLikeExplain {
+ withSubjects("~", "a", "b", "x", null) {
+ Seq("[a~~b]", "[^a~~b]").foreach { pat =>
+ val df = sql(s"SELECT s, s rlike '$pat' FROM t")
+ checkSparkAnswerAndOperator(df)
+ assert(
+ explainOf(df).contains("JVM codegen dispatcher: rlike"),
+ s"expected dispatcher for $pat, got:\n${explainOf(df)}")
+ }
+ }
+ withSubjects("b", "a", "z", "-", null) {
+ val df = sql("SELECT s, s rlike '[a-z--b]' FROM t")
+ checkSparkAnswerAndOperator(df)
+ assert(
+ explainOf(df).contains("JVM codegen dispatcher: rlike"),
+ s"expected dispatcher for [a-z--b], got:\n${explainOf(df)}")
+ }
+ }
+ }
+
+ test("rlike: leading-bracket class ranges stay on the dispatcher") {
+ withRLikeExplain {
+ withSubjects("_", "-", "]", "a", "z", null) {
+ Seq("[]-a]", "[^]-a]").foreach { pat =>
+ val df = sql(s"SELECT s, s rlike '$pat' FROM t")
+ checkSparkAnswerAndOperator(df)
+ assert(
+ explainOf(df).contains("JVM codegen dispatcher: rlike"),
+ s"expected dispatcher for [$pat], got:\n${explainOf(df)}")
+ }
+ }
+ }
+ }
+
+ test("rlike: raw [ range endpoint stays on the dispatcher and preserves
Spark error") {
+ withRLikeExplain {
+ withSubjects("@", "[", "A", null) {
+ Seq("[@-[]", "[^@-[]").foreach { pat =>
+ val query = s"SELECT s, s rlike '$pat' FROM t"
+ val df = sql(query)
+ assert(
+ explainOf(df).contains("JVM codegen dispatcher: rlike"),
+ s"expected dispatcher for $pat, got:\n${explainOf(df)}")
+ assertSparkRegexError(query)
+ }
+ }
+ }
+ }
+
+ test("rlike: over-budget counted repetition stays on the dispatcher") {
+ withRLikeExplain {
+ withSubjects("a", "aaa", null) {
+ val df = sql("SELECT s, s rlike 'a{1000000}' FROM t")
+ checkSparkAnswerAndOperator(df)
+ assert(
+ explainOf(df).contains("JVM codegen dispatcher: rlike"),
+ s"expected dispatcher for a{1000000}, got:\n${explainOf(df)}")
+ }
+ withSubjects(";", "x", "xx", null) {
+ val df = sql("SELECT s, s rlike '[^;]{20000}' FROM t")
+ checkSparkAnswerAndOperator(df)
+ assert(
+ explainOf(df).contains("JVM codegen dispatcher: rlike"),
+ s"expected dispatcher for [^;]{20000}, got:\n${explainOf(df)}")
+ }
+ withSubjects("a", "aaa", null) {
+ val df = sql("SELECT s, s rlike '(a{100}){100}' FROM t")
+ checkSparkAnswerAndOperator(df)
+ assert(
+ explainOf(df).contains("JVM codegen dispatcher: rlike"),
+ s"expected dispatcher for (a{100}){100}, got:\n${explainOf(df)}")
+ }
+ withSubjects("", "x", ";" * 256, null) {
+ val pat = "(([^;]{256}){0,}){256}"
+ val df = sql(s"SELECT s, s rlike '$pat' FROM t")
+ checkSparkAnswerAndOperator(df)
+ assert(
+ explainOf(df).contains("JVM codegen dispatcher: rlike"),
+ s"expected dispatcher for $pat, got:\n${explainOf(df)}")
+ }
+ withSubjects("a", "b", null) {
+ val nested = "(" * 33 + "a" + ")" * 33
+ val df = sql(s"SELECT s, s rlike '$nested' FROM t")
+ checkSparkAnswerAndOperator(df)
+ assert(
+ explainOf(df).contains("JVM codegen dispatcher: rlike"),
+ s"expected dispatcher for 33 nested groups, got:\n${explainOf(df)}")
+ }
+ }
+ }
+
+ test("rlike: compile-budget boundary stays native") {
+ withRLikeExplain {
+ withSubjects("a", "aaa", null) {
+ val pat = "(?:a{64}){64}"
+ val df = sql(s"SELECT s, s rlike '$pat' FROM t")
+ checkSparkAnswerAndOperator(df)
+ assert(
+ !explainOf(df).contains("JVM codegen dispatcher: rlike"),
+ s"expected native path for expansion-4096 pattern,
got:\n${explainOf(df)}")
+ }
+ }
+ }
+
+ test("rlike: exact-zero counted repetitions stay native") {
+ withRLikeExplain {
+ withSubjects("", "x", ";" * 256, null) {
+ Seq("(([^;]{256}){0}){256}", "(([^;]{256}){0,0}){256}").foreach { pat
=>
+ val df = sql(s"SELECT s, s rlike '$pat' FROM t")
+ checkSparkAnswerAndOperator(df)
+ assert(
+ !explainOf(df).contains("JVM codegen dispatcher: rlike"),
+ s"expected native path for exact-zero pattern $pat,
got:\n${explainOf(df)}")
+ }
+ }
+ }
+ }
+
+ test("rlike: aggregate expansion budget stays on the dispatcher") {
+ withRLikeExplain {
+ withSubjects("a", "aaa", null) {
+ Seq("a{256}" * 17, "a{0}" * 4097).foreach { pat =>
+ val df = sql(s"SELECT s, s rlike '$pat' FROM t")
+ checkSparkAnswerAndOperator(df)
+ assert(
+ explainOf(df).contains("JVM codegen dispatcher: rlike"),
+ s"expected dispatcher for aggregate over-budget pattern,
got:\n${explainOf(df)}")
+ }
+ }
+ }
+ }
+
+ test("rlike: nested quantified stars stay on the dispatcher") {
+ withRLikeExplain {
+ withSubjects("b", "c", null) {
+ val q = (1 to 30).foldLeft("a") { (p, _) => s"($p)*" }
+ val pat = s"(($q){255}){16}b"
+ val df = sql(s"SELECT s, s rlike '$pat' FROM t")
+ checkSparkAnswerAndOperator(df)
+ assert(
+ explainOf(df).contains("JVM codegen dispatcher: rlike"),
+ s"expected dispatcher for nested quantified stars,
got:\n${explainOf(df)}")
+ }
+ }
+ }
+
+ test("rlike: capturing groups copied by counted repetition stay on the
dispatcher") {
+ withRLikeExplain {
+ withSubjects("x", ";", null) {
+ val q = (1 to 7).foldLeft("[^;]") { (p, _) => s"($p)*" }
+ val wrapped = ("(" * 16) + q + (")" * 16)
+ val pat = (wrapped + "{256}") * 16
+ val df = sql(s"SELECT s, s rlike '$pat' FROM t")
+ checkSparkAnswerAndOperator(df)
+ assert(
+ explainOf(df).contains("JVM codegen dispatcher: rlike"),
+ s"expected dispatcher for capture-cost residual,
got:\n${explainOf(df)}")
+ }
+ withSubjects("x", "y", null) {
+ val df = sql("SELECT s, s rlike '[^x]{256}' FROM t")
+ checkSparkAnswerAndOperator(df)
+ assert(
+ !explainOf(df).contains("JVM codegen dispatcher: rlike"),
+ s"expected native path for [^x]{256}, got:\n${explainOf(df)}")
+ }
+ }
+ }
+
+ test("rlike: uncounted capturing groups stay on the dispatcher") {
+ withRLikeExplain {
+ withSubjects("", "a", null) {
+ val pattern = uncountedCapturePattern(branches = 64)
+ val df = spark.table("t").select(col("s"), col("s").rlike(pattern))
+ checkSparkAnswerAndOperator(df)
+ assert(
+ explainOf(df).contains("JVM codegen dispatcher: rlike"),
+ s"expected dispatcher for uncounted capture budget residual,
got:\n${explainOf(df)}")
+ }
+ }
+ }
+
+ test("rlike: uncounted capturing groups fall back to Spark when the
dispatcher is disabled") {
+ withSubjects("", "a", null) {
+ withSQLConf(
+ CometConf.getExprAllowIncompatConfigKey("RLike") -> "false",
+ CometConf.COMET_SCALA_UDF_CODEGEN_ENABLED.key -> "false") {
+ val pattern = uncountedCapturePattern(branches = 64)
+ val df = spark.table("t").select(col("s"), col("s").rlike(pattern))
+ val (_, cometPlan) = checkSparkAnswerAndFallbackReason(
+ df,
+ CometConf.COMET_SCALA_UDF_CODEGEN_ENABLED.key + "=false")
+ val explain = new ExtendedExplainInfo().generateExtendedInfo(cometPlan)
+ assert(
+ !explain.toLowerCase.contains("compiledtoobig"),
+ s"native compile must not run for over-budget captures,
got:\n$explain")
+ assert(
+ !explain.contains("JVM codegen dispatcher: rlike"),
+ s"dispatcher is disabled; expected Spark fallback, got:\n$explain")
+ }
+ }
+ }
+
+ test(
+ "rlike: full uncounted-capture reproducer falls back to Spark when JVM
dispatcher is disabled") {
+ assume(isSpark40Plus && !isSpark41Plus, "full 4096-branch reproducer is
Spark 4.0-only")
+ assume(
+ sys.props("java.specification.version") == "21",
+ "full 4096-branch reproducer is JDK 21-only")
+ withSubjects("", "a", null) {
+ withSQLConf(
+ CometConf.getExprAllowIncompatConfigKey("RLike") -> "false",
+ CometConf.COMET_SCALA_UDF_CODEGEN_ENABLED.key -> "false") {
+ val pattern = uncountedCapturePattern(branches = 4096)
+ assert(pattern.length == 303103)
+ val df = spark.table("t").select(col("s"), col("s").rlike(pattern))
+ val (_, cometPlan) = checkSparkAnswerAndFallbackReason(
+ df,
+ CometConf.COMET_SCALA_UDF_CODEGEN_ENABLED.key + "=false")
+ val explain = new ExtendedExplainInfo().generateExtendedInfo(cometPlan)
+ assert(
+ !explain.toLowerCase.contains("compiledtoobig"),
+ s"native compilation must not run for the full uncounted-capture
reproducer, " +
Review Comment:
Thanks — fixed in
https://github.com/apache/datafusion-comet/pull/5415/commits/bb15f0184f771e02ebaa30e143840f9c8a4e3d34.
I removed the redundant `s` prefix from the non-interpolated string and
retained it on the following string that interpolates `$explain`.
--
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]
