MySQL: Bind && with the precedence of AND - #2576
LucaCappelletti94 wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2576 +/- ##
==========================================
+ Coverage 81.02% 81.15% +0.13%
==========================================
Files 42 42
Lines 33436 33744 +308
Branches 33436 33744 +308
==========================================
+ Hits 27090 27384 +294
- Misses 2789 2797 +8
- Partials 3557 3563 +6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| | Token::ExclamationMarkDoubleTilde | ||
| | Token::ExclamationMarkDoubleTildeAsterisk | ||
| | Token::Spaceship => Ok(p!(Eq)), | ||
| Token::Overlap if self.supports_double_ampersand_operator() => Ok(p!(And)), |
There was a problem hiding this comment.
My bad. After looking into it more, I think overriding get_next_precedence in MySqlDialect would be cleaner, the same way Oracle does for StringConcat:
smt like this
fn get_next_precedence(&self, parser: &Parser) -> Option<Result<u8, ParserError>> {
match parser.peek_token_ref().token {
Token::Overlap => Some(Ok(self.prec_value(Precedence::And))),
_ => None,
}
}
There was a problem hiding this comment.
Nit, not for this PR: supports_double_ampersand_operator() reads as "supports &&", while it actually means "&& is boolean AND" (Generic/PG/Redshift support && as overlap with the flag off). A name like supports_double_ampersand_as_and() could be a follow-up; it's a public trait method, so it'd be a breaking change.
s5dsn-eqee
left a comment
s5dsn-eqee
left a comment
There was a problem hiding this comment.
The framework already lets a dialect change precedence locally by overriding Dialect::get_next_precedence.
c100cac to
ee97692
Compare
3 participants
Since #2144, MySQL's
&&parses asBinaryOperator::And, but its precedence still came from the shared group that ranks the PostgreSQL overlap operator above the comparison operators. Sox < 1 && y > 2parsed as((x < (1 AND y)) > 2)andx = a && casx = (a AND c). Both then printed withAND, which reads back as a different tree.MySQL ranks
&&withAND, below every comparison (https://dev.mysql.com/doc/refman/8.4/en/operator-precedence.html). The precedence lookup now givesToken::OverlaptheANDprecedence wheneversupports_double_ampersand_operator()holds, the same checkparse_infixuses to build theANDnode. PostgreSQL, Redshift and Generic keep&&as the overlap operator at its current precedence. The test added in #2144 now also checks that the comparison cases give the same trees as theirANDspellings.