Skip to content

Commit

Permalink
fix: Patch division precedence issues (#3969)
Browse files Browse the repository at this point in the history
  • Loading branch information
max-sixty authored Dec 19, 2023
1 parent 880fdc1 commit c219ac2
Show file tree
Hide file tree
Showing 19 changed files with 91 additions and 53 deletions.
2 changes: 1 addition & 1 deletion prqlc/prql-compiler/src/sql/gen_expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -935,7 +935,7 @@ impl SQLExpression for BinaryOperator {
fn associativity(&self) -> Associativity {
use BinaryOperator::*;
match self {
Minus | Divide => Associativity::Left,
Minus | Divide | Modulo => Associativity::Left,
_ => Associativity::Both,
}
}
Expand Down
30 changes: 15 additions & 15 deletions prqlc/prql-compiler/src/sql/std.sql.prql
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,10 @@ let div_i = l r -> s"FLOOR(ABS({l:11} / {r:11})) * SIGN({l:0}) * SIGN({r:0})"

# We have a simple float division by default, but it can be overridden by dialects.
@{binding_strength=11}
let div_f = l r -> s"{l} / {r}"
let div_f = l r -> s"{l} / ({r})"

@{binding_strength=11}
let mod = l r -> s"{l} % {r}"
let mod = l r -> s"{l} % ({r})"

@{binding_strength=10}
let add = l r -> null
Expand Down Expand Up @@ -176,12 +176,12 @@ let not = l -> s"NOT {l}"

module ansi {
@{binding_strength=11}
let div_f = l r -> s"({l} * 1.0 / {r})"
let div_f = l r -> s"({l} * 1.0 / ({r}))"
}

module bigquery {
@{binding_strength=11}
let div_f = l r -> s"({l} * 1.0 / {r})"
let div_f = l r -> s"({l} * 1.0 / ({r}))"

# Mathematical functions
module math {
Expand All @@ -196,7 +196,7 @@ module bigquery {
module clickhouse {
# https://clickhouse.com/docs/en/sql-reference/functions/arithmetic-functions#divide
@{binding_strength=11}
let div_f = l r -> s"({l} / {r})"
let div_f = l r -> s"({l} / ({r}))"

@{binding_strength=11}
let div_i = l r -> s"({l} DIV {r})"
Expand All @@ -210,7 +210,7 @@ module clickhouse {

module duckdb {
@{binding_strength=11}
let div_f = l r -> s"({l} / {r})"
let div_f = l r -> s"({l} / ({r}))"

@{binding_strength=11}
let div_i = l r -> s"TRUNC({l:11} / {r:11})"
Expand All @@ -234,7 +234,7 @@ module duckdb {

module mssql {
@{binding_strength=11}
let div_f = l r -> s"({l} * 1.0 / {r})"
let div_f = l r -> s"({l} * 1.0 / ({r}))"

# Mathematical functions
module math {
Expand All @@ -255,10 +255,10 @@ module mssql {

module mysql {
@{binding_strength=11}
let div_f = l r -> s"({l} / {r})"
let div_f = l r -> s"({l} / ({r}))"

@{binding_strength=11}
let div_i = l r -> s"({l} DIV {r})"
let div_i = l r -> s"({l} DIV ({r}))"

@{binding_strength=100}
let mod = l r -> s"ROUND(MOD({l:0}, {r:0}))"
Expand All @@ -269,10 +269,10 @@ module mysql {

module postgres {
@{binding_strength=11}
let div_f = l r -> s"({l} * 1.0 / {r})"
let div_f = l r -> s"({l} * 1.0 / ({r}))"

@{binding_strength=100}
let div_i = l r -> s"TRUNC({l:11} / {r:11})"
let div_i = l r -> s"TRUNC({l:11} / ({r:11}))"

# Mathematical functions
module math {
Expand All @@ -293,10 +293,10 @@ module postgres {

module glaredb {
@{binding_strength=11}
let div_f = l r -> s"({l} * 1.0 / {r})"
let div_f = l r -> s"({l} * 1.0 / ({r}))"

@{binding_strength=100}
let div_i = l r -> s"TRUNC({l:11} / {r:11})"
let div_i = l r -> s"TRUNC({l:11} / ({r:11}))"

# Mathematical functions
module math {
Expand Down Expand Up @@ -324,7 +324,7 @@ module sqlite {
let concat_array = column -> s"GROUP_CONCAT({column:0}, '')"

@{binding_strength=11}
let div_f = l r -> s"({l} * 1.0 / {r})"
let div_f = l r -> s"({l} * 1.0 / ({r}))"

@{binding_strength=100}
let div_i = l r -> s"ROUND(ABS({l:11} / {r:11}) - 0.5) * SIGN({l:0}) * SIGN({r:0})"
Expand All @@ -345,5 +345,5 @@ module sqlite {
module snowflake {
# https://docs.snowflake.com/en/sql-reference/operators-arithmetic#division
@{binding_strength=11}
let div_f = l r -> s"({l} / {r})"
let div_f = l r -> s"({l} / ({r}))"
}
18 changes: 18 additions & 0 deletions prqlc/prql-compiler/tests/integration/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1386,6 +1386,24 @@ fn test_op_precedence() {
Integer: 4
"###);

assert_yaml_snapshot!(parse_expr(r#"1 / (3 * 4)"#).unwrap(), @r###"
---
Binary:
left:
Literal:
Integer: 1
op: DivFloat
right:
Binary:
left:
Literal:
Integer: 3
op: Mul
right:
Literal:
Integer: 4
"###);

assert_yaml_snapshot!(parse_expr(r#"1 / 2 - 3 * 4 + 1"#).unwrap(), @r###"
---
Binary:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,32 +37,32 @@ WITH table_0 AS (
)
SELECT
id,
x_int / k_int,
x_int / k_float,
x_float / k_int,
x_float / k_float,
x_int / (k_int),
x_int / (k_float),
x_float / (k_int),
x_float / (k_float),
FLOOR(ABS(x_int / k_int)) * SIGN(x_int) * SIGN(k_int) AS q_ii,
FLOOR(ABS(x_int / k_float)) * SIGN(x_int) * SIGN(k_float) AS q_if,
FLOOR(ABS(x_float / k_int)) * SIGN(x_float) * SIGN(k_int) AS q_fi,
FLOOR(ABS(x_float / k_float)) * SIGN(x_float) * SIGN(k_float) AS q_ff,
x_int % k_int AS r_ii,
x_int % k_float AS r_if,
x_float % k_int AS r_fi,
x_float % k_float AS r_ff,
x_int % (k_int) AS r_ii,
x_int % (k_float) AS r_if,
x_float % (k_int) AS r_fi,
x_float % (k_float) AS r_ff,
ROUND(
FLOOR(ABS(x_int / k_int)) * SIGN(x_int) * SIGN(k_int) * k_int + x_int % k_int,
FLOOR(ABS(x_int / k_int)) * SIGN(x_int) * SIGN(k_int) * k_int + x_int % (k_int),
0
),
ROUND(
FLOOR(ABS(x_int / k_float)) * SIGN(x_int) * SIGN(k_float) * k_float + x_int % k_float,
FLOOR(ABS(x_int / k_float)) * SIGN(x_int) * SIGN(k_float) * k_float + x_int % (k_float),
0
),
ROUND(
FLOOR(ABS(x_float / k_int)) * SIGN(x_float) * SIGN(k_int) * k_int + x_float % k_int,
FLOOR(ABS(x_float / k_int)) * SIGN(x_float) * SIGN(k_int) * k_int + x_float % (k_int),
0
),
ROUND(
FLOOR(ABS(x_float / k_float)) * SIGN(x_float) * SIGN(k_float) * k_float + x_float % k_float,
FLOOR(ABS(x_float / k_float)) * SIGN(x_float) * SIGN(k_float) * k_float + x_float % (k_float),
0
)
FROM
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ WITH table_0 AS (
tracks
WHERE
REGEXP(name, 'Love')
AND milliseconds / 1000 / 60 BETWEEN 3 AND 4
AND milliseconds / (1000) / (60) BETWEEN 3 AND 4
ORDER BY
track_id
LIMIT
Expand Down
28 changes: 24 additions & 4 deletions prqlc/prql-compiler/tests/integration/sql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,19 +246,39 @@ fn json_of_test() {
assert_eq!(json.chars().nth(json.len() - 1).unwrap(), ']');
}

#[test]
fn test_precedence_again() {
assert_display_snapshot!((compile(r###"
from artists
derive {
p2 = x / (y * z), # needs parentheses
p1 = a - (b + c), # needs parentheses
}
"###).unwrap()), @r###"
SELECT
*,
x / (y * z) AS p2,
a - (b + c) AS p1
FROM
artists
"###);
}

#[test]
fn test_precedence() {
assert_display_snapshot!((compile(r###"
from artists
derive {
p1 = a - (b + c), # needs parentheses
p2 = a / (b * c), # needs parentheses
np1 = a + (b - c), # no parentheses
np2 = (a + b) - c, # no parentheses
}
"###).unwrap()), @r###"
SELECT
*,
a - (b + c) AS p1,
a / (b * c) AS p2,
a + b - c AS np1,
a + b - c AS np2
FROM
Expand All @@ -275,8 +295,8 @@ fn test_precedence() {
"###).unwrap()), @r###"
SELECT
*,
(temp_f - 32) / 1.8 AS temp_c,
(temp_f - 32) / 1.8 * 9 / 5 AS temp_f,
(temp_f - 32) / (1.8) AS temp_c,
(temp_f - 32) / (1.8) * 9 / (5) AS temp_f,
temp_x + 9 - 5 AS temp_z
FROM
x
Expand Down Expand Up @@ -356,7 +376,7 @@ fn test_precedence() {
c + a - b,
c - d - (a - b),
c + d + a - b,
a / b * c,
a / (b * c),
y - z AS x,
-(y - z)
FROM
Expand Down Expand Up @@ -2966,7 +2986,7 @@ fn test_casting() {
CAST(a AS int) + 10 AS b,
CAST(a AS int) - 10 AS c,
CAST(a AS float) * 10 AS d,
CAST(a AS float) / 10 AS e
CAST(a AS float) / (10) AS e
FROM
x
"###
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ expression: "let fahrenheit_to_celsius = temp -> (temp - 32) / 1.8\n\nfrom citie
---
SELECT
*,
(temp_f - 32) / 1.8 AS temp_c
(temp_f - 32) / (1.8) AS temp_c
FROM
cities

Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ expression: "let interp = low:0 high x -> (x - low) / (high - low)\n\nfrom stude
---
SELECT
*,
(sat_score - 0) / (1600 - 0) AS sat_proportion_1,
(sat_score - 0) / (1600 - 0) AS sat_proportion_2
(sat_score - 0) / ((1600 - 0)) AS sat_proportion_1,
(sat_score - 0) / ((1600 - 0)) AS sat_proportion_2
FROM
students

Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ SELECT
labor,
overhead,
cost_total,
materials / cost_total AS materials_share,
labor / cost_total AS labor_share,
overhead / cost_total AS overhead_share
materials / (cost_total) AS materials_share,
labor / (cost_total) AS labor_share,
overhead / (cost_total) AS overhead_share
FROM
costs

Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ expression: "let interp = low:0 high x -> (x - low) / (high - low)\n\nfrom stude
---
SELECT
*,
(sat_score - 0) / (1600 - 0) AS sat_proportion_1,
(sat_score - 0) / (1600 - 0) AS sat_proportion_2
(sat_score - 0) / ((1600 - 0)) AS sat_proportion_1,
(sat_score - 0) / ((1600 - 0)) AS sat_proportion_2
FROM
students

Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ expression: "let fahrenheit_to_celsius = temp -> (temp - 32) / 1.8\n\nfrom citie
---
SELECT
*,
(temp_f - 32) / 1.8 AS temp_c
(temp_f - 32) / (1.8) AS temp_c
FROM
cities

Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ expression: "let fahrenheit_to_celsius = temp -> (temp - 32) / 1.8\nlet interp =
---
SELECT
*,
((temp_c - 32) / 1.8 - 0) / (100 - 0) AS boiling_proportion
((temp_c - 32) / (1.8) - 0) / ((100 - 0)) AS boiling_proportion
FROM
kettles

Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ WITH table_0 AS (
5 AS y
)
SELECT
(x * 1.0 / y) AS quotient,
(x * 1.0 / (y)) AS quotient,
ROUND(ABS(x / y) - 0.5) * SIGN(x) * SIGN(y) AS int_quotient
FROM
table_0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ WITH table_0 AS (
5 AS y
)
SELECT
(x / y) AS quotient,
(x DIV y) AS int_quotient
(x / (y)) AS quotient,
(x DIV (y)) AS int_quotient
FROM
table_0

Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ WITH table_0 AS (
-2 AS b
)
SELECT
(a * 1.0 / b) AS div_out,
(a * 1.0 / (b)) AS div_out,
ROUND(ABS(a / b) - 0.5) * SIGN(a) * SIGN(b) AS int_div_out
FROM
table_0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ SELECT
distance BETWEEN 0 AND 20 AS is_proximate,
SUM(distance) OVER () AS total_distance,
MIN(COALESCE(distance, 5)) OVER () AS min_capped_distance,
distance / 40 AS travel_time,
distance / (40) AS travel_time,
ROUND(distance, 1 + 1) AS distance_rounded_2_dp,
distance >= 100 AS is_far,
distance BETWEEN -100 AND 0,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
---
source: web/book/tests/documentation/book.rs
expression: "from tracks\n# This would be a really long line without being able to split it:\nselect listening_time_years = (spotify_plays + apple_music_plays + pandora_plays)\n# * length_seconds\n# Actually it's `length_s` I think:\n\\ * length_s\n# min hour day year\n\\ / 60 / 60 / 24 / 365\n"
expression: "from tracks\n# This would be a really long line without being able to split it:\nselect listening_time_years = (spotify_plays + apple_music_plays + pandora_plays)\n# We can toggle between lines when developing:\n# \\ * length_seconds\n\\ * length_s\n# min hour day year\n\\ / 60 / 60 / 24 / 365\n"
---
SELECT
(
spotify_plays + apple_music_plays + pandora_plays
) * length_s / 60 / 60 / 24 / 365 AS listening_time_years
) * length_s / (60) / (60) / (24) / (365) AS listening_time_years
FROM
tracks

4 changes: 2 additions & 2 deletions web/website/data/examples/expressions.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ sql: |
SELECT
*,
started + unfinished AS finished,
(started + unfinished) / started AS fin_share,
(started + unfinished) / started / (1 - (started + unfinished) / started)
(started + unfinished) / (started) AS fin_share,
(started + unfinished) / (started) / ((1 - (started + unfinished) / (started)))
AS fin_ratio
FROM
track_plays
2 changes: 1 addition & 1 deletion web/website/data/examples/functions.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ prql: |
select temp_f = (fahrenheit_from_celsius temp_c)
sql: |
SELECT
temp_c * 9 / 5 + 32 AS temp_f
temp_c * 9 / (5) + 32 AS temp_f
FROM
weather

0 comments on commit c219ac2

Please sign in to comment.