Skip to content

fix: route translate through codegen dispatcher by default #5606

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
adibmbrk wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: route translate through codegen dispatcher by default #5606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/source/user-guide/latest/expressions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,7 @@ The type-name conversion functions (`bigint`, `binary`, `boolean`, `date`, `deci
| `to_char` | ✅ | Codegen dispatch | |
| `to_number` | ✅ | Codegen dispatch | |
| `to_varchar` | ✅ | Codegen dispatch | |
| `translate` | ✅ | Native | DataFusion's `translate` iterates over Unicode graphemes (Spark uses code points) and substitutes U+0000 instead of treating it as a deletion sentinel, so the native path is opt-in via allowIncompatible |
| `translate` | ✅ | Hybrid | Routes through the JVM codegen dispatcher by default; the native path (which iterates Unicode graphemes rather than code points and substitutes U+0000 instead of a deletion sentinel) is opt-in via allowIncompatible |
| `trim` | ✅ | Native | |
| `try_to_binary` | ✅ | — | Runs natively (rewrites to `try_eval(to_binary(...))`) |
| `try_to_number` | ✅ | Codegen dispatch | Routed through the JVM codegen dispatcher |
Expand Down
28 changes: 25 additions & 3 deletions spark/src/main/scala/org/apache/comet/serde/strings.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 =

@sunchao sunchao Sep 1, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Some(NativeOptIn(CometConf.getExprAllowIncompatConfigKey(getExprConfigName(expr)))))
} else {
Compatible()
}

override def convert(
expr: StringTranslate,
inputs: Seq[Attribute],
binding: Boolean): Option[Expr] = {
if (CometConf.isExprAllowIncompat(getExprConfigName(expr))) {
// Native path: faster but iterates Unicode graphemes and substitutes U+0000 rather than
// treating it as a deletion sentinel, so it is only used when incompatibility is allowed.
super.convert(expr, inputs, binding)
} else {
// Default: run Spark's own generated code inside the Comet pipeline for exact
// compatibility. Falls back to Spark when the codegen dispatcher is disabled.
CometScalaUDF.emitJvmCodegenDispatch(expr, inputs, binding)
}
}
}

object CometLevenshtein extends CometExpressionSerde[Levenshtein] {
Expand Down