patrickswedish commented on PR #24699:
URL: https://github.com/apache/datafusion/pull/24699#issuecomment-5427286804

   > FYI @patrickswedish as the original author, would you mind reviewing this 
back port?
   
   Thanks for asking me to review the backport. I compared it against the 
merged #24394 and the actual branch-55 base rather than treating it as a 
mechanical cherry-pick.
   
   The Struct/Union path looks correctly carried over, and moving normalization 
to the MemoryStream producer boundary is still the right invariant.
   
   One backport-specific gap stood out: #24394 originally merged on top of 
#23914, which had already added recursive Map schema adaptation in 
nested_struct. branch-55 does not have that prerequisite.
   
   In this backport, adapt_batch_to_schema can now run for contained 
runtime-schema mismatches, but branch-55's cast_column still has no 
DataType::Map adaptation arm. So a stricter nested Map schema accepted by 
Schema::contains may fall through to Arrow's generic cast path rather than the 
recursive schema-conformance path present under the original PR.
   
   Before approval, I think it would be worth adding one focused regression:
   
   declared schema: Map<..., Struct<field nullable>>
   runtime batch: Map<..., Struct<field non-nullable>>
   confirm Schema::contains accepts it
   exercise adapt_batch_to_schema / MemoryStream
   
   If Arrow 56 already handles this case correctly, then no extra code is 
needed. If it fails, we should either backport the minimal Map prerequisite 
from #23914 or explicitly constrain the branch-55 adapter to shapes this branch 
can safely normalize.
   
   Everything else I checked looks aligned with the original fix direction.


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