feat: add support for duckdb style "FROM <tbl>" statements #1144
Conversation
| @@ -6722,6 +6726,8 @@ impl<'a> Parser<'a> { | |||
| SetExpr::Values(self.parse_values(is_mysql)?) | |||
| } else if self.parse_keyword(Keyword::TABLE) { | |||
| SetExpr::Table(Box::new(self.parse_as_table()?)) | |||
| } else if self.parse_keyword(Keyword::FROM) { | |||
There was a problem hiding this comment.
I have no idea why, but adding this branch causes SO in the recursion tests such as parse_deeply_nested_parens_hits_recursion_limits
There was a problem hiding this comment.
even if the branch is never met, the presence of it is causing SO.
There was a problem hiding this comment.
maybe @alamb, @andygrove @nickolay knows why adding this condition causes a SO.
There was a problem hiding this comment.
This branch probably makes the stack frame size increase in debug builds as it adds some new local variables. I think we could fix the test by decreasing the recursion limit
There was a problem hiding this comment.
Thank you for this contribution @universalmind303 -- I left some comments that hopefully will help us get this PR good to go
| @@ -6722,6 +6726,8 @@ impl<'a> Parser<'a> { | |||
| SetExpr::Values(self.parse_values(is_mysql)?) | |||
| } else if self.parse_keyword(Keyword::TABLE) { | |||
| SetExpr::Table(Box::new(self.parse_as_table()?)) | |||
| } else if self.parse_keyword(Keyword::FROM) { | |||
There was a problem hiding this comment.
This branch probably makes the stack frame size increase in debug builds as it adds some new local variables. I think we could fix the test by decreasing the recursion limit
| #[test] | ||
| fn test_duckdb_from_statement_with_filter_and_select() { | ||
| let stmt = duckdb().verified_only_select("FROM t1 SELECT b WHERE a = 1"); | ||
| println!("{:?}", stmt); |
| #[test] | ||
| fn test_duckdb_from_statement_with_filter() { | ||
| let stmt = duckdb().verified_only_select("FROM t1 WHERE a = 1"); | ||
| println!("{:?}", stmt); |
|
|
||
| /// Parse a duckdb style `FROM` statement without a select. | ||
| /// assuming the initial `FROM` was already consumed | ||
| pub fn parse_from_select(&mut self) -> Result<Select, ParserError> { |
There was a problem hiding this comment.
this seems largely copied from the normal select parsing -- is there any way to refactor the code to reduce the duplication
|
Marking as Draft to signify this PR is no longer waiting on review. Please mark it as ready for review when it is ready for another look. |
|
FWIW @Nikita-str has solved the stack overflow issues in #1171 if you wanted to revive this PR. |
allows for parsing of duckdb's
fromqueriescloses #1143