Conversation
| // MySQL specific statement | ||
| // <https://dev.mysql.com/doc/refman/8.4/en/show.html#:~:text=SHOW%20%7BCHARACTER%20SET%20%7C%20CHARSET%7D%20%5Blike_or_where%5D> | ||
| ShowCharset { | ||
| is_shorthand: bool, |
There was a problem hiding this comment.
Could we add a description what this flag does?
| ShowCharset { | ||
| is_shorthand: bool, | ||
| filter: Option<ShowStatementFilter>, | ||
| }, |
There was a problem hiding this comment.
Can we switch to using a named struct syntax for this statement? See ref we're trying to move away from the anonymous struct syntax ideally.
We could have something like?
struct ShowCharSet { ... }
Statement::ShowCharSet(ShowCharSet{...})| // MySQL specific statement | ||
| // <https://dev.mysql.com/doc/refman/8.4/en/show.html#:~:text=SHOW%20%7BCHARACTER%20SET%20%7C%20CHARSET%7D%20%5Blike_or_where%5D> |
There was a problem hiding this comment.
| // MySQL specific statement | |
| // <https://dev.mysql.com/doc/refman/8.4/en/show.html#:~:text=SHOW%20%7BCHARACTER%20SET%20%7C%20CHARSET%7D%20%5Blike_or_where%5D> | |
| // | |
| [MySQL]: https://dev.mysql.com/doc/refman/8.4/en/show.html#:~:text=SHOW%20%7BCHARACTER%20SET%20%7C%20CHARSET%7D%20%5Blike_or_where%5D |
Or something similar, essentially dropping the mention of it being mysql specific, since that can change in the future or other existing dialects might also support it in some form
| // SHOW {CHARACTER SET | CHARSET} [like_or_where] | ||
| // ``` | ||
| // where: | ||
| // like_or_where: { | ||
| // LIKE 'pattern' | ||
| // | WHERE expr | ||
| // } |
There was a problem hiding this comment.
| // SHOW {CHARACTER SET | CHARSET} [like_or_where] | |
| // ``` | |
| // where: | |
| // like_or_where: { | |
| // LIKE 'pattern' | |
| // | WHERE expr | |
| // } | |
| // SHOW {CHARACTER SET | CHARSET} | |
| // ``` |
Thinking a brief example would be enough. For detailed syntax readers can follow the documentation
|
|
||
| #[test] | ||
| fn parse_show_charset() { | ||
| let _ = mysql().verified_stmt("SHOW CHARACTER SET"); |
There was a problem hiding this comment.
Since this adds a new node to the AST, can we assert one of the scenarios that the AST looks as expected? For example
Improves the initial `SHOW CHARSET` implementation by: - Using a named struct for the AST node - Adding AST assertion to the parser test - Improving code comments
iffyio
left a comment
iffyio
left a comment
There was a problem hiding this comment.
Thanks @etgarperets! Left a minor doc comment otherwise I think this looks good!
iffyio
changed the title
SHOW CHARSET
Co-authored-by: Ifeanyi Ubah <ify1992@yahoo.com>
iffyio
left a comment
iffyio
left a comment
There was a problem hiding this comment.
LGTM! Thanks @etgarperets!
cc @alamb
Co-authored-by: Ifeanyi Ubah <ify1992@yahoo.com>
2 participants
No description provided.