schenksj opened a new issue, #7478:
URL: https://github.com/apache/datafusion-comet/issues/7478
### Describe the bug
On Spark 4.1, setting `spark.comet.expression.ParseUrl.enabled=false`,
`spark.comet.expression.StructsToJson.enabled=false` or
`spark.comet.expression.LengthOfJsonArray.enabled=false` does not make the
projection fall back to
Spark. The projection is still converted to `CometProject`. The regex
compatibility page describes
this per-expression switch as one that "disables Comet's serde for that
expression and forces a
Spark fallback".
The results are correct. The problem is only that the switch has no effect.
### Steps to reproduce
Run on `main` (`0f7f8e453`) with the default profile (Spark 4.1.3), in a
`CometTestBase` suite:
```scala
sql(
"SELECT * FROM VALUES ('http://spark.apache.org/path?q=1', 1, '[1,2,3]'),
" +
"('https://a.b/c', 2, '[]') AS v(s, x, j)")
.coalesce(1).write.parquet(path)
spark.read.parquet(path).createOrReplaceTempView("u")
withSQLConf("spark.comet.expression.ParseUrl.enabled" -> "false") {
sql("SELECT parse_url(s, 'HOST') FROM u") // CometProject
}
withSQLConf("spark.comet.expression.StructsToJson.enabled" -> "false") {
sql("SELECT to_json(named_struct('a', x)) FROM u") // CometProject
}
withSQLConf("spark.comet.expression.LengthOfJsonArray.enabled" -> "false") {
sql("SELECT json_array_length(j) FROM u") // CometProject
}
```
In each case the executed plan's projection is `CometProject`, and the
results are
`[a.b], [spark.apache.org]`, `[{"a":1}], [{"a":2}]` and `[0], [3]`.
Also disabling the generic serde that the call is lowered to makes the
projection fall back to
Spark:
- `ParseUrl.enabled=false` plus `spark.comet.expression.Invoke.enabled=false`
- `StructsToJson.enabled=false` plus
`spark.comet.expression.Invoke.enabled=false`
- `LengthOfJsonArray.enabled=false` plus
`spark.comet.expression.StaticInvoke.enabled=false`
### Cause
On Spark 4.x these three expressions are `RuntimeReplaceable`, and Comet
sees them as `Invoke`
(`parse_url`, `to_json`) or `StaticInvoke` (`json_array_length`) calls.
`sparkVersionSpecificExprToProtoInternal` in
`spark/src/main/spark-4.x/org/apache/comet/shims/Spark4xCometExprShim.scala`
rebuilds the original
expression and converts it. When the switch is off, that conversion returns
`None`. The shim
lifts the fallback reason onto the `Invoke` / `StaticInvoke` and returns
`None`.
`QueryPlanSerde.exprToProtoInternal` treats a `None` from the shim as "no
version-specific
handling", and continues with `.orElse` to the generic `Invoke` /
`StaticInvoke` serde. That serde
converts the call, so the projection is converted after all. This is why
also disabling `Invoke` /
`StaticInvoke`, as shown above, restores the fallback.
### Expected behavior
With `spark.comet.expression.<Name>.enabled=false`, the projection falls
back to Spark, with the
"disabled" fallback reason.
### Additional context
A possible fix: when the shim matches a lowered call and the conversion of
the rebuilt expression
fails, stop the conversion of the `Invoke` / `StaticInvoke` rather than
continuing to the generic
serde. A regression test for each of the three expressions could assert the
Spark fallback with
the switch off.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
