Skip to content

Commit cb959fa

Browse files
zvonimir-ddclaude
andcommitted
Honour operator precedence in MySQL and Spark DIV
`get_next_precedence` announces `DIV` at `Precedence::MulDivModOp`, but both dialects parsed its right operand with `parse_expr()` (= `parse_subexpr(0)`), so the operand absorbed every following operator: `7 DIV 2 + 1` grouped as `7 DIV (2 + 1)` = 2 where MySQL and Spark both give `(7 DIV 2) + 1` = 4. Thread the caller's precedence through instead, as SqliteDialect already does for REGEXP / MATCH / GLOB. `Display` emits no parentheses, so a mis-grouped tree round-trips to the original SQL and the round-trip helpers could not catch this; the new tests assert on the tree. Fixes #2460 Environment: Datadog workspace Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent c0054b7 commit cb959fa

4 files changed

Lines changed: 90 additions & 4 deletions

File tree

src/dialect/mysql.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,12 +98,12 @@ impl Dialect for MySqlDialect {
9898
&self,
9999
parser: &mut crate::parser::Parser,
100100
expr: &crate::ast::Expr,
101-
_precedence: u8,
101+
precedence: u8,
102102
) -> Option<Result<crate::ast::Expr, ParserError>> {
103103
// Parse DIV as an operator
104104
if parser.parse_keyword(Keyword::DIV) {
105105
let left = Box::new(expr.clone());
106-
let right = Box::new(match parser.parse_expr() {
106+
let right = Box::new(match parser.parse_subexpr(precedence) {
107107
Ok(expr) => expr,
108108
Err(e) => return Some(Err(e)),
109109
});

src/dialect/spark.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,11 +137,11 @@ impl Dialect for SparkSqlDialect {
137137
&self,
138138
parser: &mut Parser,
139139
expr: &Expr,
140-
_precedence: u8,
140+
precedence: u8,
141141
) -> Option<Result<Expr, ParserError>> {
142142
if parser.parse_keyword(Keyword::DIV) {
143143
let left = Box::new(expr.clone());
144-
let right = Box::new(match parser.parse_expr() {
144+
let right = Box::new(match parser.parse_subexpr(precedence) {
145145
Ok(expr) => expr,
146146
Err(e) => return Some(Err(e)),
147147
});

tests/sqlparser_mysql.rs

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3759,6 +3759,62 @@ fn parse_div_infix_propagates_parse_error() {
37593759
assert_matches!(err, ParserError::ParserError(_));
37603760
}
37613761

3762+
#[test]
3763+
fn parse_div_precedence() {
3764+
let div = |left: Expr, right: Expr| Expr::BinaryOp {
3765+
left: Box::new(left),
3766+
op: BinaryOperator::MyIntegerDivide,
3767+
right: Box::new(right),
3768+
};
3769+
let num = |n: &str| Expr::value(number(n));
3770+
3771+
// `DIV` shares the precedence of `*` and `/`, so `+` must end up at the root.
3772+
assert_eq!(
3773+
Expr::BinaryOp {
3774+
left: Box::new(div(num("7"), num("2"))),
3775+
op: BinaryOperator::Plus,
3776+
right: Box::new(num("1")),
3777+
},
3778+
mysql().verified_expr("7 DIV 2 + 1")
3779+
);
3780+
3781+
// Equal precedence resolves left-associatively, both against `*` and against itself.
3782+
assert_eq!(
3783+
Expr::BinaryOp {
3784+
left: Box::new(div(num("9"), num("3"))),
3785+
op: BinaryOperator::Multiply,
3786+
right: Box::new(num("3")),
3787+
},
3788+
mysql().verified_expr("9 DIV 3 * 3")
3789+
);
3790+
assert_eq!(
3791+
div(div(num("10"), num("5")), num("2")),
3792+
mysql().verified_expr("10 DIV 5 DIV 2")
3793+
);
3794+
3795+
assert_eq!(
3796+
Expr::BinaryOp {
3797+
left: Box::new(div(Expr::Identifier(Ident::new("a")), num("2"))),
3798+
op: BinaryOperator::Eq,
3799+
right: Box::new(num("1")),
3800+
},
3801+
mysql().verified_expr("a DIV 2 = 1")
3802+
);
3803+
3804+
// Explicit parentheses still push the whole expression into the right operand.
3805+
assert_eq!(
3806+
div(
3807+
num("7"),
3808+
Expr::Nested(Box::new(Expr::BinaryOp {
3809+
left: Box::new(num("2")),
3810+
op: BinaryOperator::Plus,
3811+
right: Box::new(num("1")),
3812+
}))
3813+
),
3814+
mysql().verified_expr("7 DIV (2 + 1)")
3815+
);
3816+
}
3817+
37623818
#[test]
37633819
fn parse_drop_temporary_table() {
37643820
let sql = "DROP TEMPORARY TABLE foo";

tests/sqlparser_spark.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,36 @@ fn test_div_literal() {
171171
spark().one_statement_parses_to("SELECT 10 div 3", "SELECT 10 DIV 3");
172172
}
173173

174+
#[test]
175+
fn test_div_precedence() {
176+
let div = |left: Expr, right: Expr| Expr::BinaryOp {
177+
left: Box::new(left),
178+
op: BinaryOperator::MyIntegerDivide,
179+
right: Box::new(right),
180+
};
181+
let num = |n: &str| Expr::value(number(n));
182+
183+
// `DIV` shares the precedence of `*` and `/`, so `+` must end up at the root.
184+
assert_eq!(
185+
Expr::BinaryOp {
186+
left: Box::new(div(num("7"), num("2"))),
187+
op: BinaryOperator::Plus,
188+
right: Box::new(num("1")),
189+
},
190+
spark().verified_expr("7 DIV 2 + 1")
191+
);
192+
193+
// Equal precedence resolves left-associatively.
194+
assert_eq!(
195+
Expr::BinaryOp {
196+
left: Box::new(div(num("9"), num("3"))),
197+
op: BinaryOperator::Multiply,
198+
right: Box::new(num("3")),
199+
},
200+
spark().verified_expr("9 DIV 3 * 3")
201+
);
202+
}
203+
174204
// --------------------------------
175205
// Struct support
176206
// --------------------------------

0 commit comments

Comments
 (0)