andygrove opened a new issue, #6756:
URL: https://github.com/apache/datafusion-comet/issues/6756
### Describe the bug
When the JVM codegen dispatcher takes an expression, it binds and
closure-serializes the whole tree under it and runs every node in one kernel.
`CometScalaUDF.emitJvmCodegenDispatch` calls `exprToProtoInternal` only on the
attribute references and on the serialized payload literal, so no other node in
the tree goes through the `spark.comet.expression.<name>.enabled` check.
Disabling an expression therefore has no effect when it appears inside a
dispatched tree, for example as the argument of a Scala UDF. The projection
stays in Comet, the disabled expression runs in the kernel, and no fallback
reason is recorded.
The user guide says the flag "disables Comet's serde for that expression and
forces a Spark fallback" (`compatibility/regex.md`), and `expressions.md`
describes it as the way to disable an expression.
### Steps to reproduce
On main (b56349697b) with Spark 4.1, over a Parquet table `t (a BIGINT, s
STRING)`:
```scala
spark.udf.register(
"plusOneBoxed",
(x: java.lang.Long) => if (x == null) null else java.lang.Long.valueOf(x +
1))
spark.udf.register("shout", (s: String) => if (s == null) null else
s.toUpperCase)
```
| Disabled | Query | Plan |
| --- | --- | --- |
| `Multiply` | `SELECT a * 2 FROM t` | Spark `Project`, with the fallback
reason `Expression support is disabled. Set
spark.comet.expression.Multiply.enabled=true to enable it.` |
| `Multiply` | `SELECT plusOneBoxed(a * 2) FROM t` | `CometProject` with
`multiply` and `plusoneboxed` dispatched, and no fallback reason |
| `RegExpReplace` | `SELECT shout(regexp_replace(s, 'a', 'b')) FROM t` |
`CometProject` with `regexp_replace` and `shout` dispatched, and no fallback
reason |
The results match Spark in every case. Only the config has no effect.
### Expected behavior
Either the dispatcher declines a tree that contains a disabled expression,
so the operator falls back and records the reason as it does for a top-level
expression, or the docs say that the flag controls the native serde only and
does not apply to expressions inside a dispatched tree.
### Additional context
@mbutrovich found this while reviewing #6715
(https://github.com/apache/datafusion-comet/pull/6715#discussion_r4197956212)
and reproduced it on branch-1.1 at e9efd9f764. #6715 checks the flag for the
nodes of a null guard before it dispatches the guard with its UDF, but leaves
the UDF's arguments unchecked, as they already were. Reading the code, the same
gap applies to the tree under any dispatched expression, not only a Scala UDF's
arguments: the `CometCodegenDispatch` serdes and the `dispatchIfFallback` path
in `QueryPlanSerde` use the same `emitJvmCodegenDispatch`.
--
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]
