gabotechs commented on issue #22708:
URL: https://github.com/apache/datafusion/issues/22708#issuecomment-4600102241

   One thing that I worry about is this piece of code:
   
   ```rust
   #[expect(clippy::needless_pass_by_value)]
   pub fn need_data_exchange(plan: Arc<dyn ExecutionPlan>) -> bool {
       plan.properties().evaluation_type == EvaluationType::Eager
   }
   ```
   
   I think this is wrong, eager evaluation does not necessarily means that data 
needs to be exchanged. I know `datafusion-distributed` does not use this, and I 
also do not see Ballista using this 
(https://github.com/search?q=repo%3Aapache%2Fdatafusion-ballista+need_data_exchange&type=code),
 but if we mark `BufferExec` and `AnalyzeExec` as `Eager`, this will start 
being wrong.
   
    Probably the solution is to change the `need_data_exchange` function body 
along with the evaluation type of `BufferExec` and `AnalyzeExec`.


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

Reply via email to