feat: allow datafusion-cli to accept multiple statements
#7138
Conversation
There was a problem hiding this comment.
Thank you! This feature is very useful.
DataFusion CLI v28.0.0
❯ select 1; select 2;
+----------+
| Int64(1) |
+----------+
| 1 |
+----------+
1 row in set. Query took 0.060 seconds.
+----------+
| Int64(2) |
+----------+
| 2 |
+----------+
1 row in set. Query took 0.061 seconds.
I suggest also add an empty line between results to make it look a little bit better
datafusion-cli/src/exec.rs
Outdated
| } | ||
| _ => ctx.execute_logical_plan(plan).await?, | ||
| }; | ||
| let statements = DFParser::parse_sql(&sql)?; |
There was a problem hiding this comment.
This will use the default dialect, we can use parse_sql_with_dialect() and pass the dialect from the current context into it.
There was a problem hiding this comment.
I am not sure what you mean by the current context -- which one are you thinking of?
There was a problem hiding this comment.
DataFusion CLI v28.0.0
❯ set datafusion.sql_parser.dialect = 'Postgres';
I'm referring to this option, it can be found in ctx.state()..., and be used to parse statements
There was a problem hiding this comment.
Done. Please let me know if there are any errors or omissions.
There was a problem hiding this comment.
Looks good to me, thank you!
There was a problem hiding this comment.
Thank you @NiwakaDev -- this is a great contribution
Thank you @2010YOUY01 for the review
datafusion-cli/src/exec.rs
Outdated
| print_options.print_batches(&results, now)?; | ||
| let results = df.collect().await?; | ||
| print_options.print_batches(&results, now)?; | ||
| println!(); |
There was a problem hiding this comment.
The println!(); here I would suggest to put in the code of fn print_timing_info(). Because it will affect the output of --quiet mode.
suggestion:
// add '\n' at the end of `println!`
fn print_timing_info(row_count: usize, now: Instant) {
println!(
"{} {} in set. Query took {:.3} seconds.\n",
row_count,
if row_count == 1 { "row" } else { "rows" },
now.elapsed().as_secs_f64()
);
}There was a problem hiding this comment.
The current --quiet mode output is as follows:
./target/debug/datafusion-cli --command 'select 1;select 2;' -q --format json
[{"Int64(1)":1}]
[{"Int64(2)":2}]There was a problem hiding this comment.
Done. Please let me know if there are any errors or omissions.
|
🤔 something happened to make CI fail. Perhaps that is what @r4ntix 's comments are about |
|
Right now I guess that the integration tests pass, and I added a new integration test to them. |
| #[case::exec_multiple_statements( | ||
| ["--command", "select 1; select 2;", "--format", "json", "-q"], | ||
| "[{\"Int64(1)\":1}]\n[{\"Int64(2)\":2}]\n" | ||
| )] |
There was a problem hiding this comment.
This is a new integration test.
There was a problem hiding this comment.
❤️ Thanks again @NiwakaDev -- I like it
|
Thanks again everyone -- this is a great contribution @NiwakaDev 🦾 |
Which issue does this PR close?
Closes #6793.
Rationale for this change
What changes are included in this PR?
This PR allows
datafusion-clito accept multiple statements.Are these changes tested?
Are there any user-facing changes?