alamb commented on code in PR #11536:
URL: https://github.com/apache/datafusion/pull/11536#discussion_r1683441390
##########
datafusion/physical-plan/src/union.rs:
##########
@@ -431,7 +431,12 @@ impl ExecutionPlan for InterleaveExec {
self: Arc<Self>,
children: Vec<Arc<dyn ExecutionPlan>>,
) -> Result<Arc<dyn ExecutionPlan>> {
- Ok(Arc::new(InterleaveExec::try_new(children)?))
+ // New children may not be able to interleave; in that case, we fall
back to UnionExec.
Review Comment:
I see how this particular change is beneficial as now the plan runs where it
didn't before, so that is hard to argue with and I would be ok merging it in
However, I feel like this might be silently masking some bug in the planner.
My personal preference would be to make this an error
```rust
if !can_interleave(children.iter()) {
return internal_err!("Can not create InterleaveExec: new children can not
be interleaved");
}
```
And then track down / fix whatever optimizer pass is causing the children to
no longer be interleavable
FYI @mustafasrepo
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
