Improve type coercion and casting

[jayzhan211]

Is your feature request related to a problem or challenge?

I think there is room for improvement in type coerceion or casting.

Background

comparison_coercion is widely used in datafusion, a lossless conversion
https://github.com/apache/arrow-datafusion/blob/main/datafusion/expr/src/type_coercion/binary.rs

can_coerce_from is used mainly for signature, a lossless conversion
https://github.com/apache/arrow-datafusion/blob/main/datafusion/expr/src/type_coercion/functions.rs

can_cast_types is from arrow-cast, which is a lossy conversion. It is also used in some comparison_coercion building block. https://github.com/apache/arrow-rs/blob/df69ef57d055453c399fa925ad315d19211d7ab2/arrow-cast/src/cast.rs#L76-L273

Not sure if there is other coercion I missed

Proposal

comparison_coercion and can_coerce_from seem like doing the similar thing, maybe we can just have one lossless conversion. If lossless conversion is useful for arrow-rs, we can introduce a lossless version of can_cast_types, then rely on it for datafusion.

Lossy conversion vs Lossless

I think the definition for lossy is that the value is not recoverable after casting back, otherwise it is lossless.

Lossy

• Int32 to Int16 / Int8

Lossless

• Int32 to Int64

Describe the solution you'd like

1. Replace can_coerce_from with comparison_coercion's building block numeric coercion, list coercion, string coercion, null coercion, etc
2. Split list_coercion from string_coercion to make each building block of coercion clear on the task it focus on. list_coercion do list/fixed size list/large list coercion, string_coercion do utf/large utf coercion.
3. Introduce these lossless coercion to arrow-rs?

Known issue or question I have

• Introduce list_coercion that currently exist in string_concat_coercion
• No list coercion for can_coerce_from
• Decimal128 can cast to Float64 in can_coerce_from, why?

Describe alternatives you've considered

If there are many customize conversion need, then this change might not be helpful at all. We need other approach to let type casting / coercion easy to use.

No response

Additional context

No response

[jayzhan211]

@tustvold Do you think introduce a lossless version of can_cast_types helpful?
cc @alamb

[tustvold]

Decimal128 can cast to Float64 in can_coerce_from, why?

Floating points inherently permit precision loss. It is fairly standard that otherwise checked conversion permit precision loss when converting between floats.

For example - https://docs.rs/num-traits/latest/num_traits/cast/trait.FromPrimitive.html

A value can be represented by the target type when it lies within the range of scalars supported by the target type. For example, a negative integer cannot be represented by an unsigned integer type, and an i64 with a very high magnitude might not be convertible to an i32. On the other hand, conversions with possible precision loss or truncation are admitted, like an f32 with a decimal part to an integer type, or even a large f64 saturating to f32 infinity.

No list coercion for can_coerce_from

I think this would make sense as an addition, the current support for nested types in arrow-cast is very limited.

If lossless conversion is useful for arrow-rs

In general I think it would be very confusing if coercion can result in silent data loss, perhaps you might articulate why this would be useful? My current thinking is casting should never silently lose data.

[jayzhan211]

My current thinking is casting should never silently lose data.

https://github.com/apache/arrow-rs/blob/df69ef57d055453c399fa925ad315d19211d7ab2/arrow-cast/src/cast.rs#L76C2-L80

Comment in can_cast_types mention that cast is lossy.

https://github.com/apache/arrow-rs/blob/df69ef57d055453c399fa925ad315d19211d7ab2/arrow-cast/src/cast.rs#L212-L216

Casting Int64 to Int32 return true. I'm not sure if the actual casting block this lossy conversion, but if not, could it silently lose data?

[tustvold]

That comment is incorrect and doesn't match the implementation. I believe it means that it supports conversions that are potentially fallible, e.g. Int32 to Int8, but it never silently looses data

[jayzhan211]

That comment is incorrect and doesn't match the implementation. I believe it means that it supports conversions that are potentially fallible, e.g. Int32 to Int8, but it never silently looses data

Any scenario that we should hint it as potentially fallible but actually do nothing for them? It seems to me return false is more straightforward if we do nothing.

[jayzhan211]

However, I think we can still cleanup comparison_coercion and can_coerce_from and forget about lossy conversion.

[jayzhan211]

That comment is incorrect and doesn't match the implementation. I believe it means that it supports conversions that are potentially fallible, e.g. Int32 to Int8, but it never silently looses data

Any scenario that we should hint it as potentially fallible but actually do nothing for them? It seems to me return false is more straightforward if we do nothing.

Maybe Int64(1) cast to Int32(1) is one that is able to safely cast without data loss.

[jayzhan211]

Maybe we need another verion of can_cast_types that no possible fallible hint. For i64, only u64 or larger type, no i32 or i16 since we don't know the actual data.

[tustvold]

Is that not what can_coerce_from is?

[jayzhan211]

can_coerce_from

Yes! Should we have it aside can_cast_types in arrow-rs? Or just let it stay in datafusion for now.

[tustvold]

Given the type coercion logic itself lives in DataFusion I personally think it makes sense for it to remain alongside it

[jayzhan211]

Short summary for the next step of this issue

1. Replace can_coerce_from with component in comparison_coercion, so we dont need to think about which one to use. Reuse the code if possible, call can_cast_types if possible.
2. Add nested support. Start from List

TODO

• Introduce Boolean Coercion #8331
• Support LargeUtf8 to Temporal Coercion #8357
• Improve numerice_coercion and reuse in both compare op and math op #8385
• Move Coercion for MakeArray to coerce_arguments_for_signature and introduce another one for ArrayAppend #8317
• coercion for MakeArray anf Expr::InList should be the same https://github.com/apache/arrow-datafusion/blob/06bbe1298fa8aa042b6a6462e55b2890969d884a/datafusion/optimizer/src/analyzer/type_coercion.rs#L284-L317
• Reuse array_concat/append/prepend signature for list concat ops.

Issue

• Return Result for comparison_coercion, so we can differentiate unimplemented casting or not possible casting Return Option<T> is probably enough.
• Consider having component of comparison_coercion public, like Allow default value on lag/lead if they are coerceable to value #8308 can have numerice_coercion instead of comparison_coercion. No use case.
• Float is converted to Decimal with possible overflow in comparison_coercion, while Decimal is coerced to Float in can_coerced_from. Confused 😕 😕 😕

Note

can_coerce_from / comparison_coercion unlike arrow-cast should only check the casting to the larger type since the actual data is unknown.