sam-1112 opened a new pull request, #6680:
URL: https://github.com/apache/datafusion-comet/pull/6680
<html><head></head><body><h2>Which issue does this PR close?</h2>
<p>Addresses #5354 for a bounded set of safe inputs.</p>
## Rationale for this change
`replace` currently uses Spark's JVM codegen dispatcher by default,
including common inputs where the native DataFusion implementation is
compatible and faster. This change enables native execution for those inputs
while retaining the dispatcher for cases with compatibility, broadcast size, or
performance concerns.
## What changes are included in this PR?
- Enable native `replace` by default for UTF8_BINARY inputs with a direct
source column and a non-empty, valid UTF-8 search literal:
- With a replacement literal, both literals must be at most 64 KiB.
- With a replacement column, the search must be at most 256 bytes and its
broadcast must fit within 8 MiB per configured batch.
- Retain the JVM codegen dispatcher for other input shapes, empty searches,
malformed UTF-8 literals, non-default collations, and oversized literals or
broadcasts.
- Preserve the existing
`spark.comet.expression.StringReplace.allowIncompatible` opt-in.
- Add correctness and EXPLAIN routing assertions covering:
- Literal and column replacements, nulls, empty sources, overlapping
matches, and multibyte UTF-8.
- Search and replacement literals at 64 KiB and 64 KiB + 1 byte, including
an exact match against a 64 KiB source.
- Replacement-column search literals at 256 and 257 bytes.
- A 32,769-row Parquet input with configured batch sizes of 32,768 and
32,769, verifying native routing at exactly 8 MiB and dispatcher routing above
it.
- Extend `CometStringExpressionBenchmark` with multiple match densities and
search literal lengths.
- Update the user guide and expression audit with the supported subset,
benchmark results, and limit rationale.
## How are these changes tested?
Local reports show all six tests selected by the replace filter and the
`string_replace.sql` fixture passing. The routing tests compare results against
Spark and assert whether `replace` appears in the native or dispatcher EXPLAIN
expression list.
```shell
./mvnw test -Dtest=none -Dsuites='org.apache.comet.CometCodegenSuite
replace' -Pspark-4.1 -nsu
./mvnw test -Dtest=none -Dsuites='org.apache.comet.CometSqlFileTestSuite
string_replace' -Pspark-4.1 -nsu
```
`git diff --check` passes. Spark SQL suite validation is pending through the
`run-spark-4.1-tests` label.
### Benchmark setup
Benchmarks used Apple M4, macOS 26.7.1, JDK 21.0.12, Spark 4.1, a release
native build, and 65,536 rows per case. Dispatcher and native runs used the
same queries and input generation, with only the default routing changed.
Timings include scan and projection.
### Literal replacement
| Match density | Dispatcher ns/row | Native ns/row | Native speedup |
| --- | ---: | ---: | ---: |
| 0% | 207.5 | 149.5 | 1.39× |
| ~10% | 277.5 | 141.8 | 1.96× |
| 100% | 309.9 | 170.5 | 1.82× |
### Replacement column: short search literal
| Match density | Dispatcher ns/row | Native ns/row | Native speedup |
| --- | ---: | ---: | ---: |
| 0% | 184.4 | 149.9 | 1.23× |
| ~10% | 243.3 | 136.9 | 1.78× |
| 100% | 281.5 | 164.9 | 1.71× |
### Replacement column: longer search literals
Source strings are 2,048 bytes long.
| Search bytes | Match density | Dispatcher ns/row | Native ns/row | Native
speedup |
| ---: | --- | ---: | ---: | ---: |
| 16 | 0% | 1,734.6 | 723.6 | 2.40× |
| 16 | ~10% | 3,071.5 | 715.8 | 4.29× |
| 16 | 100% | 2,667.7 | 715.1 | 3.73× |
| 256 | 0% | 2,823.3 | 1,270.9 | 2.22× |
| 256 | ~10% | 2,802.2 | 1,280.0 | 2.19× |
| 256 | 100% | 2,234.7 | 1,464.3 | 1.53× |
| 1,024 | 0% | 1,847.0 | 3,239.8 | 0.57× |
| 1,024 | ~10% | 1,808.6 | 3,287.6 | 0.55× |
| 1,024 | 100% | 1,195.0 | 4,004.1 | 0.30× |
The 1,024-byte native measurements used a temporary routing override. These
regressions motivated the 256-byte limit; the submitted implementation keeps
those inputs on the dispatcher by default.
Each route was measured in a separate benchmark invocation. Treat the
results as directional rather than statistically conclusive.
--
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]
