From 70ea37e8f4b91d945481bacfe88048064f7651a2 Mon Sep 17 00:00:00 2001 From: hhj Date: Sat, 2 Dec 2023 18:17:39 +0800 Subject: [PATCH 1/2] fix transforming LogicalPlan::Explain use TreeNode::transform fails --- datafusion/expr/src/logical_plan/plan.rs | 65 +++++++++++++++++++----- 1 file changed, 53 insertions(+), 12 deletions(-) diff --git a/datafusion/expr/src/logical_plan/plan.rs b/datafusion/expr/src/logical_plan/plan.rs index ea7a48d2c4f4..ee4c73bed4d8 100644 --- a/datafusion/expr/src/logical_plan/plan.rs +++ b/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!( + 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, .. @@ -3076,4 +3077,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) + } } From 3dfcade03a9668620978b56670eaf26ddd3e446c Mon Sep 17 00:00:00 2001 From: Huaijin Date: Sun, 3 Dec 2023 21:01:44 +0800 Subject: [PATCH 2/2] Update datafusion/expr/src/logical_plan/plan.rs Co-authored-by: Andrew Lamb --- datafusion/expr/src/logical_plan/plan.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/datafusion/expr/src/logical_plan/plan.rs b/datafusion/expr/src/logical_plan/plan.rs index ee4c73bed4d8..9bb47c7da058 100644 --- a/datafusion/expr/src/logical_plan/plan.rs +++ b/datafusion/expr/src/logical_plan/plan.rs @@ -878,7 +878,6 @@ impl LogicalPlan { })) } LogicalPlan::Explain(e) => { - // Explain should be handled specially in the optimizers; assert!( expr.is_empty(), "Invalid EXPLAIN command. Expression should empty"