sam-1112 commented on code in PR #7196:
URL: https://github.com/apache/datafusion-comet/pull/7196#discussion_r4432343069


##########
native/core/src/parquet/schema_adapter.rs:
##########
@@ -739,17 +675,29 @@ fn check_conversion(
     match (physical_type, target_type) {
         (DataType::Struct(physical_fields), DataType::Struct(target_fields)) 
=> {
             let physical_indices = match_struct_fields(physical_fields, 
target_fields, options)?;
-            
combine_checks(target_fields.iter().zip(physical_indices).filter_map(
-                |(target_field, physical_index)| {
-                    let physical_field = &physical_fields[physical_index?];
-                    Some(check_conversion(
-                        physical_field.data_type(),
-                        target_field.data_type(),
-                        &format!("{column}, {}", physical_field.name()),
-                        options,
-                    ))
-                },
-            ))
+            combine_checks(
+                target_fields
+                    .iter()
+                    .zip(physical_indices.iter())
+                    .filter_map(|(target_field, (physical_index, owns_value))| 
{
+                        let physical_field = &physical_fields[physical_index?];
+                        Some(
+                            check_conversion(
+                                physical_field.data_type(),
+                                target_field.data_type(),
+                                &format!("{column}, {}", 
physical_field.name()),
+                                options,
+                            )
+                            .map(|check| {
+                                if owns_value {
+                                    check
+                                } else {
+                                    ConversionCheck::Accept
+                                }
+                            }),

Review Comment:
   Losing nested selections now retain their `RejectOnNonEmpty` result; 
ownership only determines whether runtime conversion supplies the value or NULL.
   
   I updated the depth 1/2 `Int32` cases and the losing-leaf validation test 
accordingly. Regression coverage confirms that both field orders fail with the 
expected `[s, A]` conversion error on non-empty input, while empty files and 
fully pruned row groups still read successfully. Compatible losing fields 
continue to return NULL.



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