Dandandan edited a comment on pull request #378: URL: https://github.com/apache/arrow-datafusion/pull/378#issuecomment-847031288
> I've added a new constructor that allows opting into the new behaviour. I wasn't aware of the way that MergeExec is plumbed into the plans and that this would break it. > > I do wonder if instead of relying on an `AddMergeExec` optimisation pass, the plan conversion from `LogicalPlan::Sort` should just inspect the input partitioning and add the Merge if necessary. After all, it already has to inspect the partitioning for operators such as `LogicalPlan::Limit`, and so not just generating a valid plan from the outset seems a touch surprising to me... I think you are right about that, we should not rely on the optimizer to make the execution plan correct. I think it would be better if the planner adds the `MergeExec`s for the appropriate nodes. -- 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]
