-
Notifications
You must be signed in to change notification settings - Fork 563
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix bug to allow parenthesis query after INSERT INTO
#1157
Changes from all commits
7d43ac0
ade716a
fad429e
95241bf
ca8d634
7443e10
5c946a0
1252b10
9738562
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7135,6 +7135,16 @@ impl<'a> Parser<'a> { | |
} else if self.consume_token(&Token::LParen) { | ||
// CTEs are not allowed here, but the parser currently accepts them | ||
let subquery = self.parse_query()?; | ||
|
||
let is_generic = dialect_of!(self is GenericDialect); | ||
if !is_generic { | ||
if let SetExpr::Values(_) = *subquery.body { | ||
return Err(ParserError::ParserError( | ||
"VALUES is not a recognized subquery".to_string(), | ||
)); | ||
} | ||
} | ||
|
||
self.expect_token(&Token::RParen)?; | ||
SetExpr::Query(Box::new(subquery)) | ||
} else if self.parse_keyword(Keyword::VALUES) { | ||
|
@@ -8409,10 +8419,7 @@ impl<'a> Parser<'a> { | |
} else { | ||
let columns = self.parse_parenthesized_column_list(Optional, is_mysql)?; | ||
|
||
let partitioned = self.parse_insert_partition()?; | ||
|
||
// Hive allows you to specify columns after partitions as well if you want. | ||
let after_columns = self.parse_parenthesized_column_list(Optional, false)?; | ||
let (partitioned, after_columns) = self.parse_insert_partition()?; | ||
|
||
let source = Some(Box::new(self.parse_query()?)); | ||
|
||
|
@@ -8492,14 +8499,20 @@ impl<'a> Parser<'a> { | |
} | ||
} | ||
|
||
pub fn parse_insert_partition(&mut self) -> Result<Option<Vec<Expr>>, ParserError> { | ||
pub fn parse_insert_partition( | ||
&mut self, | ||
) -> Result<(Option<Vec<Expr>>, Vec<Ident>), ParserError> { | ||
if self.parse_keyword(Keyword::PARTITION) { | ||
self.expect_token(&Token::LParen)?; | ||
let partition_cols = Some(self.parse_comma_separated(Parser::parse_expr)?); | ||
let partition_cols = self.parse_comma_separated(Parser::parse_expr)?; | ||
self.expect_token(&Token::RParen)?; | ||
Ok(partition_cols) | ||
|
||
// Hive allows you to specify columns after partitions as well if you want. | ||
let after_columns = self.parse_parenthesized_column_list(Optional, false)?; | ||
|
||
Ok((Some(partition_cols), after_columns)) | ||
} else { | ||
Ok(None) | ||
Ok((None, vec![])) | ||
} | ||
} | ||
|
||
|
@@ -10150,7 +10163,35 @@ mod tests { | |
fn test_replace_into_select() { | ||
let sql = r#"REPLACE INTO t1 (a, b, c) (SELECT * FROM t2)"#; | ||
|
||
assert!(Parser::parse_sql(&GenericDialect {}, sql).is_err()); | ||
assert!(Parser::parse_sql(&GenericDialect {}, sql).is_ok()); | ||
} | ||
|
||
#[test] | ||
fn test_insert_into_select() { | ||
let sql = r#"INSERT INTO t1 (a, b, c) (SELECT * FROM t2)"#; | ||
|
||
assert!(Parser::parse_sql(&GenericDialect {}, sql).is_ok()); | ||
} | ||
|
||
#[test] | ||
fn test_insert_into_values() { | ||
let sql = r#"INSERT INTO t1 (a) VALUES(1)"#; | ||
|
||
assert!(Parser::parse_sql(&GenericDialect {}, sql).is_ok()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please put these tests in common with other similar tests, like this: |
||
} | ||
|
||
#[test] | ||
fn test_insert_into_values_wrapped() { | ||
let sql = r#"INSERT INTO t1 (a) (VALUES(1))"#; | ||
|
||
assert!(Parser::parse_sql(&GenericDialect {}, sql).is_ok()); | ||
} | ||
|
||
#[test] | ||
fn test_insert_into_values_wrapped_not_generic() { | ||
let sql = r#"INSERT INTO t1 (a) (VALUES(1))"#; | ||
|
||
assert!(Parser::parse_sql(&MySqlDialect {}, sql).is_err()); | ||
} | ||
|
||
#[test] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -107,6 +107,9 @@ fn parse_insert_values() { | |
} | ||
|
||
verified_stmt("INSERT INTO customer WITH foo AS (SELECT 1) SELECT * FROM foo UNION VALUES (1)"); | ||
|
||
// allow parenthesis query after insert into | ||
verified_stmt("INSERT INTO tbla (cola) (SELECT cola FROM tblb)"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does this syntax mean? Is it the same as INSERT INTO tbla (cola) SELECT cola FROM tblb (no parens?) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe so, well at the least, running an explain statement in Athena (Trino) yields no difference between
And registers both as valid queries. From what I recall, I think you can generally wrap any select statements in parenthesis |
||
} | ||
|
||
#[test] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it is applying a semantic check: https://github.com/sqlparser-rs/sqlparser-rs?tab=readme-ov-file#syntax-vs-semantics
I think any such errors should be generated downstream. Can you please remove this check?