0lai0 opened a new pull request, #5614:
URL: https://github.com/apache/datafusion-comet/pull/5614
## Which issue does this PR close?
Closes #5349.
## Rationale for this change
Integral `sequence(start, stop[, step])` currently runs through the JVM
codegen dispatcher, which allocates two `long[]` per row. This PR adds a native
kernel that reserves the Arrow child buffer once per batch. Date and timestamp
sequences stay on the dispatcher (timezone / DST / legacy calendar).
## What changes are included in this PR?
- Native `spark_sequence` kernel
(`native/spark-expr/src/array_funcs/sequence.rs`) for `Byte` / `Short` / `Int`
/ `Long`. Matches Spark `Sequence.sequenceLength`, including the
overflow-report paths (`2^63`, `2^63+1`, and the internal-error edge).
- `CometSequence` with `CodegenDispatchFallback`: `Compatible()` for
integral types, `Unsupported` for date/timestamp so they stay on the codegen
dispatcher.
- Error mapping via `ShimSparkErrorConverter`: Spark 3.x throws
`IllegalArgumentException("Illegal sequence boundaries: ...")`; Spark 4.x
throws `SparkIllegalArgumentException("_LEGACY_ERROR_TEMP_3243")`.
- `CollectionSizeLimitExceeded` now carries a decimal `String` count (can
exceed `i64`) and a `function_name` for Spark 4.x. This is the first producer
of that error, and it also fixes a latent Spark 3.5 shim bug that passed a
Scala tuple as `count` and rendered `(array,N)`.
- Docs: `sequence` marked `Hybrid` in `expressions.md`; audit notes added
under `array_funcs.md`.
## How are these changes tested?
- Five unit tests in `sequence.rs`.
- `spark/src/test/resources/sql-tests/expressions/array/sequence.sql`:
integral types, default/explicit step, nulls, explode, seven error cases, plus
date/timestamp dispatcher coverage.
- `CometCodegenSuite`: integral sequences are native; date sequences still
show "JVM codegen dispatcher".
**Criterion** (`cargo bench --bench sequence`, N=8192). Absolute numbers;
the kernel is not on `main`.
| Shape | Time | Elems/batch | ns / elem |
|------------------------------|--------------|-------------|-----------|
| `short_2_elems` | 49.83 µs | 16,384 | 3.04 |
| `short_5_elems` | 55.35 µs | 40,960 | 1.35 |
| `long_365_elems` | 1.318 ms | 2,990,080 | 0.44 |
| `long_10000_elems` | 34.87 ms | 81,920,000 | 0.43 |
| `descending_365_elems` | 1.324 ms | 2,990,080 | 0.44 |
| `zero_step_start_eq_stop` | 35.70 µs | 8,192 | 4.36 |
| `sparse_nulls_365_elems` | 1.200 ms | 2,691,072 | 0.45 |
| `dense_nulls_365_elems` | 673.2 µs | 1,495,040 | 0.45 |
| `error_illegal_boundaries` | 673 ns | (errors) | - |
**Spark** (`CometSequenceBenchmark`, 8192 rows, Apple M5, Spark 4.1.3 /
Scala 2.13)
| Shape | Spark best (ms) | Comet best (ms) |
speedup |
|--------------------------------|-----------------|-----------------|---------|
| `seq_short_5_elems` | 15 | 5 | 2.9X
|
| `seq_spine_365_elems` | 14 | 6 | 2.2X
|
| `seq_long_10000_elems` | 59 | 57 | 1.0X
|
| `seq_descending_default_step` | 13 | 6 | 2.3X
|
| `seq_explicit_step_7` | 11 | 3 | 3.2X
|
| `seq_sparse_nulls_365_elems` | 12 | 5 | 2.4X
|
| `seq_date_spine_dispatcher` | 12 | 12 | 1.0X
|
`seq_date_spine_dispatcher` is a control (date path unchanged).
`seq_long_10000_elems` is memory-bandwidth-bound at 82M elements/batch. Typical
speedup is 2X–3X on the shorter-list shapes the issue targets.
--
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]
