Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Patch division precedence issues #3969

Merged
merged 2 commits into from
Dec 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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