Copilot commented on code in PR #25342:
URL: https://github.com/apache/datafusion/pull/25342#discussion_r4019866081
##########
datafusion/datasource-parquet/src/schema_coercion.rs:
##########
@@ -607,52 +595,253 @@ mod tests {
#[test]
fn nested_coercion_preserves_list_containers() {
- let wrap: [fn(FieldRef) -> DataType; 6] = [
+ // The table side deliberately differs from the file side in the child
+ // field name, nullability and metadata, as well as in the
+ // FixedSizeList width and Map ordering: all of those must be taken
+ // from the file, only the leaf types come from the table.
+ let file_wrap: [fn(FieldRef) -> DataType; 6] = [
DataType::List,
DataType::LargeList,
DataType::ListView,
DataType::LargeListView,
|field| DataType::FixedSizeList(field, 3),
|field| DataType::Map(field, false),
];
- for wrap in wrap {
- let file_element = Arc::new(Field::new_struct(
- "entries",
- vec![
- Field::new("key", DataType::Utf8, false),
- Field::new("value", DataType::Binary, true),
- ],
- false,
- ));
+ let table_wrap: [fn(FieldRef) -> DataType; 6] = [
+ DataType::List,
+ DataType::LargeList,
+ DataType::ListView,
+ DataType::LargeListView,
+ |field| DataType::FixedSizeList(field, 7),
+ |field| DataType::Map(field, true),
+ ];
+ for (file_wrap, table_wrap) in file_wrap.into_iter().zip(table_wrap) {
+ let file_element = Arc::new(
+ Field::new_struct(
+ "key_value",
+ vec![
+ Field::new("key", DataType::Utf8, false),
+ Field::new("value", DataType::Binary, true),
+ ],
+ false,
+ )
+ .with_metadata(HashMap::from([("source".into(),
"file".into())])),
+ );
let table_element = Arc::new(Field::new_struct(
"entries",
vec![
- Field::new("value", DataType::Utf8View, true),
- Field::new("key", DataType::Utf8View, false),
- ],
- false,
- ));
- let expected_element = Arc::new(Field::new_struct(
- "entries",
- vec![
- Field::new("key", DataType::Utf8View, false),
- Field::new("value", DataType::Utf8View, true),
+ Field::new("value", DataType::Utf8View, false),
+ Field::new("key", DataType::Utf8View, true),
],
- false,
+ true,
));
+ let expected_element = Arc::new(
+ Field::new_struct(
+ "key_value",
+ vec![
+ Field::new("key", DataType::Utf8View, false),
+ Field::new("value", DataType::Utf8View, true),
+ ],
+ false,
+ )
+ .with_metadata(HashMap::from([("source".into(),
"file".into())])),
+ );
let file_schema =
- Schema::new(vec![Field::new("data", wrap(file_element),
true)]);
+ Schema::new(vec![Field::new("data", file_wrap(file_element),
true)]);
let table_schema =
- Schema::new(vec![Field::new("data", wrap(table_element),
true)]);
+ Schema::new(vec![Field::new("data", table_wrap(table_element),
false)]);
let expected =
- Schema::new(vec![Field::new("data", wrap(expected_element),
true)]);
+ Schema::new(vec![Field::new("data",
file_wrap(expected_element), true)]);
assert_eq!(
apply_file_schema_type_coercions(&table_schema, &file_schema),
Some(expected)
);
}
}
+ #[test]
+ fn nested_coercion_matches_map_entries_by_position() {
+ // Parquet readers always name map children `key_value`/`key`/`value`,
+ // while Arrow producers (e.g. `MapBuilder`) default to
+ // `entries`/`keys`/`values`. Names must not matter for maps.
+ let file_schema = Schema::new(vec![Field::new_map(
+ "m",
+ "key_value",
+ Field::new("key", DataType::Utf8, false),
+ Field::new("value", DataType::Binary, true),
+ false,
+ true,
+ )]);
+ let table_schema = Schema::new(vec![Field::new_map(
+ "m",
+ "entries",
+ Field::new("keys", DataType::Utf8View, false),
+ Field::new("values", DataType::Utf8View, true),
+ false,
+ true,
+ )]);
+ let expected = Schema::new(vec![Field::new_map(
+ "m",
+ "key_value",
+ Field::new("key", DataType::Utf8View, false),
+ Field::new("value", DataType::Utf8View, true),
+ false,
+ true,
+ )]);
Review Comment:
Both map positions currently request `Utf8View`, so this test cannot detect
an accidental key/value swap: reversed positional matching produces the same
expected schema. Use distinct target types (for example, `Utf8View` for the key
and `Utf8` for the value) so the positional contract is actually covered.
--
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]
