alexeigor opened a new issue, #10955: URL: https://github.com/apache/arrow-rs/issues/10955
> Re-filed from my primary account; supersedes [#10953](https://github.com/apache/arrow-rs/issues/10953), which I closed unanswered. Content is unchanged. ## Is your feature request related to a problem or challenge? All 110 numeric↔numeric arms of `cast_with_options` route through `cast_numeric_arrays` (`arrow-cast/src/cast/mod.rs:2506`), which performs a per-element checked `num_traits::cast` — including for conversions that are total, such as `Int32 → Int64`. Neither `CastOptions::safe` setting avoids this: ```rust if cast_options.safe { Ok(Arc::new(numeric_cast::<FROM, TO>(from.as_primitive::<FROM>()))) // unary_opt } else { Ok(Arc::new(try_numeric_cast::<FROM, TO>(from.as_primitive::<FROM>())?)) // try_unary } ``` * `safe: true` → `unary_opt`, which allocates and **rebuilds a new validity bitmap** from each element's `Option`. For a widening cast the result is provably identical to the input's bitmap. * `safe: false` → `try_unary`, which clones the null buffer but still calls a fallible `op` per element and iterates through `nulls.try_for_each_valid_idx` — an index-by-index walk rather than a linear scan. For `Int32 → Int64` the range check can never fire, the rebuilt bitmap is redundant, and the per-element `Option`/`Result` prevents the compiler from emitting a widening SIMD instruction. The docs on the adjacent `try_unary_mut` already state the consequence: > Note: LLVM is currently unable to effectively vectorize fallible operations ### Measurements `i32 → i64`, Criterion medians. Apple M5 Pro, rustc 1.98.0 stable, `lto = "fat"`, `codegen-units = 1`, mimalloc. | n | nulls | `cast` (safe) | `cast` (safe=false) | `unary(\|v\| v as i64)` | gap | |---|---|---|---|---|---| | 1 000 | 0% | 0.205 µs | 0.202 µs | **0.101 µs** | 2.0× | | 100 000 | 0% | 10.8 µs | 11.7 µs | **9.07 µs** | 1.2× | | 10 000 000 | 0% | 1263.9 µs | 1236.0 µs | **940.9 µs** | 1.3× | | 1 000 | 10% | 0.563 µs | 0.534 µs | **0.100 µs** | 5.3× | | 100 000 | 10% | 53.4 µs | 52.1 µs | **8.57 µs** | 6.1× | | 10 000 000 | 10% | 5353.9 µs | 5329.5 µs | **934.3 µs** | 5.7× | The cost is concentrated in the null path: stock `cast` gets **4.3× slower** once a column has any nulls (1236 → 5330 µs at 10M), while the `unary` version is flat (941 → 934 µs). The ratio holds from n=1 000 to n=10 000 000, including at n=8 192. ### Downstream impact DataFusion 55.0.0 pins arrow 59.3.0 — the version measured above — so DataFusion can be built against the modified `arrow-cast` and the same query run both ways. The substitution was confirmed present in the resolved dependency graph before measuring. DataFusion's `sum` signature coerces `Int8/Int16/Int32 → Int64` explicitly, so `SUM(int32_col)` casts the whole column. Two queries over identical data (1220 × 8192 = 9,994,240 rows, 10% nulls, `target_partitions = 1`, DataFusion's default 8192-row batches). The only difference between them is the cast: | query | stock | patched | speedup | |---|---|---|---| | `SELECT SUM(c_i32) FROM t` | 7.426 ms | **3.349 ms** | **2.22×** | | `SELECT SUM(c_i64) FROM t` *(control)* | 2.097 ms | 2.205 ms | 0.95× | The cast component drops from **5.329 ms to 1.144 ms (4.66×)**, taking it from **72% to 34%** of query time. The unchanged control shows nothing else moved. Both arms were run back to back in one session, using exactly the predicate shown below. An earlier run of a wider predicate (one that also covered `Float16` targets) gave 2.24× / 4.60× — the same result within noise, as expected, since those arms are not reachable from an `Int32 → Int64` cast. DataFusion reaches this path from more than `SUM`: comparison against an `Int64` literal, join keys of differing width, `UNION` branch unification, `CASE`/`IN` result types, and schema unification across files with differing types. ## Describe the solution you'd like A fast path in `cast_numeric_arrays` for conversions where `num_cast` is total. `num_traits::cast::AsPrimitive` is already imported in the module, and since all 110 call sites use concrete types, adding the bound needs no call-site changes: ```rust fn cast_numeric_arrays<FROM, TO>(from: &dyn Array, cast_options: &CastOptions) -> Result<ArrayRef, ArrowError> where FROM: ArrowPrimitiveType, TO: ArrowPrimitiveType, FROM::Native: NumCast + AsPrimitive<TO::Native>, TO::Native: NumCast, { if is_infallible_numeric_cast(&FROM::DATA_TYPE, &TO::DATA_TYPE) { return Ok(Arc::new( from.as_primitive::<FROM>().unary::<_, TO>(|v| v.as_()), )); } // ... existing safe / try paths unchanged } ``` `unary` clones the input `NullBuffer` (a refcount bump — no scan, no allocation) and maps the closure over the values, which vectorizes. ### The predicate ```rust /// True when `num_cast::<FROM, TO>` is total — it can never return `None`, so the /// per-element check can never fire and the conversion is a plain `as`. fn is_infallible_numeric_cast(from: &DataType, to: &DataType) -> bool { use DataType::*; matches!( (from, to), (Int8, Int16 | Int32 | Int64 | Float32 | Float64) | (Int16, Int32 | Int64 | Float32 | Float64) | (Int32, Int64 | Float32 | Float64) | (Int64, Float32 | Float64) | (UInt8, UInt16 | UInt32 | UInt64 | Int16 | Int32 | Int64 | Float32 | Float64) | (UInt16, UInt32 | UInt64 | Int32 | Int64 | Float32 | Float64) | (UInt32, UInt64 | Int64 | Float32 | Float64) | (UInt64, Float32 | Float64) | (Float32, Float64) ) } ``` **It costs nothing at runtime.** It is called with `FROM::DATA_TYPE` / `TO::DATA_TYPE`, which are associated constants rather than values read from the array. `cast_numeric_arrays` is monomorphised per concrete pair, so the `Int32 → Int64` instantiation sees `is_infallible_numeric_cast(&Int32, &Int64)`, folds it to `true`, and drops the branch and the checked path entirely. It reads like a runtime match; it is not one. **Why each group is total:** | group | reasoning | |---|---| | integer widening within a signedness (`i8→i16→i32→i64`, `u8→u16→u32→u64`) | the target range strictly contains the source range | | unsigned → strictly wider signed (`u8→i16`, `u16→i32`, `u32→i64`) | `i64` has 63 magnitude bits, so every `u32` fits. Note `u32→i32` is absent: same width, so it can overflow | | integer → float | `num_cast` never fails here, it rounds. Lossy above the mantissa width, but `as` rounds *identically*, so the substitution is value-preserving. Totality is the property that matters, not exactness | | `f32 → f64` | every `f32` is exactly representable, NaN and infinities included | **Every narrowing conversion is excluded**, where the check is load-bearing. **The predicate is fail-safe by construction.** `matches!` yields `false` for anything unlisted, and `false` means "use the existing checked path". An omission therefore costs performance, never correctness. A wrong *addition* would be a correctness bug, which is what the negative control below guards. **On `Float16`:** the version I benchmarked also included `Float16` targets. They are omitted above because the correctness test cannot cover them — `v as f16` is not a primitive cast, so the direct numeric proof does not apply, and their safety would rest on `half::f16`'s `NumCast` impl agreeing with saturation on values like `i64::MAX`. That is plausible but unproven, and the set above is already sufficient to produce the DataFusion result. `Float16` can be added later with a purpose-built test. ### Correctness The substitution is safe iff `num_cast::<FROM, TO>(v) == Some(v as TO)` for every value of `FROM`. That is a claim about the numbers, not about arrow, so it is worth testing directly rather than by comparing two arrow code paths: ```rust for v in samples!($from) { // MIN, MAX, 0, 1 + a 65-point spread assert_eq!(num_cast::<$from, $to>(v), Some(v as $to)); } ``` This passes for every pair in the proposed set, with a negative control asserting that the narrowing pairs genuinely are *not* infallible (`num_cast::<i64, i32>(i64::MAX)` is `None`, whereas `i64::MAX as i32` is `-1` — which is exactly why those keep the check). The patched build also passes all **360 existing tests in `arrow-cast`**. I am happy to open a PR if the approach looks right. ## Describe alternatives you've considered * **`CastOptions { safe: false }`** — does not help. It swaps `unary_opt` for `try_unary`, keeping the per-element check; measured within 2% of the safe path. DataFusion already uses `safe: false` throughout (`datafusion-physical-expr/src/expressions/cast.rs:37`) and so cannot opt out. * **Callers using `unary` directly** — works, and is what I used to obtain the numbers, but it is not reachable through the public `cast` API. It also pushes the "is this conversion infallible?" decision onto every caller, which is exactly the knowledge `arrow-cast` already has. * **Leaving it to downstream** — the two closest prior issues (#7097, #7055) were resolved in the Parquet reader rather than in `cast_numeric_arrays`, so this function has already been worked around once from another direction. * **Expressing infallibility in the type system** rather than as a matched pair — an associated `const INFALLIBLE: bool` on a `CastFrom`-style trait would make it a compile-time fact instead of relying on the match being folded, and would make an unhandled pair a compile error rather than a silent slow path. It is more invasive, and it duplicates knowledge `num_traits` already encodes, but if maintainers prefer that shape I am happy to write it that way. The matched-pair version above was chosen because it needs no new trait and no call-site changes. ## Additional context ### Prior art in other Arrow implementations * **polars-compute** encodes the distinction in its dispatch table: `primitive_to_primitive_dyn` branches on `options.wrapped`, choosing `primitive_as_primitive` (`unary` + `AsPrimitive::as_`) for 48 arms tagged `as_options`, and the checked `primitive_to_primitive` otherwise. * **Arrow C++** already skips the check for a widening cast: `CastOptions::Unsafe()` measured identical to the safe default (23.9 vs 23.6 µs at 100 k), i.e. the safe path was not doing extra work to begin with. ### Why the existing benchmarks do not surface this `arrow/benches/cast_kernels.rs` builds its numeric inputs with `build_array::<T>(512)`. `cast int32 to int64 512` exists, but 512 × 4 bytes = 2 KB measures dispatch rather than the per-element conversion. There is also no `unary` reference arm in the suite — grepping `unary|try_unary|unary_opt` across all 63 bench files in the workspace matches only `array_iter.rs`, and not as a kernel comparison. Two suggestions, independent of whether the fix lands: * raise the numeric cast sizes in `cast_kernels.rs` to at least 64 Ki and sweep null density — the null path is where the cost concentrates * add `unary` as a reference arm so the cost of the checked conversion is visible ### Related issues (searched; none appear to duplicate this) * **#1918 — "Replace checked casts with `as` for performance"** (closed 2022). Same words, different subject: `try_into().unwrap()` on offsets and indices in array-data internals, not `num_traits::cast` in the cast kernel. * **#7097 / #7055 — Parquet performance reading int8/int16** (closed). *Narrowing* casts (32 bits down to 8/16), where the range check is load-bearing. Opposite direction from this report. * **#10726 — "support unsafe casting with masked null values"** (open PR). Touches `safe: false` behaviour for string, list, struct and map casts; does not modify `cast_numeric_arrays`. * **#9789 — "Remove redundant benchmarks in `cast_kernels`"** (merged). Pruned redundant decimal cases to cut benchmark runtime; noted only because it bears on the benchmark suggestions above. ### Environment * arrow-rs 59.3.0, rustc 1.98.0 stable, `aarch64-apple-darwin` * Apple M5 Pro (6P + 12E), 48 GB, macOS Darwin 25.6.0 * Criterion 1 s warm-up / 3 s measurement; the DataFusion arm built with `CARGO_PROFILE_BENCH_LTO=thin`, applied identically to both sides Numbers come from a laptop with no core pinning, so absolute values are directional; the ratios reproduced across two independent full runs. -- 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]
