You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "coderfender (via GitHub)" <gi...@apache.org> on 2026/02/09 19:56:53 UTC
[I] RFC [datafusion-comet]
coderfender opened a new issue, #3459:
URL: https://github.com/apache/datafusion-comet/issues/3459
### What is the problem the feature request solves?
# RFC: Cast Module Modularization
**Author:** Bhargava Vadlamani (@coderfender)
**Date:** 2026-02-09
**Status:** Draft
## Summary
Refactor the monolithic `cast.rs` (3,700 LOC) into separate focused modules organized by data type category, using a two-level static dispatch flow
## Problems
1. **Merge Conflicts** - Multiple contributors editing the same large file causes frequent conflicts
2. **No Enforced Contract** - No trait defining what a cast implementation must provide
3. **Poor Discoverability** - Hard to find specific cast logic in 3,700 lines
4. **Scattered Changes** - Adding a new cast type requires edits across distant parts of the file
## Proposal
### Trait Definition
```rust
/// Core trait for Spark-compatible cast operations.
///
/// # Future Optimization
/// TODO: Impl should branch on EvalMode at array level, not row level.
pub trait SparkCast: Send + Sync + Debug {
fn can_cast(&self, from: &DataType, to: &DataType) -> bool;
fn cast(
&self,
array: &ArrayRef,
from: &DataType,
to: &DataType,
opts: &SparkCastOptions,
) -> SparkResult<ArrayRef>;
}
```
### Module Structure
```
native/spark-expr/src/cast/
├── mod.rs # Public API, dispatcher
├── traits.rs # SparkCast trait
├── string.rs # String ↔ other types
├── numeric.rs # Integer, Float, Decimal
├── temporal.rs # Date, Timestamp
├── boolean.rs # Boolean conversions
├── complex_types.rs # Struct, Array, Map
├── binary.rs # Binary conversions
└── utils.rs # Shared helpers
```
### Two-Level Static Dispatch
**Level 1 - mod.rs** (routes by `from` type category):
```rust
match from {
_ if is_string(from) => string::cast_from_string(array, to, opts),
_ if is_boolean(from) => boolean::cast_from_boolean(array, to, opts),
_ if is_numeric(from) => numeric::cast_from_numeric(array, from, to, opts),
_ if is_temporal(from) => temporal::cast_from_temporal(array, from, to, opts),
_ if is_complex(from) => complex_types::cast_from_complex(array, from, to, opts),
_ if is_binary(from) => binary::cast_from_binary(array, to, opts),
}
```
**utils.rs** (type category helpers):
```rust
pub fn is_numeric(dt: &DataType) -> bool {
matches!(dt, Int8 | Int16 | Int32 | Int64 | Float32 | Float64 | Decimal128(_, _))
}
pub fn is_temporal(dt: &DataType) -> bool {
matches!(dt, Date32 | Date64 | Timestamp(_, _))
}
pub fn is_string(dt: &DataType) -> bool {
matches!(dt, Utf8 | LargeUtf8)
}
// ...
```
**Level 2 - string.rs** (routes by `to` type):
```rust
pub fn cast_from_string(array: &ArrayRef, to: &DataType, opts: &SparkCastOptions) -> SparkResult<ArrayRef> {
match to {
Boolean => utf8_to_boolean(array, opts),
Int8 => utf8_to_int8(array, opts),
Int16 => utf8_to_int16(array, opts),
// ...
}
}
```
**Level 3 - actual function** (branches by eval_mode at array level): (This is adaptable and not all casts might need separate implementation at an EValMode level)
```rust
fn utf8_to_int8(array: &ArrayRef, opts: &SparkCastOptions) -> SparkResult<ArrayRef> {
match opts.eval_mode {
EvalMode::Legacy => utf8_to_int8_legacy(array),
EvalMode::Ansi => utf8_to_int8_ansi(array),
EvalMode::Try => utf8_to_int8_try(array),
}
}
```
All static dispatch. Zero overhead. Clean separation.
## Migration Strategy
**Phase 1: Setup (1 PR)**
- Create `cast/` directory
- Add `mod.rs`, `traits.rs`, `utils.rs` with type category helpers
- Old `cast.rs` re-exports from new location (backward compatible)
**Phase 2: Move modules (1 PR per module)**
- Start with smallest: `boolean.rs`
- Proceed to other cast operation refactor based on datatypes
**Phase 3: Cleanup (1 PR)**
- Remove old `cast.rs`
- refactor imports across cast subsystem
## Testing
- Existing tests remain valid (re-exports preserve API)
- Each module gets its own `#[cfg(test)]` section
- No new tests needed initially - existing coverage carries over
- Future: add module-specific unit tests as needed
## Alternatives Considered
1. **Keep single file, just reorganize** - Doesn't solve merge conflicts
2. **Trait objects with dynamic dispatch** - Virtual call overhead, rejected for performance
3. **Macros** - Less readable, harder to debug
## Open Questions
1. Should we handle Dictionary types in a separate module or within each category?
2. Naming convention can be better
3. Generate dynamic cast support documentation (similar to scala side) and probably handle uncreachable paths during compile time ?
### Describe the potential solution
_No response_
### Additional context
_No response_
--
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: github-unsubscribe@datafusion.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] RFC: Propose Cast Module Modularization [datafusion-comet]
Posted by "coderfender (via GitHub)" <gi...@apache.org>.
coderfender commented on issue #3459:
URL: https://github.com/apache/datafusion-comet/issues/3459#issuecomment-3873774763
@andygrove , @comphead , @mbutrovich , @parthchandra . Please comment on this whenever you get a chance
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] RFC: Cast sub system Modularization [datafusion-comet]
Posted by "coderfender (via GitHub)" <gi...@apache.org>.
coderfender commented on issue #3459:
URL: https://github.com/apache/datafusion-comet/issues/3459#issuecomment-3874427981
@andygrove , the goal here is to eshtablish a trait / contract for cast operations through a trait . Example as follows
Boolean module (boolean.rs)
```
use super::traits::SparkCast;
use super::utils::is_boolean;
pub struct BooleanCast;
impl SparkCast for BooleanCast {
fn can_cast(&self, from: &DataType, to: &DataType) -> bool {
is_boolean(from) || is_boolean(to)
}
fn cast(
&self,
array: &ArrayRef,
from: &DataType,
to: &DataType,
opts: &SparkCastOptions,
) -> SparkResult<ArrayRef> {
match (from, to) {
// Boolean -> other types
(Boolean, Int8) => self.boolean_to_int8(array),
(Boolean, Int16) => self.boolean_to_int16(array),
(Boolean, Int32) => self.boolean_to_int32(array),
(Boolean, Int64) => self.boolean_to_int64(array),
(Boolean, Float32) => self.boolean_to_float32(array),
(Boolean, Float64) => self.boolean_to_float64(array),
(Boolean, Utf8) => self.boolean_to_string(array),
// Other types -> Boolean
(Int8 | Int16 | Int32 | Int64, Boolean) => self.int_to_boolean(array),
(Utf8, Boolean) => self.string_to_boolean(array, opts),
(Decimal128(_, _), Boolean) => self.decimal_to_boolean(array),
_ => Err(SparkError::Internal(format!(
"BooleanCast: unsupported {} -> {}", from, to
))),
}
}
}
impl BooleanCast {
fn boolean_to_int8(&self, array: &ArrayRef) -> SparkResult<ArrayRef> {
<implentation details>
}
fn string_to_boolean(&self, array: &ArrayRef, opts: &SparkCastOptions) -> SparkResult<ArrayRef> {
// <implentation details>
}
// other methods
}
on the caller side `cast.rs` / `mod.rs`
```
pub fn spark_cast(
array: &ArrayRef,
from: &DataType,
to: &DataType,
opts: &SparkCastOptions,
) -> SparkResult<ArrayRef> {
match from {
_ if is_boolean(from) => BooleanCast.cast.cast(array, from, to, opts),
_ if is_string(from) => StringCast.cast(array, from, to, opts),
_ if is_numeric(from) => NumericCast.cast(array, from, to, opts),
// ... other data types
}
}
```
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] RFC: Cast sub system Modularization [datafusion-comet]
Posted by "coderfender (via GitHub)" <gi...@apache.org>.
coderfender commented on issue #3459:
URL: https://github.com/apache/datafusion-comet/issues/3459#issuecomment-3874470677
In this approach ,we will be having two phased dispatch to the right function
1. The match arm in the main code (which is now ~ 100 + lines) will be calling the right cast sub module (the length would now be around the number of data types we support)
2. The cast impl guarantees a contract to pass fromtype, to_type , eval mode and other spark options (like session timezone needed for temporal types) and newer implementations , benchmarks and automated test suites ( in future) would benefit from this contract
3. We could potentially avoid the trait and can create a type alias to make it simpler but might be less performant
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] RFC: Cast sub system Modularization [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on issue #3459:
URL: https://github.com/apache/datafusion-comet/issues/3459#issuecomment-3874467190
Thanks for the additional info @coderfender. I don't understand why the trait adds value given that there is exactly one implementation per cast and the routing is all static and known at compile time. There is no dynamic dispatch requirement.
I am definitely in favor of splitting the current code up into separate files per cast.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] RFC: Cast sub system Modularization [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on issue #3459:
URL: https://github.com/apache/datafusion-comet/issues/3459#issuecomment-3873982744
Refactoring the current code into separate files makes sense to me. I am not sure that I understand the plan for the proposed trait though.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] RFC: Cast sub system Modularization [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove closed issue #3459: RFC: Cast sub system Modularization
URL: https://github.com/apache/datafusion-comet/issues/3459
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] RFC: Cast sub system Modularization [datafusion-comet]
Posted by "coderfender (via GitHub)" <gi...@apache.org>.
coderfender commented on issue #3459:
URL: https://github.com/apache/datafusion-comet/issues/3459#issuecomment-3874505402
@andygrove , The sole reason I wanted to add a trait is to enforce contract and consistency across various cast implementations. For example , some of the cast implementations which have different signatures
```
fn spark_cast_utf8_to_boolean<OffsetSize>(from: &dyn Array, eval_mode: EvalMode) -> SparkResult<ArrayRef>
fn cast_string_to_int<OffsetSize>(to_type: &DataType, array: &ArrayRef, eval_mode: EvalMode) -> SparkResult<ArrayRef>
fn cast_float64_to_decimal128(array: &dyn Array, precision: u8, scale: i8, eval_mode: EvalMode) -> SparkResult<ArrayRef>
```
But I do agree that we could split this into multiple files as a first step and perhaps build on that at a later time
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] RFC: Cast sub system Modularization [datafusion-comet]
Posted by "coderfender (via GitHub)" <gi...@apache.org>.
coderfender commented on issue #3459:
URL: https://github.com/apache/datafusion-comet/issues/3459#issuecomment-3875792165
I started modularizing all casts involving fromtype : `boolean` and already found that we have a bug in the nested / recursive structure function : `can_cast_from_boolean` and match arm ``` DataType::Boolean => matches!(
to_type,
DataType::Int8
| DataType::Int16
| DataType::Int32
| DataType::Int64
| DataType::Float32
| DataType::Float64
| DataType::Utf8
), ```
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] RFC: Cast sub system Modularization [datafusion-comet]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on issue #3459:
URL: https://github.com/apache/datafusion-comet/issues/3459#issuecomment-3873863508
It makes a lot of sense IMO as currently working with casting in scope of DF52 migration and there is def a room for restructure.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] RFC: Cast sub system Modularization [datafusion-comet]
Posted by "coderfender (via GitHub)" <gi...@apache.org>.
coderfender commented on issue #3459:
URL: https://github.com/apache/datafusion-comet/issues/3459#issuecomment-3992058899
take
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org