You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "adibmbrk (via GitHub)" <gi...@apache.org> on 2026/09/01 01:26:04 UTC
[PR] fix: route translate through codegen dispatcher by default [datafusion-comet]
adibmbrk opened a new pull request, #5606:
URL: https://github.com/apache/datafusion-comet/pull/5606
## Which issue does this PR close?
Closes #5585.
## Rationale for this change
`CometStringTranslate` reported itself as `Incompatible` unconditionally, forcing whole projections to fall back to Spark unless `spark.comet.expression.StringTranslate.allowIncompatible=true` was set. Comparable string functions (`CometInitCap`, `CometStringReplace`) instead run natively by default through the JVM codegen dispatcher and treat their semantic differences as an opt-in caveat.
## What changes are included in this PR?
- `CometStringTranslate` mixes in `NativeOptInAvailable`: default is `Compatible` and routes through the codegen dispatcher (Spark-compatible); the incompatible native DataFusion path is opt-in via `allowIncompatible`.
- Docs: `translate` updated from `Native` to `Hybrid`.
## How are these changes tested?
Ran the affected suites against Spark 4.1 (`-Dtest=none -Dsuites=...`):
- `CometFallbackInvarianceSuite` — `pass=25 fail=0 excused=0 vacuous=0` (translate is now a real native-vs-fallback parity pass rather than the previous SKIPPED-VACUOUS case).
- `CometStringExpressionSuite` — 39 tests succeeded, 0 failed.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: route translate through codegen dispatcher by default [datafusion-comet]
Posted by "adibmbrk (via GitHub)" <gi...@apache.org>.
adibmbrk commented on PR #5606:
URL: https://github.com/apache/datafusion-comet/pull/5606#issuecomment-5702705132
Closing as superseded by #5032, which landed the same change on `main`
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: route translate through codegen dispatcher by default [datafusion-comet]
Posted by "sunchao (via GitHub)" <gi...@apache.org>.
sunchao commented on code in PR #5606:
URL: https://github.com/apache/datafusion-comet/pull/5606#discussion_r3901095367
##########
spark/src/main/scala/org/apache/comet/serde/strings.scala:
##########
@@ -109,15 +109,37 @@ object CometOctetLength extends CometScalarFunction[OctetLength]("octet_length")
}
}
-object CometStringTranslate extends CometScalarFunction[StringTranslate]("translate") {
+object CometStringTranslate
+ extends CometScalarFunction[StringTranslate]("translate")
+ with NativeOptInAvailable {
private val incompatReason =
"DataFusion's translate iterates over Unicode graphemes (Spark uses code points) and" +
" substitutes U+0000 instead of treating it as a deletion sentinel"
override def getIncompatibleReasons(): Seq[String] = Seq(incompatReason)
- override def getSupportLevel(expr: StringTranslate): SupportLevel = Incompatible(
- Some(incompatReason))
+ override def getSupportLevel(expr: StringTranslate): SupportLevel =
+ if (!CometConf.isExprAllowIncompat(getExprConfigName(expr))) {
+ Compatible(nativeOptIn =
Review Comment:
[P2] Update the default translate SQL fixture
Could you update the default cases in `string_translate.sql` along with this change? It still has four `expect_fallback(is not fully compatible with Spark)` blocks. The SQL-file suite registers these blocks and requires that fallback contract, but this `Compatible` branch now accepts the first ordinary three-string-column query and dispatches it by default. The successful new route therefore cannot satisfy the old fixture expectation, even when its result values are correct. Please check Comet execution and result parity for the default cases, and keep any dispatcher-disabled fallback case separate with its actual reason. This is source-derived, not an observed test failure.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: route translate through codegen dispatcher by default [datafusion-comet]
Posted by "adibmbrk (via GitHub)" <gi...@apache.org>.
adibmbrk closed pull request #5606: fix: route translate through codegen dispatcher by default
URL: https://github.com/apache/datafusion-comet/pull/5606
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org