Add Create Schema functionality in SQL #1959

matthewmturner · 2022-03-08T18:06:37Z

Which issue does this PR close?

Task 1 for #1877

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

matthewmturner · 2022-03-09T16:27:17Z

I'm working on this and im wondering why register_schema isnt a trait method for CatalogProvider. For example register_table is part of SchemaProvider trait and i consider a schema to be the equivalent for a catalog.

matthewmturner · 2022-03-10T20:15:52Z

This has surprisingly been a bit of a pain. Currently having issues with getting the schema registration to work. At the moment its not clear to me what the issue is.

matthewmturner · 2022-03-10T21:37:03Z

datafusion/src/execution/context.rs

+                            "test".into(),
+                            Arc::new(DataFrameImpl::new(self.state.clone(), &plan)),
+                        )?;
+                        let schem_reg_res = catalog.register_schema(&schema_name, schema);


It's not clear to me why registering schema here isnt working. I reregister the catalog below just in case but still has no effect. I wasnt expecting to have to reregister. Still digging deep into the implementations to get better idea whats going on.

(i know theres a lot of fluff right now that im using for debugging purposes)

matthewmturner · 2022-03-14T16:11:48Z

Got this working. Just need to resolve conflicts.

@alamb do you have preference on whether to leave this PR as is or if i can also add CREATE CATALOG on here? I see sql-parser has a CreateDatabase statement already that i was thinking i could use. Or do you think that would get confusing? i.e. CREATE DATABASE would call ctx.register_catalog and then be visible as a catalog.

matthewmturner · 2022-03-14T17:24:01Z

Ah need to do ballista update still

alamb · 2022-03-14T21:41:19Z

@alamb do you have preference on whether to leave this PR as is or if i can also add CREATE CATALOG on here?

I suggest we do it in a follow on PR to keep this one smaller.

I see sql-parser has a CreateDatabase statement already that i was thinking i could use. Or do you think that would get confusing? i.e. CREATE DATABASE would call ctx.register_catalog and then be visible as a catalog

seems reasonable

(sorry for the delayed replies -- I am only inline intermittently this week)

matthewmturner · 2022-03-16T13:12:36Z

@alamb i believe this is ready now

doki23 · 2022-03-17T02:14:10Z

datafusion/src/catalog/mod.rs

+                table: parts[2],
+            },
+            _ => Self::Bare { table: s },
+        }
    }


👍🏻! It's helpful to me!

doki23

LGTM

alamb

Looks good to me -- thanks @doki23 and @matthewmturner !

alamb · 2022-03-17T12:36:29Z

datafusion/src/execution/context.rs

@@ -2907,6 +2947,29 @@ mod tests {
        assert_eq!(Weak::strong_count(&catalog_weak), 0);
    }

+    #[tokio::test]
+    async fn sql_create_schema() -> Result<()> {
+        // the information schema used to introduce cyclic Arcs


not sure what this comment means

oops that may have been straggling comment from a copy paste, sry about that. i can remove in subsequent PR

alamb · 2022-03-17T12:37:28Z

datafusion/src/execution/context.rs

+                if_not_exists,
+                ..
+            }) => {
+                // sqlparser doesnt accept database / catalog as parameter to CREATE SCHEMA


maybe worth a ticket to sqlparser to support this? Or maybe just a PR :)

alamb · 2022-03-17T12:39:00Z

datafusion/src/execution/context.rs

+
+                let schema = catalog.schema(&schema_name);
+
+                match (if_not_exists, schema) {


github-actions bot added datafusion Changes in the datafusion crate sql SQL Planner labels Mar 8, 2022

matthewmturner commented Mar 10, 2022

View reviewed changes

matthewmturner mentioned this pull request Mar 12, 2022

Improved schema and catalog support from SQL #1877

Closed

3 tasks

matthewmturner force-pushed the add_create_schema branch from 4e44f4d to ff3aabb Compare March 14, 2022 16:16

matthewmturner marked this pull request as ready for review March 14, 2022 16:21

github-actions bot added the ballista label Mar 15, 2022

matthewmturner added 6 commits March 15, 2022 10:47

Start adding Create Schema

1448bce

Refactor catalog and schema providers

bec0c7a

Working on catalog and schema registration within ExecutionContext

0f0d150

Create schema SQL working and added test

73d01d5

Fix tests

bc65517

Ballista updates

877ccd6

matthewmturner force-pushed the add_create_schema branch from 0ec8628 to 877ccd6 Compare March 15, 2022 14:48

Rebase remainder

71d5a46

yjshen added enhancement New feature or request api change Changes the API exposed to users of the crate labels Mar 16, 2022

doki23 reviewed Mar 17, 2022

View reviewed changes

doki23 approved these changes Mar 17, 2022

View reviewed changes

alamb approved these changes Mar 17, 2022

View reviewed changes

alamb merged commit 3ec0d55 into apache:master Mar 17, 2022

xudong963 mentioned this pull request Mar 17, 2022

fix: replace ExecutionContex and ExecutionConfig with SessionContext and SessionConfig #2030

Merged

matthewmturner mentioned this pull request Mar 17, 2022

Support DATABASE or CATALOG parameter as part of CREATE SCHEMA apache/datafusion-sqlparser-rs#440

Open

alamb mentioned this pull request Mar 21, 2022

panic in register_catalog if default catalog not named "datafusion" and information schema enabled #2049

Closed

This was referenced Mar 21, 2022

Add test in register_catalog and change to use named symbolic constants #2050

Merged

Allow CatalogProvider::register_catalog to return an error #2052

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Create Schema functionality in SQL #1959

Add Create Schema functionality in SQL #1959

matthewmturner commented Mar 8, 2022

matthewmturner commented Mar 9, 2022

matthewmturner commented Mar 10, 2022

matthewmturner Mar 10, 2022

matthewmturner Mar 10, 2022

matthewmturner commented Mar 14, 2022

matthewmturner commented Mar 14, 2022

alamb commented Mar 14, 2022

matthewmturner commented Mar 16, 2022

doki23 Mar 17, 2022 •

edited

Loading

doki23 left a comment

alamb left a comment

alamb Mar 17, 2022

matthewmturner Mar 17, 2022

alamb Mar 17, 2022

alamb Mar 17, 2022


		let schema = catalog.schema(&schema_name);

		match (if_not_exists, schema) {

Add Create Schema functionality in SQL #1959

Add Create Schema functionality in SQL #1959

Conversation

matthewmturner commented Mar 8, 2022

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

matthewmturner commented Mar 9, 2022

matthewmturner commented Mar 10, 2022

matthewmturner Mar 10, 2022

Choose a reason for hiding this comment

matthewmturner Mar 10, 2022

Choose a reason for hiding this comment

matthewmturner commented Mar 14, 2022

matthewmturner commented Mar 14, 2022

alamb commented Mar 14, 2022

matthewmturner commented Mar 16, 2022

doki23 Mar 17, 2022 • edited Loading

Choose a reason for hiding this comment

doki23 left a comment

Choose a reason for hiding this comment

alamb left a comment

Choose a reason for hiding this comment

alamb Mar 17, 2022

Choose a reason for hiding this comment

matthewmturner Mar 17, 2022

Choose a reason for hiding this comment

alamb Mar 17, 2022

Choose a reason for hiding this comment

alamb Mar 17, 2022

Choose a reason for hiding this comment

doki23 Mar 17, 2022 •

edited

Loading