apache / apache/arrow-rs

cast() uses a per-element checked conversion for numeric casts that cannot fail

Open
#10,955 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
3.6k
Forks
1.3k
Avg merge
2d 18h
Merged PRs (30d)
169

Description

> 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.as_primitive::()))) // unary_opt
} else {
Ok(Arc::new(try_numeric_cast::(from.as_primitive::())?)) // 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: &dyn Array, cast_options: &CastOptions)
-> Result
where
FROM: ArrowPrimitiveType,
TO: ArrowPrimitiveType,
FROM::Native: NumCast + AsPrimitive,
TO::Native: NumCast,
{
if is_infallible_numeric_cast(&FROM::DATA_TYPE, &TO::DATA_TYPE) {
return Ok(Arc::new(
from.as_primitive::().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::` 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::(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::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::(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.

Contributor guide

Open the contributing guide

Research direction

Start at arrow-cast/src/cast/mod.rs:2506 and inspect cast_numeric_arrays, then review arrow/benches/cast_kernels.rs and the existing arrow-cast test suite. Done means the reported total numeric pairs avoid unnecessary checked conversion while narrowing casts retain their checks, with direct numeric coverage for the proposed pairs and a negative control.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
data-engineering, performance
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
70/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.