zhuqi-lucas commented on PR #21827:
URL: https://github.com/apache/datafusion/pull/21827#issuecomment-4323945220
> `assert_or_internal_err` only returns an `internal_err`, and it seems like
the optimizer didn't propagate the error correctly, causing a panic.
The error is propagated correctly — it bubbles up through `transform_up` and
fails the entire optimizer rule. The problem is that `with_new_children` is
called internally by `transform_up` during tree traversal, so the optimizer
rule has no opportunity to catch the error and fall back. The error propagates
all the way up and kills the optimization pass.
> I think it is appropriate to return an error here. It should be up to the
user (e.g., the optimizer rule) to decide whether to fall back
I agree in principle, but in practice the optimizer rule never gets that
chance. Here's the call chain:
```
optimizer_rule.optimize(plan)
→ plan.transform_up(closure)
→ node.with_new_children(modified_children) // error here
→ Err propagates up through transform_up
→ Err propagates up, entire rule fails
```
There's no hook point where the optimizer rule can intercept the error from
`with_new_children` and decide to fall back. The `transform_up` machinery
doesn't expose per-node error handling.
Falling back to `UnionExec` inside `with_new_children` is safe because
`UnionExec` is always a valid replacement — it's semantically correct, just
without the interleave optimization. `EnforceDistribution` will add
`RepartitionExec` as needed downstream.
--
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]
