unikdahal commented on PR #5414: URL: https://github.com/apache/datafusion-comet/pull/5414#issuecomment-5606105798
Thanks @sunchao and @andygrove for the review here. Given how many additional empty-struct edge cases this uncovered across DataFusion-backed paths, I think it makes more sense to stop growing the set of Comet-side guards/workarounds in this PR and revisit this from a cleaner dependency baseline. The upstream `ScalarValue::compact` / `new_default` fix has now been backported to `branch-55` in apache/datafusion#24876 for 55.1.0. That addresses one important class of failures we hit here, but the review also surfaced several other paths that should be re-evaluated independently rather than assuming empty structs are universally safe. I'm going to close this PR for now and pick it back up once Comet is on DataFusion 55.1. I'll re-audit the remaining cases from that baseline and keep the Comet-side changes as narrow as possible. Thanks again for digging into all of these cases. -- 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]
