Skip to content

Commit 618aa58

Browse files
committed
fix: Wrap is_null expressions in parens, to avoid operator precedence issues
Expression like `(foo IS NOT NULL = bar IS NOT NULL)`` would try to compare `foo IS NOT NULL` with `bar`, not with `bar IS NOT NULL`
1 parent 57bcbc4 commit 618aa58

File tree

4 files changed

+15
-8
lines changed

4 files changed

+15
-8
lines changed

packages/cubejs-schema-compiler/src/adapter/BaseQuery.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3327,7 +3327,7 @@ export class BaseQuery {
33273327
column_aliased: '{{expr}} {{quoted_alias}}',
33283328
query_aliased: '{{ query }} AS {{ quoted_alias }}',
33293329
case: 'CASE{% if expr %} {{ expr }}{% endif %}{% for when, then in when_then %} WHEN {{ when }} THEN {{ then }}{% endfor %}{% if else_expr %} ELSE {{ else_expr }}{% endif %} END',
3330-
is_null: '{{ expr }} IS {% if negate %}NOT {% endif %}NULL',
3330+
is_null: '({{ expr }} IS {% if negate %}NOT {% endif %}NULL)',
33313331
binary: '({{ left }} {{ op }} {{ right }})',
33323332
sort: '{{ expr }} {% if asc %}ASC{% else %}DESC{% endif %} NULLS {% if nulls_first %}FIRST{% else %}LAST{% endif %}',
33333333
order_by: '{% if index %} {{ index }} {% else %} {{ expr }} {% endif %} {% if asc %}ASC{% else %}DESC{% endif %}{% if nulls_first %} NULLS FIRST{% endif %}',

rust/cubesql/cubesql/src/compile/mod.rs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12188,7 +12188,7 @@ ORDER BY "source"."str0" ASC
1218812188
}
1218912189
init_testing_logger();
1219012190

12191-
let logical_plan = convert_select_to_query_plan(
12191+
let query_plan = convert_select_to_query_plan(
1219212192
r#"
1219312193
WITH "qt_0" AS (
1219412194
SELECT "ta_1"."customer_gender" "ca_1"
@@ -12205,13 +12205,20 @@ ORDER BY "source"."str0" ASC
1220512205
.to_string(),
1220612206
DatabaseProtocol::PostgreSQL,
1220712207
)
12208-
.await
12209-
.as_logical_plan();
12208+
.await;
12209+
12210+
let physical_plan = query_plan.as_physical_plan().await.unwrap();
12211+
println!(
12212+
"Physical plan: {}",
12213+
displayable(physical_plan.as_ref()).indent()
12214+
);
12215+
12216+
let logical_plan = query_plan.as_logical_plan();
1221012217

1221112218
let sql = logical_plan.find_cube_scan_wrapped_sql().wrapped_sql.sql;
1221212219

12213-
// check wrapping for `NOT(.. IS NULL OR LOWER(..) IN)`
12214-
let re = Regex::new(r"NOT \(.+ IS NULL OR .*LOWER\(.+ IN ").unwrap();
12220+
// check wrapping for `NOT((.. IS NULL) OR LOWER(..) IN)`
12221+
let re = Regex::new(r"NOT \(\(.+ IS NULL\) OR .*LOWER\(.+ IN ").unwrap();
1221512222
assert!(re.is_match(&sql));
1221612223
}
1221712224

rust/cubesql/cubesql/src/compile/test/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -578,7 +578,7 @@ OFFSET {{ offset }}{% endif %}"#.to_string(),
578578
"{{expr}} {{quoted_alias}}".to_string(),
579579
),
580580
("expressions/binary".to_string(), "({{ left }} {{ op }} {{ right }})".to_string()),
581-
("expressions/is_null".to_string(), "{{ expr }} IS {% if negate %}NOT {% endif %}NULL".to_string()),
581+
("expressions/is_null".to_string(), "({{ expr }} IS {% if negate %}NOT {% endif %}NULL)".to_string()),
582582
("expressions/case".to_string(), "CASE{% if expr %} {{ expr }}{% endif %}{% for when, then in when_then %} WHEN {{ when }} THEN {{ then }}{% endfor %}{% if else_expr %} ELSE {{ else_expr }}{% endif %} END".to_string()),
583583
("expressions/sort".to_string(), "{{ expr }} {% if asc %}ASC{% else %}DESC{% endif %}{% if nulls_first %} NULLS FIRST {% endif %}".to_string()),
584584
("expressions/cast".to_string(), "CAST({{ expr }} AS {{ data_type }})".to_string()),

rust/cubesql/cubesql/src/compile/test/test_wrapper.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1174,7 +1174,7 @@ WHERE
11741174
let last_mod_month_re =
11751175
Regex::new(r#""logs_alias"."[a-zA-Z0-9_]{1,16}" "last_mod_month""#).unwrap();
11761176
assert!(last_mod_month_re.is_match(&sql));
1177-
let sum_price_re = Regex::new(r#"CASE WHEN "logs_alias"."[a-zA-Z0-9_]{1,16}" IS NOT NULL THEN "logs_alias"."[a-zA-Z0-9_]{1,16}" ELSE 0 END "sum_price""#)
1177+
let sum_price_re = Regex::new(r#"CASE WHEN \("logs_alias"."[a-zA-Z0-9_]{1,16}" IS NOT NULL\) THEN "logs_alias"."[a-zA-Z0-9_]{1,16}" ELSE 0 END "sum_price""#)
11781178
.unwrap();
11791179
assert!(sum_price_re.is_match(&sql));
11801180
let cube_user_re = Regex::new(r#""logs_alias"."[a-zA-Z0-9_]{1,16}" "cube_user""#).unwrap();

0 commit comments

Comments
 (0)