Skip to content

Commit

Permalink
Wrap all math operations in parens
Browse files Browse the repository at this point in the history
Previously if you used local variables or parens in Rust to override the
default order of operations, we would completely lose that information
in the generated SQL. This would at best result in an unexpected value,
but at worse would cause a query like `now + 2.minutes() * (some_int +
1)` to result in a runtime error complaining that there's no operator
for `interval + integer`

I intend to backport this to 1.4

Fixes #2017
  • Loading branch information
sgrif committed Mar 19, 2019
1 parent 568d3e2 commit 1f2ab1f
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 1 deletion.
10 changes: 9 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,15 @@ All user visible changes to this project will be documented in this file.
This project adheres to [Semantic Versioning](http://semver.org/), as described
for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/text/1105-api-evolution.md)

## Unreleased
## [1.4.2] - 2019-03019

### Fixed

* Parenthesis are now inserted around all mathematical operations. This means
that `(2.into_sql() + 3) * 4` will correctly evaluate to 20 as expected.
Previously we would generate SQL that evaluated to 14. This could even result
in runtime errors if multiple types were involved (for example, `interval *
(integer + 1)`)

## [1.4.1] - 2019-01-24

Expand Down
2 changes: 2 additions & 0 deletions diesel/src/expression/ops/numeric.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,11 @@ macro_rules! numeric_operation {
Rhs: QueryFragment<DB>,
{
fn walk_ast(&self, mut out: AstPass<DB>) -> QueryResult<()> {
out.push_sql("(");
self.lhs.walk_ast(out.reborrow())?;
out.push_sql($op);
self.rhs.walk_ast(out.reborrow())?;
out.push_sql(")");
Ok(())
}
}
Expand Down
9 changes: 9 additions & 0 deletions diesel_tests/tests/expressions/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ fn test_dividing_nullables() {
let data = nullable_table.select(value / value).load(&connection);
assert_eq!(Ok(expected_data), data);
}

#[test]
fn mix_and_match_all_numeric_ops() {
use schema::users::dsl::*;
Expand All @@ -190,3 +191,11 @@ fn mix_and_match_all_numeric_ops() {
let data = users.select(id * 3 / 2 + 4 - 1).load(&connection);
assert_eq!(Ok(expected_data), data);
}

#[test]
fn precedence_with_parens_is_maintained() {
use diesel::sql_types::Integer;

let x = select((2.into_sql::<Integer>() + 3) * 4).get_result::<i32>(&connection());
assert_eq!(Ok(20), x);
}

0 comments on commit 1f2ab1f

Please sign in to comment.