Skip to content

Commit

Permalink
fix: return error for unsupported SQL (#3933)
Browse files Browse the repository at this point in the history 
* fix: return error for  unsupported SQL

* add some test

* fix some test & cargo fmt
  • Loading branch information
@Kikkon
Kikkon authored Oct 25, 2022
1 parent ae13455 commit 7cba758
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 61 deletions.
92 changes: 37 additions & 55 deletions datafusion/core/src/datasource/view.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -493,67 +493,49 @@ mod tests {
let view_sql = "CREATE VIEW xyz AS SELECT * FROM abc";
session_ctx.sql(view_sql).await?.collect().await?;

let results = session_ctx
let plan = session_ctx
.sql("EXPLAIN CREATE VIEW xyz AS SELECT * FROM abc")
.await?
.collect()
.await?;

let expected = vec![
"+---------------+-----------------------------------------------------------+",
"| plan_type | plan |",
"+---------------+-----------------------------------------------------------+",
"| logical_plan | CreateView: \"xyz\" |",
"| | Projection: abc.column1, abc.column2, abc.column3 |",
"| | TableScan: abc projection=[column1, column2, column3] |",
"| physical_plan | EmptyExec: produce_one_row=false |",
"| | |",
"+---------------+-----------------------------------------------------------+",
];

assert_batches_eq!(expected, &results);

let results = session_ctx
.to_logical_plan()
.unwrap();
let plan = session_ctx.optimize(&plan).unwrap();
let actual = format!("{}", plan.display_indent());
let expected = "\
Explain\
\n CreateView: \"xyz\"\
\n Projection: abc.column1, abc.column2, abc.column3\
\n TableScan: abc projection=[column1, column2, column3]";
assert_eq!(expected, actual);

let plan = session_ctx
.sql("EXPLAIN CREATE VIEW xyz AS SELECT * FROM abc WHERE column2 = 5")
.await?
.collect()
.await?;

let expected = vec![
"+---------------+-------------------------------------------------------------+",
"| plan_type | plan |",
"+---------------+-------------------------------------------------------------+",
"| logical_plan | CreateView: \"xyz\" |",
"| | Projection: abc.column1, abc.column2, abc.column3 |",
"| | Filter: abc.column2 = Int64(5) |",
"| | TableScan: abc projection=[column1, column2, column3] |",
"| physical_plan | EmptyExec: produce_one_row=false |",
"| | |",
"+---------------+-------------------------------------------------------------+",
];

assert_batches_eq!(expected, &results);

let results = session_ctx
.to_logical_plan()
.unwrap();
let plan = session_ctx.optimize(&plan).unwrap();
let actual = format!("{}", plan.display_indent());
let expected = "\
Explain\
\n CreateView: \"xyz\"\
\n Projection: abc.column1, abc.column2, abc.column3\
\n Filter: abc.column2 = Int64(5)\
\n TableScan: abc projection=[column1, column2, column3]";
assert_eq!(expected, actual);

let plan = session_ctx
.sql("EXPLAIN CREATE VIEW xyz AS SELECT column1, column2 FROM abc WHERE column2 = 5")
.await?
.collect()
.await?;

let expected = vec![
"+---------------+----------------------------------------------------+",
"| plan_type | plan |",
"+---------------+----------------------------------------------------+",
"| logical_plan | CreateView: \"xyz\" |",
"| | Projection: abc.column1, abc.column2 |",
"| | Filter: abc.column2 = Int64(5) |",
"| | TableScan: abc projection=[column1, column2] |",
"| physical_plan | EmptyExec: produce_one_row=false |",
"| | |",
"+---------------+----------------------------------------------------+",
];

assert_batches_eq!(expected, &results);
.to_logical_plan()
.unwrap();
let plan = session_ctx.optimize(&plan).unwrap();
let actual = format!("{}", plan.display_indent());
let expected = "\
Explain\
\n CreateView: \"xyz\"\
\n Projection: abc.column1, abc.column2\
\n Filter: abc.column2 = Int64(5)\
\n TableScan: abc projection=[column1, column2]";
assert_eq!(expected, actual);

Ok(())
}
Expand Down
41 changes: 35 additions & 6 deletions datafusion/core/src/physical_plan/planner.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1051,12 +1051,41 @@ impl DefaultPhysicalPlanner {
"Unsupported logical plan: CreateCatalog".to_string(),
))
}
| LogicalPlan::CreateMemoryTable(_) | LogicalPlan::DropTable(_) | LogicalPlan::DropView(_) | LogicalPlan::CreateView(_) => {
// Create a dummy exec.
Ok(Arc::new(EmptyExec::new(
false,
SchemaRef::new(Schema::empty()),
)))
LogicalPlan::CreateMemoryTable(_) => {
// There is no default plan for "CREATE MEMORY TABLE".
// It must be handled at a higher level (so
// that the schema can be registered with
// the context)
Err(DataFusionError::Internal(
"Unsupported logical plan: CreateMemoryTable".to_string(),
))
}
LogicalPlan::DropTable(_) => {
// There is no default plan for "DROP TABLE".
// It must be handled at a higher level (so
// that the schema can be registered with
// the context)
Err(DataFusionError::Internal(
"Unsupported logical plan: DropTable".to_string(),
))
}
LogicalPlan::DropView(_) => {
// There is no default plan for "DROP VIEW".
// It must be handled at a higher level (so
// that the schema can be registered with
// the context)
Err(DataFusionError::Internal(
"Unsupported logical plan: DropView".to_string(),
))
}
LogicalPlan::CreateView(_) => {
// There is no default plan for "CREATE VIEW".
// It must be handled at a higher level (so
// that the schema can be registered with
// the context)
Err(DataFusionError::Internal(
"Unsupported logical plan: CreateView".to_string(),
))
}
LogicalPlan::Explain(_) => Err(DataFusionError::Internal(
"Unsupported logical plan: Explain must be root of the plan".to_string(),
Expand Down
37 changes: 37 additions & 0 deletions datafusion/core/tests/sql/errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,3 +136,40 @@ async fn invalid_qualified_table_references() -> Result<()> {
}
Ok(())
}

#[tokio::test]
async fn unsupported_sql_returns_error() -> Result<()> {
let ctx = SessionContext::new();
register_aggregate_csv(&ctx).await?;
// create view
let sql = "create view test_view as select * from aggregate_test_100";
let plan = ctx.create_logical_plan(sql);
let physical_plan = ctx.create_physical_plan(&plan.unwrap()).await;
assert!(physical_plan.is_err());
assert_eq!(
format!("{}", physical_plan.unwrap_err()),
"Internal error: Unsupported logical plan: CreateView. \
This was likely caused by a bug in DataFusion's code and we would welcome that you file an bug report in our issue tracker"
);
// // drop view
let sql = "drop view test_view";
let plan = ctx.create_logical_plan(sql);
let physical_plan = ctx.create_physical_plan(&plan.unwrap()).await;
assert!(physical_plan.is_err());
assert_eq!(
format!("{}", physical_plan.unwrap_err()),
"Internal error: Unsupported logical plan: DropView. \
This was likely caused by a bug in DataFusion's code and we would welcome that you file an bug report in our issue tracker"
);
// // drop table
let sql = "drop table aggregate_test_100";
let plan = ctx.create_logical_plan(sql);
let physical_plan = ctx.create_physical_plan(&plan.unwrap()).await;
assert!(physical_plan.is_err());
assert_eq!(
format!("{}", physical_plan.unwrap_err()),
"Internal error: Unsupported logical plan: DropTable. \
This was likely caused by a bug in DataFusion's code and we would welcome that you file an bug report in our issue tracker"
);
Ok(())
}

0 comments on commit 7cba758

Please sign in to comment.