This is an automated email from the ASF dual-hosted git repository.
alamb pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow-datafusion.git
The following commit(s) were added to refs/heads/master by this push:
new 025345972 The CLI panics when passing an invalid explain query (#4429)
025345972 is described below
commit 02534597248c21a5ccf81737c69f171fc85b46bb
Author: comphead <[email protected]>
AuthorDate: Thu Dec 1 04:16:04 2022 -0800
The CLI panics when passing an invalid explain query (#4429)
* explain plan change panic to Err
* adding test
---
datafusion/core/tests/sql/explain_analyze.rs | 22 ++++++++++++++++++++++
datafusion/expr/src/utils.rs | 22 +++++++++++++---------
2 files changed, 35 insertions(+), 9 deletions(-)
diff --git a/datafusion/core/tests/sql/explain_analyze.rs
b/datafusion/core/tests/sql/explain_analyze.rs
index c4802b43f..5479bf529 100644
--- a/datafusion/core/tests/sql/explain_analyze.rs
+++ b/datafusion/core/tests/sql/explain_analyze.rs
@@ -907,3 +907,25 @@ async fn explain_physical_plan_only() {
]];
assert_eq!(expected, actual);
}
+
+#[tokio::test]
+async fn explain_nested() {
+ async fn test_nested_explain(explain_phy_plan_flag: bool) {
+ let config = SessionConfig::new()
+ .set_bool(OPT_EXPLAIN_PHYSICAL_PLAN_ONLY, explain_phy_plan_flag);
+ let ctx = SessionContext::with_config(config);
+ let sql = "EXPLAIN explain select 1";
+ let plan = ctx.create_logical_plan(sql).unwrap();
+
+ let plan = ctx.create_physical_plan(&plan).await;
+
+ assert!(plan
+ .err()
+ .unwrap()
+ .to_string()
+ .contains("Explain must be root of the plan"));
+ }
+
+ test_nested_explain(true).await;
+ test_nested_explain(false).await;
+}
diff --git a/datafusion/expr/src/utils.rs b/datafusion/expr/src/utils.rs
index c84f4b8d7..71088d2db 100644
--- a/datafusion/expr/src/utils.rs
+++ b/datafusion/expr/src/utils.rs
@@ -563,16 +563,20 @@ pub fn from_plan(
}
LogicalPlan::Explain(_) => {
// Explain should be handled specially in the optimizers;
- // If this assert fails it means some optimizer pass is
+ // If this check cannot pass it means some optimizer pass is
// trying to optimize Explain directly
- assert!(
- expr.is_empty(),
- "Explain can not be created from utils::from_expr"
- );
- assert!(
- inputs.is_empty(),
- "Explain can not be created from utils::from_expr"
- );
+ if expr.is_empty() {
+ return Err(DataFusionError::Plan(
+ "Invalid EXPLAIN command. Expression is empty".to_string(),
+ ));
+ }
+
+ if inputs.is_empty() {
+ return Err(DataFusionError::Plan(
+ "Invalid EXPLAIN command. Inputs are empty".to_string(),
+ ));
+ }
+
Ok(plan.clone())
}
LogicalPlan::EmptyRelation(_)
