fix: mysql ON UPDATE not taking an expression#586
Conversation
2585a03 to
02f41c2
Compare
|
I think the enum idea is a good one |
|
FYI, I am planning to make a new sqlparser-rs release in the next day or two. This PR has some outstanding comments and appears to have have stalled |
|
Marking as draft to signify this PR has been reviewed. Please mark this PR as ready for review when it is ready again. |
7180583 to
16b0dc5
Compare
16b0dc5 to
653cd2b
Compare
|
Commit updated: |
Pull Request Test Coverage Report for Build 3769028368
💛 - Coveralls |
|
@alamb Just had a quick review and found an improvement could be made, so added a refactor commit. If you would prefer I can squash into the primary commit and force push. |
8322d7f to
c2a0559
Compare
parse::check_on_update_expr_is_valid - using local const instead of vec for valid object names
c2a0559 to
e6b0c64
Compare
|
Added NOW keyword, included that in the parse time functions match arm |
| self.index | ||
| } | ||
|
|
||
| fn check_on_update_expr_is_valid(expr: &Expr) -> Result<(), ParserError> { |
There was a problem hiding this comment.
my preference is to leave this type of "semantic" check downstream -- so I would prefer that sqlparser accepts any expression in this place, and then downstream crates can enforce limits like the expr must be one of the specified time functions.
@AugustoFKL and I tried to explain this difference in the readme recently
https://github.com/sqlparser-rs/sqlparser-rs#extensible-sql-lexer-and-parser-for-rust
This crate provides only a syntax parser, and tries to avoid applying
any SQL semantics, and accepts queries that specific databases would
reject, even when using that Database's specificDialect. For
example,CREATE TABLE(x int, x int)is accepted by this crate, even
though most SQL engines will reject this statement due to the repeated
column namex.
This crate avoids semantic analysis because it varies drastically
between dialects and implementations. If you want to do semantic
analysis, feel free to use this project as a base
| Token::make_keyword("ON UPDATE"), | ||
| ]))) | ||
| let expr = self.parse_expr()?; | ||
| Self::check_on_update_expr_is_valid(&expr)?; |
There was a problem hiding this comment.
I recommend just accepting expr here
|
Given #602 has something similar, perhaps I can try and push this one through |
Attempt to resolve the #543, the PR resolves the issue however I was not sure on the test construction so there is currently a pending todo before this can be merged. As per my comments on #543 any assistance/guidance would be appreciated.
Also not sure the extra enum variant is the best way to do this, but wasn't able to pull the tokens and expr from
DialectSpecificvariant.closes #543