viirya commented on code in PR #5226:
URL: https://github.com/apache/arrow-rs/pull/5226#discussion_r1431917969
##########
arrow-cast/src/cast.rs:
##########
@@ -160,11 +160,18 @@ pub fn can_cast_types(from_type: &DataType, to_type:
&DataType) -> bool {
(Decimal128(_, _) | Decimal256(_, _), Utf8 | LargeUtf8) => true,
// Utf8 to decimal
(Utf8 | LargeUtf8, Decimal128(_, _) | Decimal256(_, _)) => true,
- (Struct(from_fields), Struct(to_fields)) => {
- from_fields.len() == to_fields.len() &&
-
from_fields.iter().zip(to_fields.iter()).all(|(f1, f2)| {
- can_cast_types(f1.data_type(),
f2.data_type())
- })
+ (Struct(from_fields), Struct(to_fields)) => {
+ from_fields.len() == to_fields.len() &&
+ from_fields.iter().zip(to_fields.iter()).all(|(f1, f2)| {
+ // Only allows cast nullable or non-nullable fields to
nullable fields.
+ // Although it is generally allowed to cast non-nullable
fields to non-nullable fields,
+ // but if casting f1.data_type to f2.data_type may return
null, e.g., overflow,
+ // it is not allowed when f2 is non-nullable. Because
`can_cast_types` doesn't
+ // take `CastOptions` so we cannot check `safe` option
here. We take safer
+ // approach to assume `safe` is true, so disallowing the
case of casting non-nullable
+ // fields to non-nullable fields.
+ f2.is_nullable() && can_cast_types(f1.data_type(),
f2.data_type())
Review Comment:
I took a safer approach here, except for we want to change `can_cast_types`
to take `CastOptions` as new parameter.
--
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]
