iffyio commented on code in PR #1520:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1520#discussion_r1839413182
##########
src/dialect/duckdb.rs:
##########
@@ -66,4 +66,9 @@ impl Dialect for DuckDbDialect {
fn supports_explain_with_utility_options(&self) -> bool {
true
}
+
+ /// See DuckDB
<https://duckdb.org/docs/sql/statements/load_and_install.html#load>
+ fn supports_load_extension(&self) -> bool {
Review Comment:
It looks like the Generic dialect should also support this one per the
previous behavior?
##########
src/parser/mod.rs:
##########
@@ -27,16 +27,15 @@ use core::{
use log::debug;
-use recursion::RecursionCounter;
-use IsLateral::*;
-use IsOptional::*;
-
use crate::ast::helpers::stmt_create_table::{CreateTableBuilder,
CreateTableConfiguration};
use crate::ast::Statement::CreatePolicy;
use crate::ast::*;
use crate::dialect::*;
use crate::keywords::{Keyword, ALL_KEYWORDS};
use crate::tokenizer::*;
+use recursion::RecursionCounter;
+use IsLateral::*;
+use IsOptional::*;
Review Comment:
where these autoformatted?
##########
src/parser/mod.rs:
##########
@@ -12042,10 +12054,35 @@ impl<'a> Parser<'a> {
Ok(Statement::Install { extension_name })
}
- /// `LOAD [extension_name]`
+ /// Parse a SQL LOAD statement
pub fn parse_load(&mut self) -> Result<Statement, ParserError> {
- let extension_name = self.parse_identifier(false)?;
- Ok(Statement::Load { extension_name })
+ if self.dialect.supports_load_extension() {
+ let extension_name = self.parse_identifier(false)?;
+ Ok(Statement::Load { extension_name })
+ } else if self.parse_keyword(Keyword::DATA) &&
self.dialect.supports_load_data() {
+ let local =
self.parse_one_of_keywords(&[Keyword::LOCAL]).is_some();
+ self.expect_keyword(Keyword::INPATH)?;
+ let inpath = self.parse_literal_string()?;
+ let overwrite =
self.parse_one_of_keywords(&[Keyword::OVERWRITE]).is_some();
+ self.expect_keyword(Keyword::INTO)?;
+ self.expect_keyword(Keyword::TABLE)?;
+ let table_name = self.parse_object_name(false)?;
+ let partitioned = self.parse_insert_partition()?;
+ let table_format = self.parse_load_data_table_format()?;
+ Ok(Statement::LoadData {
+ local,
+ inpath,
+ overwrite,
+ table_name,
+ partitioned,
+ table_format,
+ })
+ } else {
+ self.expected(
+ "dialect supports `LOAD DATA` or `LOAD extension` to parse
`LOAD` statements",
Review Comment:
```suggestion
" DATA` or an extension name after `LOAD`",
```
##########
tests/sqlparser_common.rs:
##########
@@ -11516,13 +11516,199 @@ fn parse_notify_channel() {
dialects.parse_sql_statements(sql).unwrap_err(),
ParserError::ParserError("Expected: an SQL statement, found:
NOTIFY".to_string())
);
- assert_eq!(
- dialects.parse_sql_statements(sql).unwrap_err(),
- ParserError::ParserError("Expected: an SQL statement, found:
NOTIFY".to_string())
- );
}
}
+#[test]
+fn parse_load_data() {
+ let dialects = all_dialects_where(|d| d.supports_load_data());
+ let only_supports_load_extension_dialects =
+ all_dialects_where(|d| !d.supports_load_data() &&
d.supports_load_extension());
+ let not_supports_load_dialects =
+ all_dialects_where(|d| !d.supports_load_data() &&
!d.supports_load_extension());
Review Comment:
if the function is testing the `load_data` feature, it ideally shouldnt care
about `supports_load_extension`? e.g. `all_dialects_where(|d|
!d.supports_load_data() && d.supports_load_extension());` I imagine should be
the same as `all_dialects_where(|d| !d.supports_load_data());`? given that
`supports_load_extension` should be a different test function
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
