-
Notifications
You must be signed in to change notification settings - Fork 564
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
Support mysql partition
to table selection
#959
Support mysql partition
to table selection
#959
Conversation
f2b7af9
to
d441f7d
Compare
@alamb PTAL |
Sorry for the delay -- I was away last week. I am looking at this PR now |
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.
Thank you for the contribution @chunshao90 -- this is looking pretty close. I left some suggestions for your consideration
src/parser/mod.rs
Outdated
let partitions = if dialect_of!(self is MySqlDialect) | ||
&& self.parse_keyword(Keyword::PARTITION) | ||
{ | ||
let mut partitions = self.parse_comma_separated(|p| p.parse_tuple(true, false))?; |
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.
As written I think this only checks the first partition for having more than one tuple element (what about values after index 0?)
I think if you used parse_expr
here this code would be significantly simpler and there would be no need for error checking
let mut partitions = self.parse_comma_separated(|p| p.parse_tuple(true, false))?; | |
let mut partitions = self.parse_comma_separated(Parser::parse_expr)?; |
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.
Returns an error if there are multiple tuples.
f287cec
to
f3f5c9e
Compare
@alamb Please review again. |
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.
Looks good to me -- thank you @chunshao90
I'll clean up the clippy failures |
partition
to table selection
Thanks again @chunshao90 ! |
Co-authored-by: Andrew Lamb <[email protected]>
Support mysql partition selection, example:
Refer to https://dev.mysql.com/doc/refman/8.0/en/partitioning-selection.html