From 8325565c87e970066a7377edf88ab339cf25fd75 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Thu, 27 Apr 2023 16:30:33 -0400 Subject: [PATCH] Extract `LogicalPlan::Drop*` DDL related plan structures into `LogicalPlan::Ddl` --- datafusion/core/src/execution/context.rs | 83 ++++++++++--------- datafusion/core/src/physical_plan/planner.rs | 18 ---- datafusion/expr/src/logical_plan/ddl.rs | 42 ++++++++++ datafusion/expr/src/logical_plan/mod.rs | 11 ++- datafusion/expr/src/logical_plan/plan.rs | 49 +---------- datafusion/expr/src/utils.rs | 2 - .../optimizer/src/common_subexpr_eliminate.rs | 2 - datafusion/proto/src/logical_plan/mod.rs | 4 +- datafusion/sql/src/statement.rs | 24 +++--- 9 files changed, 109 insertions(+), 126 deletions(-) diff --git a/datafusion/core/src/execution/context.rs b/datafusion/core/src/execution/context.rs index bb6d58fb908e..23e2e9eed831 100644 --- a/datafusion/core/src/execution/context.rs +++ b/datafusion/core/src/execution/context.rs @@ -398,46 +398,13 @@ impl SessionContext { self.create_catalog_schema(cmd).await } DdlStatement::CreateCatalog(cmd) => self.create_catalog(cmd).await, + DdlStatement::DropTable(cmd) => self.drop_table(cmd).await, + DdlStatement::DropView(cmd) => self.drop_view(cmd).await, }, - - LogicalPlan::DropTable(DropTable { - name, if_exists, .. - }) => { - let result = self.find_and_deregister(&name, TableType::Base).await; - match (result, if_exists) { - (Ok(true), _) => self.return_empty_dataframe(), - (_, true) => self.return_empty_dataframe(), - (_, _) => Err(DataFusionError::Execution(format!( - "Table '{name}' doesn't exist." - ))), - } - } - - LogicalPlan::DropView(DropView { - name, if_exists, .. - }) => { - let result = self.find_and_deregister(&name, TableType::View).await; - match (result, if_exists) { - (Ok(true), _) => self.return_empty_dataframe(), - (_, true) => self.return_empty_dataframe(), - (_, _) => Err(DataFusionError::Execution(format!( - "View '{name}' doesn't exist." - ))), - } + // TODO what about the other statements (like TransactionStart and TransactionEnd) + LogicalPlan::Statement(Statement::SetVariable(stmt)) => { + self.set_variable(stmt).await } - - LogicalPlan::Statement(Statement::SetVariable(SetVariable { - variable, - value, - .. - })) => { - let mut state = self.state.write(); - state.config.options_mut().set(&variable, &value)?; - drop(state); - - self.return_empty_dataframe() - } - LogicalPlan::DescribeTable(DescribeTable { schema, .. }) => { self.return_describe_table_dataframe(schema).await } @@ -675,6 +642,46 @@ impl SessionContext { } } + async fn drop_table(&self, cmd: DropTable) -> Result { + let DropTable { + name, if_exists, .. + } = cmd; + let result = self.find_and_deregister(&name, TableType::Base).await; + match (result, if_exists) { + (Ok(true), _) => self.return_empty_dataframe(), + (_, true) => self.return_empty_dataframe(), + (_, _) => Err(DataFusionError::Execution(format!( + "Table '{name}' doesn't exist." + ))), + } + } + + async fn drop_view(&self, cmd: DropView) -> Result { + let DropView { + name, if_exists, .. + } = cmd; + let result = self.find_and_deregister(&name, TableType::View).await; + match (result, if_exists) { + (Ok(true), _) => self.return_empty_dataframe(), + (_, true) => self.return_empty_dataframe(), + (_, _) => Err(DataFusionError::Execution(format!( + "View '{name}' doesn't exist." + ))), + } + } + + async fn set_variable(&self, stmt: SetVariable) -> Result { + let SetVariable { + variable, value, .. + } = stmt; + + let mut state = self.state.write(); + state.config.options_mut().set(&variable, &value)?; + drop(state); + + self.return_empty_dataframe() + } + async fn create_custom_table( &self, cmd: &CreateExternalTable, diff --git a/datafusion/core/src/physical_plan/planner.rs b/datafusion/core/src/physical_plan/planner.rs index 782dcf13352c..73aae6ba6784 100644 --- a/datafusion/core/src/physical_plan/planner.rs +++ b/datafusion/core/src/physical_plan/planner.rs @@ -1105,24 +1105,6 @@ impl DefaultPhysicalPlanner { "Unsupported logical plan: Prepare".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::NotImplemented( - "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::NotImplemented( - "Unsupported logical plan: DropView".to_string(), - )) - } LogicalPlan::Dml(_) => { // DataFusion is a read-only query engine, but also a library, so consumers may implement this Err(DataFusionError::NotImplemented( diff --git a/datafusion/expr/src/logical_plan/ddl.rs b/datafusion/expr/src/logical_plan/ddl.rs index a0a3c180bf72..9f87417281d8 100644 --- a/datafusion/expr/src/logical_plan/ddl.rs +++ b/datafusion/expr/src/logical_plan/ddl.rs @@ -41,6 +41,10 @@ pub enum DdlStatement { CreateCatalogSchema(CreateCatalogSchema), /// Creates a new catalog (aka "Database"). CreateCatalog(CreateCatalog), + /// Drops a table. + DropTable(DropTable), + /// Drops a view. + DropView(DropView), } impl DdlStatement { @@ -56,6 +60,8 @@ impl DdlStatement { schema } DdlStatement::CreateCatalog(CreateCatalog { schema, .. }) => schema, + DdlStatement::DropTable(DropTable { schema, .. }) => schema, + DdlStatement::DropView(DropView { schema, .. }) => schema, } } @@ -68,6 +74,8 @@ impl DdlStatement { DdlStatement::CreateView(_) => "CreateView", DdlStatement::CreateCatalogSchema(_) => "CreateCatalogSchema", DdlStatement::CreateCatalog(_) => "CreateCatalog", + DdlStatement::DropTable(_) => "DropTable", + DdlStatement::DropView(_) => "DropView", } } @@ -81,6 +89,8 @@ impl DdlStatement { vec![input] } DdlStatement::CreateView(CreateView { input, .. }) => vec![input], + DdlStatement::DropTable(_) => vec![], + DdlStatement::DropView(_) => vec![], } } @@ -127,6 +137,16 @@ impl DdlStatement { }) => { write!(f, "CreateCatalog: {catalog_name:?}") } + DdlStatement::DropTable(DropTable { + name, if_exists, .. + }) => { + write!(f, "DropTable: {name:?} if not exist:={if_exists}") + } + DdlStatement::DropView(DropView { + name, if_exists, .. + }) => { + write!(f, "DropView: {name:?} if not exist:={if_exists}") + } } } } @@ -231,3 +251,25 @@ pub struct CreateCatalogSchema { /// Empty schema pub schema: DFSchemaRef, } + +/// Drops a table. +#[derive(Clone, PartialEq, Eq, Hash)] +pub struct DropTable { + /// The table name + pub name: OwnedTableReference, + /// If the table exists + pub if_exists: bool, + /// Dummy schema + pub schema: DFSchemaRef, +} + +/// Drops a view. +#[derive(Clone, PartialEq, Eq, Hash)] +pub struct DropView { + /// The view name + pub name: OwnedTableReference, + /// If the view exists + pub if_exists: bool, + /// Dummy schema + pub schema: DFSchemaRef, +} diff --git a/datafusion/expr/src/logical_plan/mod.rs b/datafusion/expr/src/logical_plan/mod.rs index 74588adb2061..6a0249320ee4 100644 --- a/datafusion/expr/src/logical_plan/mod.rs +++ b/datafusion/expr/src/logical_plan/mod.rs @@ -29,15 +29,14 @@ pub use builder::{ }; pub use ddl::{ CreateCatalog, CreateCatalogSchema, CreateExternalTable, CreateMemoryTable, - CreateView, DdlStatement, + CreateView, DdlStatement, DropTable, DropView, }; pub use dml::{DmlStatement, WriteOp}; pub use plan::{ - Aggregate, Analyze, CrossJoin, DescribeTable, Distinct, DropTable, DropView, - EmptyRelation, Explain, Extension, Filter, Join, JoinConstraint, JoinType, Limit, - LogicalPlan, Partitioning, PlanType, Prepare, Projection, Repartition, Sort, - StringifiedPlan, Subquery, SubqueryAlias, TableScan, ToStringifiedPlan, Union, - Unnest, Values, Window, + Aggregate, Analyze, CrossJoin, DescribeTable, Distinct, EmptyRelation, Explain, + Extension, Filter, Join, JoinConstraint, JoinType, Limit, LogicalPlan, Partitioning, + PlanType, Prepare, Projection, Repartition, Sort, StringifiedPlan, Subquery, + SubqueryAlias, TableScan, ToStringifiedPlan, Union, Unnest, Values, Window, }; pub use statement::{ SetVariable, Statement, TransactionAccessMode, TransactionConclusion, TransactionEnd, diff --git a/datafusion/expr/src/logical_plan/plan.rs b/datafusion/expr/src/logical_plan/plan.rs index 1100a40209fe..5be235c89564 100644 --- a/datafusion/expr/src/logical_plan/plan.rs +++ b/datafusion/expr/src/logical_plan/plan.rs @@ -91,10 +91,6 @@ pub enum LogicalPlan { Limit(Limit), /// [`Statement`] Statement(Statement), - /// Drops a table. - DropTable(DropTable), - /// Drops a view. - DropView(DropView), /// Values expression. See /// [Postgres VALUES](https://www.postgresql.org/docs/current/queries-values.html) /// documentation for more details. @@ -148,8 +144,6 @@ impl LogicalPlan { LogicalPlan::Analyze(analyze) => &analyze.schema, LogicalPlan::Extension(extension) => extension.node.schema(), LogicalPlan::Union(Union { schema, .. }) => schema, - LogicalPlan::DropTable(DropTable { schema, .. }) => schema, - LogicalPlan::DropView(DropView { schema, .. }) => schema, LogicalPlan::DescribeTable(DescribeTable { dummy_schema, .. }) => { dummy_schema } @@ -218,10 +212,7 @@ impl LogicalPlan { self.inputs().iter().map(|p| p.schema()).collect() } // return empty - LogicalPlan::Statement(_) - | LogicalPlan::DropTable(_) - | LogicalPlan::DropView(_) - | LogicalPlan::DescribeTable(_) => vec![], + LogicalPlan::Statement(_) | LogicalPlan::DescribeTable(_) => vec![], } } @@ -336,8 +327,6 @@ impl LogicalPlan { | LogicalPlan::SubqueryAlias(_) | LogicalPlan::Limit(_) | LogicalPlan::Statement(_) - | LogicalPlan::DropTable(_) - | LogicalPlan::DropView(_) | LogicalPlan::CrossJoin(_) | LogicalPlan::Analyze(_) | LogicalPlan::Explain(_) @@ -381,8 +370,6 @@ impl LogicalPlan { | LogicalPlan::Statement { .. } | LogicalPlan::EmptyRelation { .. } | LogicalPlan::Values { .. } - | LogicalPlan::DropTable(_) - | LogicalPlan::DropView(_) | LogicalPlan::DescribeTable(_) => vec![], } } @@ -536,8 +523,6 @@ impl LogicalPlan { LogicalPlan::Values(v) => Some(v.values.len()), LogicalPlan::Unnest(_) => None, LogicalPlan::Ddl(_) - | LogicalPlan::DropTable(_) - | LogicalPlan::DropView(_) | LogicalPlan::Explain(_) | LogicalPlan::Analyze(_) | LogicalPlan::Dml(_) @@ -1103,16 +1088,6 @@ impl LogicalPlan { LogicalPlan::Statement(statement) => { write!(f, "{}", statement.display()) } - LogicalPlan::DropTable(DropTable { - name, if_exists, .. - }) => { - write!(f, "DropTable: {name:?} if not exist:={if_exists}") - } - LogicalPlan::DropView(DropView { - name, if_exists, .. - }) => { - write!(f, "DropView: {name:?} if not exist:={if_exists}") - } LogicalPlan::Distinct(Distinct { .. }) => { write!(f, "Distinct:") } @@ -1223,28 +1198,6 @@ pub enum JoinConstraint { Using, } -/// Drops a table. -#[derive(Clone, PartialEq, Eq, Hash)] -pub struct DropTable { - /// The table name - pub name: OwnedTableReference, - /// If the table exists - pub if_exists: bool, - /// Dummy schema - pub schema: DFSchemaRef, -} - -/// Drops a view. -#[derive(Clone, PartialEq, Eq, Hash)] -pub struct DropView { - /// The view name - pub name: OwnedTableReference, - /// If the view exists - pub if_exists: bool, - /// Dummy schema - pub schema: DFSchemaRef, -} - /// Produces no rows: An empty relation with an empty schema #[derive(Clone, PartialEq, Eq, Hash)] pub struct EmptyRelation { diff --git a/datafusion/expr/src/utils.rs b/datafusion/expr/src/utils.rs index 373e8b72ef44..7babd659e7ef 100644 --- a/datafusion/expr/src/utils.rs +++ b/datafusion/expr/src/utils.rs @@ -915,8 +915,6 @@ pub fn from_plan( } LogicalPlan::EmptyRelation(_) | LogicalPlan::Ddl(_) - | LogicalPlan::DropTable(_) - | LogicalPlan::DropView(_) | LogicalPlan::Statement(_) => { // All of these plan types have no inputs / exprs so should not be called assert!(expr.is_empty(), "{plan:?} should have no exprs"); diff --git a/datafusion/optimizer/src/common_subexpr_eliminate.rs b/datafusion/optimizer/src/common_subexpr_eliminate.rs index 7e53c927c451..9f97de04a428 100644 --- a/datafusion/optimizer/src/common_subexpr_eliminate.rs +++ b/datafusion/optimizer/src/common_subexpr_eliminate.rs @@ -230,8 +230,6 @@ impl OptimizerRule for CommonSubexprEliminate { | LogicalPlan::Ddl(_) | LogicalPlan::Explain(_) | LogicalPlan::Analyze(_) - | LogicalPlan::DropTable(_) - | LogicalPlan::DropView(_) | LogicalPlan::Statement(_) | LogicalPlan::DescribeTable(_) | LogicalPlan::Distinct(_) diff --git a/datafusion/proto/src/logical_plan/mod.rs b/datafusion/proto/src/logical_plan/mod.rs index 82a1af8843e6..709c39177208 100644 --- a/datafusion/proto/src/logical_plan/mod.rs +++ b/datafusion/proto/src/logical_plan/mod.rs @@ -1366,10 +1366,10 @@ impl AsLogicalPlan for LogicalPlanNode { LogicalPlan::Ddl(DdlStatement::CreateMemoryTable(_)) => Err(proto_error( "LogicalPlan serde is not yet implemented for CreateMemoryTable", )), - LogicalPlan::DropTable(_) => Err(proto_error( + LogicalPlan::Ddl(DdlStatement::DropTable(_)) => Err(proto_error( "LogicalPlan serde is not yet implemented for DropTable", )), - LogicalPlan::DropView(_) => Err(proto_error( + LogicalPlan::Ddl(DdlStatement::DropView(_)) => Err(proto_error( "LogicalPlan serde is not yet implemented for DropView", )), LogicalPlan::Statement(_) => Err(proto_error( diff --git a/datafusion/sql/src/statement.rs b/datafusion/sql/src/statement.rs index a84118d0423d..b48261dcc406 100644 --- a/datafusion/sql/src/statement.rs +++ b/datafusion/sql/src/statement.rs @@ -258,16 +258,20 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { }?; match object_type { - ObjectType::Table => Ok(LogicalPlan::DropTable(DropTable { - name, - if_exists, - schema: DFSchemaRef::new(DFSchema::empty()), - })), - ObjectType::View => Ok(LogicalPlan::DropView(DropView { - name, - if_exists, - schema: DFSchemaRef::new(DFSchema::empty()), - })), + ObjectType::Table => { + Ok(LogicalPlan::Ddl(DdlStatement::DropTable(DropTable { + name, + if_exists, + schema: DFSchemaRef::new(DFSchema::empty()), + }))) + } + ObjectType::View => { + Ok(LogicalPlan::Ddl(DdlStatement::DropView(DropView { + name, + if_exists, + schema: DFSchemaRef::new(DFSchema::empty()), + }))) + } _ => Err(DataFusionError::NotImplemented( "Only `DROP TABLE/VIEW ...` statement is supported currently" .to_string(),