albertlockett commented on code in PR #10958:
URL: https://github.com/apache/arrow-rs/pull/10958#discussion_r3970281144
##########
arrow-cast/src/cast/mod.rs:
##########
@@ -2570,6 +2586,45 @@ where
}
}
+/// Returns `true` when the per-element checked conversion can be replaced by a
+/// plain widening conversion, that is when
+///
+/// ```text
+/// num_cast::<FROM, TO>(v) == Some(v.as_())
+/// ```
+///
+/// holds for *every* value of `FROM`.
+///
+/// Note that the required invariant is agreement with [`AsPrimitive::as_`],
not
+/// merely that `num_cast` is total; a pair belongs here only if it also has a
+/// case in `test_infallible_numeric_casts`, which asserts exactly that
equality.
+///
+/// The match is fail-closed: an unlisted pair keeps the existing checked
path, so
+/// an omission costs performance and never correctness. The set is
deliberately
+/// conservative and does not attempt to be exhaustive. In particular
`Float16` is
+/// excluded, because `f16` has no primitive `as` conversion and so needs
separate
+/// correctness reasoning.
+///
+/// Called with associated constants, so each monomorphisation folds this to a
+/// constant and the branch disappears.
+fn is_infallible_numeric_cast(from: &DataType, to: &DataType) -> bool {
Review Comment:
I'm wondering if this could/should be a method on
[`ArrowPrimitiveType`](https://github.com/apache/arrow-rs/blob/b7e5977133eadfe652eee4d4dc814a4c9d135533/arrow-array/src/types.rs#L59-L67)?
The reason I'm asking specifically is: I'd like to use this in the
dictionary builders to optimize converting keys in these `try_new_from_builder`
methods:
https://github.com/apache/arrow-rs/blob/b7e5977133eadfe652eee4d4dc814a4c9d135533/arrow-array/src/builder/generic_bytes_dictionary_builder.rs#L198-L207
The canonical use case for these mtethods is: as a dictionary array is
being built, if it turns out it would overflow, it can be upgraded efficiently
to a builder with a wider key, which would typically be an infallible. It would
be nice if we could reuse this same logic to know when we can do the same
optimization.
e.g., wondering if it'd be acceptable to do something like:
```rs
pub trait ArrowPrimitiveType: primitive::PrimitiveTypeSealed + 'static {
// ...
// the new function:
fn can_widen_to<K: ArrowPrimitiveType>() -> bool {
matches!(
(Self::DATA_TYPE, K::DATA_TYPE),
// ... the widening match cases
)
}
```
Then in `cast_numeric_arrays` I think the the check becomes
`FROM::can_widen_to::<TO>()`
--
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]
