Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 53 additions & 13 deletions datafusion/expr/src/logical_plan/plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -877,19 +877,19 @@ impl LogicalPlan {
input: Arc::new(inputs[0].clone()),
}))
}
LogicalPlan::Explain(_) => {
// 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())
LogicalPlan::Explain(e) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find we can correct transform LogicalPlan::Analyze in below
https://github.com/apache/arrow-datafusion/blob/340ecfdfe0e6667d2c9f528a60d8ee7fa5c34805/datafusion/expr/src/logical_plan/plan.rs#L871-L878
so I used the same approach in LogicalPlan::Explain

assert!(
Copy link
Contributor

@alamb alamb Dec 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend we make these checks return internal_err (so they error, rather than panic in case of bug), but this is consistent with the rest of the code in this file, so this is good

expr.is_empty(),
"Invalid EXPLAIN command. Expression should empty"
);
assert_eq!(inputs.len(), 1, "Invalid EXPLAIN command. Inputs are empty");
Ok(LogicalPlan::Explain(Explain {
verbose: e.verbose,
plan: Arc::new(inputs[0].clone()),
stringified_plans: e.stringified_plans.clone(),
schema: e.schema.clone(),
logical_optimization_succeeded: e.logical_optimization_succeeded,
}))
}
LogicalPlan::Prepare(Prepare {
name, data_types, ..
Expand Down Expand Up @@ -3076,4 +3076,44 @@ digraph {
.unwrap()
.is_nullable());
}

#[test]
fn test_transform_explain() {
let schema = Schema::new(vec![
Field::new("foo", DataType::Int32, false),
Field::new("bar", DataType::Int32, false),
]);

let plan = table_scan(TableReference::none(), &schema, None)
.unwrap()
.explain(false, false)
.unwrap()
.build()
.unwrap();

let external_filter =
col("foo").eq(Expr::Literal(ScalarValue::Boolean(Some(true))));

// after transformation, because plan is not the same anymore,
// the parent plan is built again with call to LogicalPlan::with_new_inputs -> with_new_exprs
let plan = plan
.transform(&|plan| match plan {
LogicalPlan::TableScan(table) => {
let filter = Filter::try_new(
external_filter.clone(),
Arc::new(LogicalPlan::TableScan(table)),
)
.unwrap();
Ok(Transformed::Yes(LogicalPlan::Filter(filter)))
}
x => Ok(Transformed::No(x)),
})
.unwrap();

let expected = "Explain\
\n Filter: foo = Boolean(true)\
\n TableScan: ?table?";
let actual = format!("{}", plan.display_indent());
assert_eq!(expected.to_string(), actual)
}
}