alamb commented on PR #9009:
URL: 
https://github.com/apache/arrow-datafusion/pull/9009#issuecomment-1914589292

   >  However, didn't cached (Previous behaviour). However, I don't think we 
will generate an invalid plan. I added your example as a test case also in this 
PR.
   
   Thank you
   
   > 
   Thank you for this contribution @mustafasrepo -- this PR looks good to me. 
Also, I found the description on this PR very clear and well written. Thank you 
very much 🙏
   
   One thought I had was will there be a problem if there is a subquery that 
would end up with a nested WindowAggExec that could be incorrectly optimized 
away 🤔
   
   Something like
   
   SELECT c3,
       SUM(c9) OVER(ORDER BY c3+c4 ASC) as sum2,
       sum1,
       FROM (
         SELECT c3, c4, c9, 
         SUM(c9) OVER(ORDER BY c3+c4 DESC) as sum1,
         FROM aggregate_test_100
       )
   I think, in these cases, we will generate a sub-optimal plan, where a 
complex expression is calculated more than once by subsequent operators. 
However, didn't cached (Previous behaviour). However, I don't think we will 
generate an invalid plan. I added your example as a test case also in this PR.
   
   > I think as a future PR, we can analyze plan from top down to count 
expression referral count, for better calculating referral counts across plan.
   
   Yes, I agree there is no need to optimize this case as part of this PR, and 
since it gives correct results, lets 🚀 
   


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

Reply via email to