askalt commented on PR #19893: URL: https://github.com/apache/datafusion/pull/19893#issuecomment-3815897160
> I am sorry this is taking so long, but I a very worried about API churn and it takes me a non trivial amount of time to review the potential implications of this change > > I have two major points: > > 1. I really think it is important to not force downstream users to change how they call `project_schema` as it is a very common operation and there is no technical reason we can't keep their code the same (though it is harder in Datafusion). I made a PR to do so (and backout many of the changes currently required in this PR): [askalt#5](https://github.com/askalt/datafusion/pull/5) as suggested by @crepererum > 2. As I mentioned previously `OptionProjectionRef` seems overly complicated to me to add to the public API -- and because the `Option` is wrapped, it means that the normal functions for `Option` (like match and .and_then, etc) don't work. I think it would be simpler, and follow the rest of the code more closely if it were `Option<ProjectionRef>` and the special projection application code is put into ProjectionRef > > Thank you for your work on this @askalt Ok, I will done it. -- 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]
