ranflarion commented on code in PR #26464:
URL: https://github.com/apache/datafusion/pull/26464#discussion_r4521031596


##########
datafusion/datasource-parquet/src/schema_coercion.rs:
##########
@@ -1523,6 +1427,93 @@ mod tests {
         assert_eq!(result, expected_schema);
     }
 
+    #[test]
+    fn coerce_int96_to_resolution_with_two_level_and_legacy_lists() {
+        // Hive's two-level `array<timestamp>`, parquet-mr Avro's `array` 
group and a bare repeated
+        // field: lists without the `list` group of the three-level layout
+        let schema = "
+        message spark_schema {
+          optional group hive (LIST) {
+            repeated int96 bag;
+          }
+          repeated int96 bare;
+          optional group avro (LIST) {
+            repeated group array {
+              optional int96 ts;
+              optional int32 id;

Review Comment:
   added `ns` (INT64 `TIMESTAMP(NANOS,false)`) ahead of `ts` in the `array` 
group, it stays nanoseconds while `ts` becomes microseconds. It also catches a 
walk that coerces every nanosecond leaf of a root containing an INT96 
regardless of position, which all five INT96 tests on the previous head passed.
   



##########
datafusion/datasource-parquet/src/schema_coercion.rs:
##########
@@ -1523,6 +1427,93 @@ mod tests {
         assert_eq!(result, expected_schema);
     }
 
+    #[test]
+    fn coerce_int96_to_resolution_with_two_level_and_legacy_lists() {
+        // Hive's two-level `array<timestamp>`, parquet-mr Avro's `array` 
group and a bare repeated
+        // field: lists without the `list` group of the three-level layout
+        let schema = "
+        message spark_schema {
+          optional group hive (LIST) {
+            repeated int96 bag;
+          }
+          repeated int96 bare;
+          optional group avro (LIST) {
+            repeated group array {
+              optional int96 ts;
+              optional int32 id;
+            }
+          }
+          optional group three (LIST) {
+            repeated group list {
+              optional int96 element;
+            }
+          }
+          optional group m (MAP) {
+            repeated group key_value {
+              required int32 key;
+              optional int96 value;
+            }
+          }
+          optional int64 nanos (TIMESTAMP(NANOS,false));
+        }
+        ";
+        let descr = SchemaDescriptor::new(Arc::new(
+            parse_message_type(schema).expect("should parse schema"),
+        ));
+        let arrow_schema = parquet_to_arrow_schema(&descr, None).unwrap();
+
+        fn leaf_types(data_type: &DataType, leaves: &mut Vec<DataType>) {
+            match data_type {
+                DataType::Struct(children) => children
+                    .iter()
+                    .for_each(|child| leaf_types(child.data_type(), leaves)),
+                DataType::List(item) | DataType::Map(item, _) => {
+                    leaf_types(item.data_type(), leaves)
+                }
+                leaf => leaves.push(leaf.clone()),
+            }
+        }
+        let schema_leaf_types = |schema: &Schema| {
+            let mut leaves = vec![];
+            schema
+                .fields()
+                .iter()
+                .for_each(|field| leaf_types(field.data_type(), &mut leaves));
+            leaves
+        };
+        let micros = DataType::Timestamp(TimeUnit::Microsecond, None);
+
+        let result = Int96Coercer::new(&descr, &arrow_schema, 
&TimeUnit::Microsecond)
+            .coerce()
+            .unwrap();
+        assert_eq!(

Review Comment:
   done, the test now compares the whole coerced schema against one spelled out 
with each layout's wrappers, child names and nullability: non-null `bag` and 
`bare` items, a non-null bare list, a non-null Avro `array` struct. The 
projected case compares against the matching fields of it, which replaced the 
leaf-flattening helper.
   



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to