Consolidate ScalarValue cast implementations (towards a non-copying cast_to)
- Dominant language
- Rust
- Stars
- 9.3k
- Forks
- 2.4k
- Avg merge
- 3d 7h
- Merged PRs (30d)
- 344
Description
## Is your feature request related to a problem or challenge?
DataFusion currently has (at least) three overlapping implementations of "cast a scalar / literal to a target type", with different contracts and duplicated per-type logic:
1. **`ScalarValue::cast_to` / `cast_to_with_options`** — [`datafusion/common/src/scalar/mod.rs`](https://github.com/apache/datafusion/blob/0add0469eb87dbb7fd9b6f78ded41ca7c8b46b6f/datafusion/common/src/scalar/mod.rs#L4216-L4221). (Recently gained array-free fast paths for identity and string↔string casts in #22576.)
2. **`try_cast_literal_to_type`** — [`datafusion/expr-common/src/casts.rs`](https://github.com/apache/datafusion/blob/0add0469eb87dbb7fd9b6f78ded41ca7c8b46b6f/datafusion/expr-common/src/casts.rs#L35). An array-free, hand-rolled cast used by the unwrap-cast optimizations. Returns `Option`, with its own per-type helpers : it only performs value-preserving casts and returns `None` for out-of-range numeric, precision-losing decimal, lossy date↔timestamp, timestamp→string, and string→numeric conversions. These restrictions are load-bearing for optimizer correctness (you may only unwrap `CAST(col AS T) = lit` when `lit` converts back to its original type exactly).
3. **`cast_literal_to_type_with_op`** — [`datafusion/optimizer/src/simplify_expressions/unwrap_cast.rs`](https://github.com/apache/datafusion/blob/0add0469eb87dbb7fd9b6f78ded41ca7c8b46b6f/datafusion/optimizer/src/simplify_expressions/unwrap_cast.rs#L198). Yet another special case (for `Utf8 = Int`-style comparisons), implemented via `cast_to` plus a manual round-trip check.
Because the logic is duplicated, casting behavior and optimizations have to be implemented in multiple places, and the implementations can (and do) diverge in subtle, correctness-relevant ways.
## Describe the solution you'd like
Consolidate the cast implementations so there is **a single place to implement and optimize scalar casting**.
My goal is to be able to add a **non-copying** version of `cast_to`: today `cast_to(&self)` forces a clone/allocation even on the fast paths, and `try_cast_literal_to_type` re-allocates strings via `to_string()`.
**First step (this issue): consolidate the implementations, without changing behavior.**
- Make `ScalarValue::cast_to` the single canonical scalar-cast implementation
- Re-express `try_cast_literal_to_type` in terms of `cast_to`,
- Fold `cast_literal_to_type_with_op` into the same path where possible.
Once consolidated, a follow-up can add the non-copying cast variant and optimize it in that one place if possible
## Describe alternatives you've considered
- Adding an owned / non-copying API to `try_cast_literal_to_type` directly (explored in #22574). This works, but optimizes only one of the three implementations and leaves the duplication in place.
## Additional context
- Background: review discussion on #22562 (LIKE `'prefix%'` pruning), which surfaced a redundant string allocation in the literal-cast path.
- #22576 added the identity / string↔string array-free fast paths to `cast_to`, and made the `cast_round_trip` test cross-check `cast_to` against the arrow cast kernel.
Contributor guide
Research direction
Start by reading ScalarValue::cast_to in datafusion/common/src/scalar/mod.rs, then compare try_cast_literal_to_type in datafusion/expr-common/src/casts.rs and cast_literal_to_type_with_op in datafusion/optimizer/src/simplify_expressions/unwrap_cast.rs. Use the cast_round_trip test and existing cast behavior as references. Done means the implementations share the canonical cast path without changing value-preserving and optimizer-related behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend, data
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100