MySQL: Add support for SELECT modifiers
#2172
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds support for MySQL-specific `SELECT` modifiers that appear after the
`SELECT` keyword. Grammar from the [docs]:
```sql
SELECT
[ALL | DISTINCT | DISTINCTROW ]
[HIGH_PRIORITY]
[STRAIGHT_JOIN]
[SQL_SMALL_RESULT] [SQL_BIG_RESULT] [SQL_BUFFER_RESULT]
[SQL_NO_CACHE] [SQL_CALC_FOUND_ROWS]
select_expr [, select_expr] ...
```
Manual testing shows that these options can appear in any order relative
to each other, so for the sake of fidelity, we parse this separately
from how we parse distinct and other options for other dialects, in a
new `Parser::parse_select_modifiers` method.
`DISTINCTROW` is a legacy (but not deprecated) synonym for `DISTINCT`,
so it just gets canonicalized as `DISTINCT`.
[docs]: https://dev.mysql.com/doc/refman/8.4/en/select.html
mvzink
force-pushed
the
push-syxoqlsrvqux
branch
from
460f829 to
a437a32
Compare
iffyio
reviewed
Jan 29, 2026
| fn parse_select_distinct_on() { | ||
| let sql = "SELECT DISTINCT ON (album_id) name FROM track ORDER BY album_id, milliseconds"; | ||
| let select = verified_only_select(sql); | ||
| let select = all_dialects_but_mysql().verified_only_select(sql); |
Contributor
There was a problem hiding this comment.
Suggested change
| let select = all_dialects_but_mysql().verified_only_select(sql); | |
| let select = all_dialects_except(|d| d.is::<MySqlDialect>()).verified_only_select(sql); |
thinking instead of introducing a dedicated function we can reuse this helper?
Comment on lines
+14053
to
+14055
| /// Parses SELECT modifiers and DISTINCT/ALL in any order. Allows HIGH_PRIORITY, STRAIGHT_JOIN, | ||
| /// SQL_SMALL_RESULT, SQL_BIG_RESULT, SQL_BUFFER_RESULT, SQL_NO_CACHE, SQL_CALC_FOUND_ROWS and | ||
| /// DISTINCT/DISTINCTROW/ALL to appear in any order. |
Contributor
There was a problem hiding this comment.
Suggested change
| /// Parses SELECT modifiers and DISTINCT/ALL in any order. Allows HIGH_PRIORITY, STRAIGHT_JOIN, | |
| /// SQL_SMALL_RESULT, SQL_BIG_RESULT, SQL_BUFFER_RESULT, SQL_NO_CACHE, SQL_CALC_FOUND_ROWS and | |
| /// DISTINCT/DISTINCTROW/ALL to appear in any order. | |
| /// Parses `SELECT` modifiers and `DISTINCT/ALL` in any order. |
Thinking we can avoid enumerating the variants so that they don't go out of sync with the impl
| return parser_err!("Cannot specify both ALL and DISTINCT".to_string(), loc); | ||
| self.prev_token(); | ||
| return self.expected( | ||
| "ALL alone without DISTINCT or DISTINCTROW", |
Contributor
There was a problem hiding this comment.
this diff seems incorrect to mention DISTINCTROW?
Comment on lines
+14060
to
+14061
| let mut distinct: Option<Distinct> = None; | ||
| let mut has_all = false; |
Contributor
There was a problem hiding this comment.
Can we instead introduce a proper enum to represent the three states? the current impl I think is harder to follow which combinations are valid and if/when the specified option is ignored in the output.
| } | ||
| distinct = Some(Distinct::Distinct); | ||
| } | ||
| Keyword::HIGH_PRIORITY => modifiers.high_priority = true, |
Contributor
There was a problem hiding this comment.
Suggested change
| Keyword::HIGH_PRIORITY => modifiers.high_priority = true, | |
| Keyword::HIGH_PRIORITY if !modifiers.high_priority => modifiers.high_priority = true, |
similar for the others is something like this needed if it's invalid that they occur multiple times (can we add test cases for the behavior)?
| /// MySQL-specific SELECT modifiers. | ||
| /// | ||
| /// See [MySQL SELECT](https://dev.mysql.com/doc/refman/8.4/en/select.html). | ||
| pub select_modifiers: SelectModifiers, |
Contributor
There was a problem hiding this comment.
Suggested change
| pub select_modifiers: SelectModifiers, | |
| pub select_modifiers: Option<SelectModifiers>, |
| #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] | ||
| #[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] | ||
| pub struct SelectModifiers { | ||
| /// `HIGH_PRIORITY` gives the SELECT higher priority than statements that update a table. |
Contributor
There was a problem hiding this comment.
could we include the mysql links individually on the fields as well? that way if other dialects add their own variants in the future the doc would be consistent
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds support for MySQL-specific
SELECTmodifiers that appear after theSELECTkeyword. Grammar from the docs:Manual testing shows that these options can appear in any order relative to each other, so for the sake of fidelity, we parse this separately from how we parse distinct and other options for other dialects, in a new
Parser::parse_select_modifiersmethod.DISTINCTROWis a legacy (but not deprecated) synonym forDISTINCT, so it just gets canonicalized asDISTINCT.