kosiew commented on code in PR #26464:
URL: https://github.com/apache/datafusion/pull/26464#discussion_r4492278779
##########
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:
Could we add an INT64 TIMESTAMP(NANOS,false) sibling before `ts` inside the
Avro `array` group and verify that it remains nanoseconds while `ts` becomes
microseconds? This would strengthen coverage of positional discrimination
between timestamp leaves within the same legacy-list root. Optional test
hardening.
##########
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:
Could we also assert the expected Arrow fields for the Hive, bare repeated,
and Avro cases, including wrappers, child names, and nullability? Flattened
leaf types cannot detect changes to those properties, and the legacy encodings
have different wrapper and nullability rules. Optional regression hardening.
--
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]
