iffyio commented on code in PR #1604:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1604#discussion_r1892588098
##########
src/parser/mod.rs:
##########
@@ -11679,14 +11679,21 @@ impl<'a> Parser<'a> {
pub fn parse_update(&mut self) -> Result<Statement, ParserError> {
let or = self.parse_conflict_clause();
let table = self.parse_table_and_joins()?;
+ let from_before_set = if self.parse_keyword(Keyword::FROM)
+ && dialect_of!(self is GenericDialect | PostgreSqlDialect |
DuckDbDialect | BigQueryDialect | SnowflakeDialect | RedshiftSqlDialect |
MsSqlDialect | SQLiteDialect )
Review Comment:
I'm thinking since the functionality does not conflict with other dialects
we can skip the dialect guards and parse the `FROM (SELECT ...)` as long as it
exists?
##########
src/parser/mod.rs:
##########
@@ -11679,14 +11679,21 @@ impl<'a> Parser<'a> {
pub fn parse_update(&mut self) -> Result<Statement, ParserError> {
let or = self.parse_conflict_clause();
let table = self.parse_table_and_joins()?;
+ let from_before_set = if self.parse_keyword(Keyword::FROM)
+ && dialect_of!(self is GenericDialect | PostgreSqlDialect |
DuckDbDialect | BigQueryDialect | SnowflakeDialect | RedshiftSqlDialect |
MsSqlDialect | SQLiteDialect )
+ {
+ Some(self.parse_table_and_joins()?)
+ } else {
+ None
+ };
self.expect_keyword(Keyword::SET)?;
let assignments =
self.parse_comma_separated(Parser::parse_assignment)?;
let from = if self.parse_keyword(Keyword::FROM)
&& dialect_of!(self is GenericDialect | PostgreSqlDialect |
DuckDbDialect | BigQueryDialect | SnowflakeDialect | RedshiftSqlDialect |
MsSqlDialect | SQLiteDialect )
{
Some(self.parse_table_and_joins()?)
} else {
- None
+ from_before_set
Review Comment:
Could we instead introduce a wrapper type to encode the position information?
Something like
```rust
enum UpdateTableFromKind {
BeforeSet(TableWithJoins),
AfterSet(TableWithJoins)
}
...
Statement::Update {... from: Option<UpdateTableFromKind> }
```
See
[TableSampleKind](https://github.com/yoavcloud/datafusion-sqlparser-rs/blob/eae5629fb86f5e262063c0bc99ff628a5855168f/src/ast/query.rs#L1157-L1162)
for an example on this approach.
That would ensure that we can produce the same sql as in the input.
##########
tests/sqlparser_snowflake.rs:
##########
@@ -2974,3 +2974,10 @@ fn test_table_sample() {
snowflake_and_generic().verified_stmt("SELECT id FROM mytable TABLESAMPLE
(10) REPEATABLE (1)");
snowflake_and_generic().verified_stmt("SELECT id FROM mytable TABLESAMPLE
(10) SEED (1)");
}
+
+#[test]
+fn parse_update_from_before_select() {
Review Comment:
note if we drop the dialect guard, we can move the tests to
sqlparser_common.rs to cover all dialects.
Also could we add a test case demonstrating the behavior mentioned above
where we have two `FROM` clauses? e.g.
```sql
UPDATE t1 FROM (SELECT ... ) AS t2 SET name = t2.name FROM (SELECT ...) AS
t2;
```
##########
src/parser/mod.rs:
##########
@@ -11679,14 +11679,21 @@ impl<'a> Parser<'a> {
pub fn parse_update(&mut self) -> Result<Statement, ParserError> {
let or = self.parse_conflict_clause();
let table = self.parse_table_and_joins()?;
+ let from_before_set = if self.parse_keyword(Keyword::FROM)
+ && dialect_of!(self is GenericDialect | PostgreSqlDialect |
DuckDbDialect | BigQueryDialect | SnowflakeDialect | RedshiftSqlDialect |
MsSqlDialect | SQLiteDialect )
+ {
+ Some(self.parse_table_and_joins()?)
+ } else {
+ None
+ };
self.expect_keyword(Keyword::SET)?;
let assignments =
self.parse_comma_separated(Parser::parse_assignment)?;
let from = if self.parse_keyword(Keyword::FROM)
Review Comment:
thinking here in order to be correct we might need to check `if
from_before_set.is_none() && self.parse_keyword(FROM) ...`? I'm assuming that
the intention is that it is invalid sql to provide both?
--
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]
