Add COPY .. TO .. syntax support
#6355
Conversation
github-actions
bot
added
core
| struct RelationVisitor<'a>(&'a mut hashbrown::HashSet<ObjectName>); | ||
| struct RelationVisitor<'a>(&'a mut hashbrown::HashSet<ObjectName>); | ||
|
|
||
| impl<'a> RelationVisitor<'a> { |
There was a problem hiding this comment.
this simply pulls the RelationVisitor up a level so it can be used for both DFStatement::DescribeTableStmt and DFStatement::CopyTo
|
|
||
| # Copy from table | ||
| statement error DataFusion error: This feature is not implemented: `COPY \.\. TO \.\.` statement is yet supported | ||
| COPY source_table to '/tmp/table.parquet'; |
There was a problem hiding this comment.
This is what I really want -- it is so close
alamb
requested review from
crepererum and
tustvold
and removed request for
crepererum
|
Sure @alamb I'll review today |
|
|
||
| impl<'a> RelationVisitor<'a> { | ||
| /// Record that `relation` was used in this statement | ||
| fn insert(&mut self, relation: &ObjectName) { |
datafusion/sql/src/parser.rs
Outdated
| Token::Number(ref n, l) => match n.parse() { | ||
| Ok(n) => Ok(Value::Number(n, l)), | ||
| Err(e) => parser_err!(format!("Could not parse '{n}' as number: {e}")), | ||
| }, |
There was a problem hiding this comment.
Just a question: what is these lines of code trying to do?
I might be wrong about this but it seems that n in Token::number(ref n, l) is already guaranteed by the tokenizer to be a number.
There was a problem hiding this comment.
I will admit to simply copy/pasting this from sqlparser code. I will change this to "internal error" to signal that it is not expected
There was a problem hiding this comment.
I tried to make it an internal error, but the type needs to be a ParserError -- I added some clarifying comments i 17c6960
|
Btw, this PR is fine and working as is. Most of my suggested change is typo fix. |
|
Thanks for the suggestions @aprimadi -- I just ran out of time yesterday to apply them. I will do so today! |
Co-authored-by: Armin Primadi <aprimadi@gmail.com>
|
Thanks again for the reviews and suggestions @aprimadi |
|
Thank you! |
Which issue does this PR close?
Closes #5988
Rationale for this change
I am working on #5654 (mostly so I can better benchmark datafusion) and part of the code needed for this is parser support.
What changes are included in this PR?
COPY .. TO ...statements based on duckdb syntax: https://duckdb.org/docs/sql/statements/copyAre these changes tested?
Yes
Are there any user-facing changes?
New syntax is parsed, but throws a
"COPY .. TO .. statement is yet supported"when used