adriangb commented on code in PR #25342:
URL: https://github.com/apache/datafusion/pull/25342#discussion_r4027087595


##########
datafusion/datasource-parquet/src/schema_coercion.rs:
##########
@@ -51,158 +66,131 @@ pub fn apply_file_schema_type_coercions(
     table_schema: &Schema,
     file_schema: &Schema,
 ) -> Option<Schema> {
-    let mut needs_view_transform = false;
-    let mut needs_string_transform = false;
-    let mut needs_nested_transform = false;
+    let fields = coerce_fields_by_name(table_schema.fields(), 
file_schema.fields())?;
+    Some(Schema::new_with_metadata(
+        fields,
+        file_schema.metadata.clone(),
+    ))
+}
 
+/// Coerce `file_fields` towards `table_fields`, matching fields by name.
+///
+/// File fields with no counterpart in `table_fields` are kept unchanged and
+/// table fields missing from the file are ignored. Returns `None` if no field
+/// changed.
+fn coerce_fields_by_name(table_fields: &Fields, file_fields: &Fields) -> 
Option<Fields> {

Review Comment:
   Good catch, fixed in adad560.
   
   Measured against `main`'s implementation (ns per 
`apply_file_schema_type_coercions` call, release build):
   
   | schema | `main` | this PR |
   | --- | --- | --- |
   | 100 primitive cols, nothing coercible | 1629 | 3255 |
   | 50 struct/list cols, nothing coercible | 17285 | 5588 |
   | 100 string cols, all coerced | 14351 | 14837 |
   
   The one regression left is a hash lookup per column on the all-primitive 
path, which the old code skipped by early-returning right after building its 
`HashMap`. At ~1.6 µs per file open for a 100-column schema I'd rather not 
reintroduce the hand-maintained list for it, but it's a one-liner if you'd 
prefer it gone.



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