Support create table with explicit column definitions
#4194
Conversation
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Thank you @doki23 -- this looks like a nice improvement! I have a few suggestions -- the only thing I think is needed prior to merge is a test for mismatched column count.
We can file a follow up to support specifying column types from a create table as select as a follow on PR
datafusion/sql/src/planner.rs
Outdated
| SetExpr::Values(_) => { | ||
| let schema = self.build_schema(columns)?.to_dfschema_ref()?; | ||
| if schema.fields().len() != input_schema.fields().len() { | ||
| return Err(DataFusionError::Plan("Mismatch between schema and batches".to_string())) |
There was a problem hiding this comment.
I suggest adding test for this situation (incorrect length)
datafusion/sql/src/planner.rs
Outdated
| _ => return Err(DataFusionError::Plan( | ||
| "You can only specify schema when create table with a `values` statement" | ||
| .to_string() | ||
| )) |
There was a problem hiding this comment.
I wonder why not support queries as well? Why only VALUES -- the code you have written should work for a CREATE TABLE AS SELECT as well
There was a problem hiding this comment.
It's because SELECT has its own schema, but surely, it makes sense if someone wants to cast these types. I'll consider your suggestions and add some tests in this situation.
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
I'm going to do it in this pr, here's no need to file a follow up issue @alamb . |
| let ctx = SessionContext::new(); | ||
| register_aggregate_simple_csv(&ctx).await?; | ||
|
|
||
| let sql = "CREATE TABLE my_table(c1 float, c2 double, c3 boolean, c4 varchar) \ |
| let ctx = SessionContext::new(); | ||
| register_aggregate_simple_csv(&ctx).await?; | ||
|
|
||
| let sql = "CREATE TABLE my_table(c1 float, c2 double, c3 boolean, c4 varchar) \ |
alamb
changed the title
create table with explicit column definitions
|
I took the liberty of merging this PR up to master and fixing the clippy issue so we can get it merged. Thanks again @doki23 |
|
BTW I also made a small change to one of the tests to satisfy clippy a2172db |
|
Thanks again @doki23 |
|
Benchmark runs are scheduled for baseline = 88c1201 and contender = 74199d6. 74199d6 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
3 participants
Which issue does this PR close?
Closes #4183
What changes are included in this PR?
Most changes are in planner of creating memory table.
Are these changes tested?
Yes.
Are there any user-facing changes?
Users can create a table with a specified schema with a values query.