sam-1112 opened a new pull request, #6653:
URL: https://github.com/apache/datafusion-comet/pull/6653
## Which issue does this PR close?
Closes part of #5354 (enable native `replace` for safe inputs, add routing
and correctness tests, and benchmark short and long search literals).
## Rationale for this change
`replace` currently uses Spark’s JVM codegen dispatcher by default, even for
inputs where Comet’s native implementation is safe and faster. This change
enables native execution for a bounded set of compatible inputs while keeping
dispatcher fallback for cases that can differ from Spark or perform poorly.
## What changes are included in this PR?
- Enable native `replace` by default for safe input shapes:
- Literal search and replacement on a direct string column, with valid
UTF-8 literals up to 64 KiB.
- Literal search with a replacement column, with a search literal up to
256 bytes and a broadcast size capped at 8 MiB per configured batch.
- Keep other input shapes, empty searches, malformed UTF-8 literals,
non-default collations, and oversized broadcasts on the JVM codegen dispatcher
by default. The existing `allowIncompatible` opt-in remains available.
- Add tests that compare results with Spark and assert whether `replace`
appears in the Native or dispatcher EXPLAIN segment.
- Extend the string expression benchmark with multiple match densities and
search literal lengths. Record results and the 256-byte limit rationale in the
expression audit.
## How are these changes tested?
- Spark 3.5: `./mvnw test -Dtest=none
-Dsuites='org.apache.comet.CometCodegenSuite replace' -Pspark-3.5 -nsu`
- Spark 4.1: `./mvnw test -Dtest=none
-Dsuites='org.apache.comet.CometCodegenSuite replace' -Pspark-4.1 -nsu`
- Benchmarks ran on Apple M4, macOS 26.7.1, JDK 21.0.12, Spark 4.1 release
build, with 65,536 rows per case. Timings include scan and projection.
### Replacement column: short search literal
| Match density | Dispatcher ns/row | Native ns/row | Dispatcher M rows/s |
Native M rows/s | Native speedup |
|---|---:|---:|---:|---:|---:|
| 0% | 184\.4 | 149\.9 | 5\.4 | 6\.7 | 1\.23× |
| \~10% | 243\.3 | 136\.9 | 4\.1 | 7\.3 | 1\.78× |
| 100% | 281\.5 | 164\.9 | 3\.6 | 6\.1 | 1\.71× |
### Replacement column: longer search literals
| Search bytes | Match density | Dispatcher ns/row | Native ns/row |
Dispatcher M rows/s | Native M rows/s | Native speedup |
|---:|---|---:|---:|---:|---:|---:|
| 16 | 0% | 1,734.6 | 723\.6 | 0\.6 | 1\.4 | 2\.40× |
| 16 | \~10% | 3,071.5 | 715\.8 | 0\.3 | 1\.4 | 4\.29× |
| 16 | 100% | 2,667.7 | 715\.1 | 0\.4 | 1\.4 | 3\.73× |
| 256 | 0% | 2,823.3 | 1,270.9 | 0\.4 | 0\.8 | 2\.22× |
| 256 | \~10% | 2,802.2 | 1,280.0 | 0\.4 | 0\.8 | 2\.19× |
| 256 | 100% | 2,234.7 | 1,464.3 | 0\.4 | 0\.7 | 1\.53× |
| 1,024 | 0% | 1,847.0 | 3,239.8 | 0\.5 | 0\.3 | 0\.57× |
| 1,024 | \~10% | 1,808.6 | 3,287.6 | 0\.6 | 0\.3 | 0\.55× |
| 1,024 | 100% | 1,195.0 | 4,004.1 | 0\.8 | 0\.2 | 0\.30× |
Each route was measured in a separate single run; treat the numbers as
directional rather than statistically conclusive. The 1,024-byte results
motivated the 256-byte default limit for replacement-column routing.
--
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]
