HippoBaro commented on code in PR #11278:
URL: https://github.com/apache/arrow-rs/pull/11278#discussion_r4174708167
##########
parquet/src/arrow/arrow_writer/levels.rs:
##########
@@ -933,42 +967,36 @@ impl LevelInfoBuilder {
/// and the other is a native array, the dictionary values must have the
same type as the
/// native array
fn types_compatible(a: &DataType, b: &DataType) -> bool {
- // if the Arrow data types are equal, the types are deemed compatible
- if a.equals_datatype(b) {
- return true;
- }
-
- // get the values out of the dictionaries
- let (a, b) = match (a, b) {
- (DataType::Dictionary(_, va), DataType::Dictionary(_, vb)) => {
- (va.as_ref(), vb.as_ref())
+ let (a, _) = logical_type(a);
+ let (b, _) = logical_type(b);
+ match (a, b) {
+ (DataType::Struct(a), DataType::Struct(b)) => {
+ a.len() == b.len()
+ && a.iter()
+ .zip(b)
+ .all(|(a, b)| Self::types_compatible(a.data_type(),
b.data_type()))
}
- (DataType::Dictionary(_, v), b) => (v.as_ref(), b),
- (a, DataType::Dictionary(_, v)) => (a, v.as_ref()),
- _ => (a, b),
- };
-
- // now that we've got the values from one/both dictionaries, if the
values
- // have the same Arrow data type, they're compatible
- if a == b {
- return true;
- }
-
- // here we have different Arrow data types, but if the array contains
the same type of data
- // then we consider the type compatible
- match a {
- // String, StringView and LargeString are compatible
- DataType::Utf8 => matches!(b, DataType::LargeUtf8 |
DataType::Utf8View),
- DataType::Utf8View => matches!(b, DataType::LargeUtf8 |
DataType::Utf8),
- DataType::LargeUtf8 => matches!(b, DataType::Utf8 |
DataType::Utf8View),
-
- // Binary, BinaryView and LargeBinary are compatible
- DataType::Binary => matches!(b, DataType::LargeBinary |
DataType::BinaryView),
- DataType::BinaryView => matches!(b, DataType::LargeBinary |
DataType::Binary),
- DataType::LargeBinary => matches!(b, DataType::Binary |
DataType::BinaryView),
-
- // otherwise we have incompatible types
- _ => false,
+ (DataType::List(a), DataType::List(b))
+ | (DataType::LargeList(a), DataType::LargeList(b))
+ | (DataType::ListView(a), DataType::ListView(b))
+ | (DataType::LargeListView(a), DataType::LargeListView(b)) => {
+ Self::types_compatible(a.data_type(), b.data_type())
+ }
+ (DataType::FixedSizeList(a, an), DataType::FixedSizeList(b, bn))
if an == bn => {
Review Comment:
Agreed, updated the compatibility checks to use descriptive names for the
left/right fields, fixed-size list lengths, and map sortedness flags.
--
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]
