alamb commented on code in PR #8400:
URL: https://github.com/apache/arrow-datafusion/pull/8400#discussion_r1413068716
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -877,19 +877,20 @@ impl LogicalPlan {
input: Arc::new(inputs[0].clone()),
}))
}
- LogicalPlan::Explain(_) => {
+ LogicalPlan::Explain(e) => {
// Explain should be handled specially in the optimizers;
- // If this check cannot pass it means some optimizer pass is
- // trying to optimize Explain directly
- if expr.is_empty() {
- return plan_err!("Invalid EXPLAIN command. Expression is
empty");
- }
-
- if inputs.is_empty() {
- return plan_err!("Invalid EXPLAIN command. Inputs are
empty");
- }
-
- Ok(self.clone())
+ assert!(
Review Comment:
I recommend we make these checks return internal_err (so they error, rather
than panic in case of bug), but I also think we could do this as a follow on PR
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -877,19 +877,20 @@ impl LogicalPlan {
input: Arc::new(inputs[0].clone()),
}))
}
- LogicalPlan::Explain(_) => {
+ LogicalPlan::Explain(e) => {
// Explain should be handled specially in the optimizers;
Review Comment:
```suggestion
```
I think this comment is entirely outdated now so we could remove it
--
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]
