You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2023/04/27 20:42:16 UTC

[GitHub] [arrow-datafusion] alamb opened a new pull request, #6144: Alamb/complete ddl

alamb opened a new pull request, #6144:
URL: https://github.com/apache/arrow-datafusion/pull/6144

   Extract `LogicalPlan::Drop*` DDL related plan structures into `LogicalPlan::Ddl`
   
   # Which issue does this PR close?
   
   Closes https://github.com/apache/arrow-datafusion/issues/1281
   
   # Rationale for this change
   
   While reviewing https://github.com/apache/arrow-datafusion/pull/6096 from @jaylmiller  I noticed that the LogicalPlan was getting quite large for the various Catalog manipulation statements
   
   # What changes are included in this PR?
   
   Move `LogicalPlan::Drop*` variants into `LogicalPlan::Ddl`  to keep them more isolated
   
   # Are these changes tested?
   
   Covered by existing tests
   
   # Are there any user-facing changes?
   Yes, this is an API change -- some LogicalPlan variants are now wrapped up another layer of struct
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] alamb merged pull request #6144: Complete Extracting LogicalPlan::Drop* DDL related plan structures into LogicalPlan::Ddl

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #6144:
URL: https://github.com/apache/arrow-datafusion/pull/6144


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #6144: Complete Extracting LogicalPlan::Drop* DDL related plan structures into LogicalPlan::Ddl

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #6144:
URL: https://github.com/apache/arrow-datafusion/pull/6144#discussion_r1179663400


##########
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)

Review Comment:
   I will address this in a follow on PR



##########
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;

Review Comment:
   I moved these into their own functions for consistency



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org