gstvg commented on code in PR #18921: URL: https://github.com/apache/datafusion/pull/18921#discussion_r2982634042
########## datafusion/functions-nested/src/array_transform.rs: ########## Review Comment: Done at https://github.com/apache/datafusion/pull/18921/changes/6ae73cb4a569d6693d37b601cca38a4c689722df, thanks for the tests > 1. replace null lists that is not empty underneath, with nulls with empty list underneath. ... I'm conflicted on whether we should fix case 1 for them as it is costly and the user might have prior knowledge to avoid that I was waiting your input whether this should be handled or not. I share your concern regarding the cost of this, but also agree that it should be fixed by default, as done now. I think it can be made configurable, either via a config like `SET array_transform.remove_nulls = false` or an optional argument, and add an UDF that does that cleanup so an array can be explicit cleaned once and then be transformed multiples times across a plan without being re-cleaned again for each transformation, but not on this PR. -- 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]
