You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by al...@apache.org on 2023/04/28 14:10:30 UTC

[arrow-datafusion] branch main updated: Extract `LogicalPlan::Drop*` DDL related plan structures into `LogicalPlan::Ddl` (#6144)

This is an automated email from the ASF dual-hosted git repository.

alamb pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-datafusion.git


The following commit(s) were added to refs/heads/main by this push:
     new 2b331828e1 Extract `LogicalPlan::Drop*` DDL related plan structures into `LogicalPlan::Ddl` (#6144)
2b331828e1 is described below

commit 2b331828e136dd0a389e3744ec45beacff3ae409
Author: Andrew Lamb <an...@nerdnetworks.org>
AuthorDate: Fri Apr 28 10:10:23 2023 -0400

    Extract `LogicalPlan::Drop*` DDL related plan structures into `LogicalPlan::Ddl` (#6144)
---
 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 dce8a1c424..ee8cfe5dc5 100644
--- a/datafusion/core/src/execution/context.rs
+++ b/datafusion/core/src/execution/context.rs
@@ -381,46 +381,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."
-                    ))),
-                }
+            // TODO what about the other statements (like TransactionStart and TransactionEnd)
+            LogicalPlan::Statement(Statement::SetVariable(stmt)) => {
+                self.set_variable(stmt).await
             }
-
-            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."
-                    ))),
-                }
-            }
-
-            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
             }
@@ -658,6 +625,46 @@ impl SessionContext {
         }
     }
 
+    async fn drop_table(&self, cmd: DropTable) -> Result<DataFrame> {
+        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<DataFrame> {
+        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<DataFrame> {
+        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 7f68d5a39a..087545a644 100644
--- a/datafusion/core/src/physical_plan/planner.rs
+++ b/datafusion/core/src/physical_plan/planner.rs
@@ -1122,24 +1122,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 a0a3c180bf..9f87417281 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 74588adb20..6a0249320e 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 1100a40209..5be235c895 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 373e8b72ef..7babd659e7 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 7e53c927c4..9f97de04a4 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 82a1af8843..709c391772 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 a84118d042..b48261dcc4 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(),