gstvg commented on PR #22853: URL: https://github.com/apache/datafusion/pull/22853#issuecomment-4662954376
Thanks @Adam-Alani for unconvering and fixing this. This looks great to me, but I wonder if we want to fix this without breaking changes so that it can be trivially backported to the 54 branch? If you and @rluvaton agree on this, is possible to: - Remove the body projection in `LambdaExpr::new`, keeping only the used columns indices collection - In `HigherOrderFunctionExpr::evaluate`, instead of projecting the batch, derive a new batch with uncaptured columns swapped with a cheap `NullArray` - In `LambdaArgument::evaluate`, inline `merge_captures_with_variables`, evaluate the parameters before spreading captures, get the len of the first evaluated parameter (0-param lambdas should return an error), then, if all columns in `captures` are `DataType::Null`, directly create a new batch of null arrays with the len of the first evaluated parameter, instead of calling `spread_captures` That's how it was implemented in the first version of the [lambda PR](https://github.com/gstvg/arrow-datafusion/blob/3ded1154a4aa7adc167ec033622308c2baa8d524/datafusion/physical-expr/src/lambda_function.rs#L299-L322) -- 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]
