Dandandan commented on pull request #268:
URL: https://github.com/apache/arrow-datafusion/pull/268#issuecomment-832722639


   > Nice catch! Though looks like removing the optimisation breaks a crapload 
of logical plan text assertions in the tests that now contain an extra 
top-level projection node :(
   > 
   > Could we retain the optimisation but only apply it when `expr` consists 
entirely of simple column refs? Not sure if that'd patch up all the tests or 
not.
   > 
   > Edit: or we could just modify the test assertions, I suppose, if we want 
to keep the initial planning process simple and do this kind of optimisation 
later?
   
   Thanks @returnString !
   I will have a look later. I think it would be best to update the tests 
instead for now.
   
   I think the optimization also doesn't improve performance that much, as the 
projection with just the fields should be almost "free" in DF. I will add an 
issue to add the optimization as a rule.


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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to