Skip to content

Commit

Permalink
fix(sql): properly parenthesize binary ops containing named expressions
Browse files Browse the repository at this point in the history
  • Loading branch information
jcrist committed Sep 4, 2024
1 parent ac79604 commit 3955ebe
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
},
)
param = ibis.param("timestamp")
f = alltypes.filter((alltypes.timestamp_col < param.name("my_param")))
f = alltypes.filter((alltypes.timestamp_col < param))
agg = f.aggregate([f.float_col.sum().name("foo")], by=[f.string_col])

result = agg.foo.count()
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
SELECT
(
"t0"."a" + "t0"."b"
) * "t0"."c" AS "x"
FROM "t" AS "t0"
2 changes: 1 addition & 1 deletion ibis/backends/tests/sql/test_compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ def test_subquery_where_location(snapshot):
],
name="alltypes",
)
param = ibis.param("timestamp").name("my_param")
param = ibis.param("timestamp")
expr = (
t[["float_col", "timestamp_col", "int_col", "string_col"]][
lambda t: t.timestamp_col < param
Expand Down
6 changes: 6 additions & 0 deletions ibis/backends/tests/sql/test_sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,12 @@ def test_binop_parens(snapshot, opname, dtype, associative):
snapshot.assert_match(combined, "out.sql")


def test_binop_with_alias_still_parenthesized(snapshot):
t = ibis.table({"a": "int", "b": "int", "c": "int"}, name="t")
sql = to_sql(((t.a + t.b).name("d") * t.c).name("x"))
snapshot.assert_match(sql, "out.sql")


@pytest.mark.parametrize(
"expr_fn",
[
Expand Down
8 changes: 8 additions & 0 deletions ibis/expr/operations/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,14 @@ class Binary(Value):
left: Value
right: Value

def __init__(self, left, right, **kwargs):
# Dealias left and right, passing through any additional fields
if isinstance(left, Alias):
left = left.arg
if isinstance(right, Alias):
right = right.arg
super().__init__(left=left, right=right, **kwargs)

@attribute
def shape(self) -> ds.DataShape:
return max(self.left.shape, self.right.shape)
Expand Down
6 changes: 6 additions & 0 deletions ibis/tests/expr/test_value_exprs.py
Original file line number Diff line number Diff line change
Expand Up @@ -731,6 +731,12 @@ def test_string_mul(table, left, right):
assert isinstance(expr.op(), ops.Repeat)


def test_binop_strips_aliases(table):
assert (table.a.name("x") + table.b).equals(table.a + table.b)
assert (table.a + table.b.name("x")).equals(table.a + table.b)
assert (table.a.name("x") + 1).equals(table.a + 1)


@pytest.mark.parametrize(
["op", "name", "case", "ex_type"],
[
Expand Down

0 comments on commit 3955ebe

Please sign in to comment.