Skip to content

Commit

Permalink
fix(comparison): wrap isnull equality check in parens (#8366)
Browse files Browse the repository at this point in the history
We wrap arguments to a sge.Equals in parens if the inputs are
themselves binary operations. There are other operations we will have
to wrap -- for now I'm just adding isNull to the list, but I imagine
this will grow, possibly on a backend-by-backend basis.

I'm going to investigate the mssql and oracle failures but they are
unrelated to this PR (they fail the added test with or without the parentheses)
  • Loading branch information
gforsyth authored Feb 15, 2024
1 parent 13d675e commit 247e2f7
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 2 deletions.
4 changes: 3 additions & 1 deletion ibis/backends/base/sqlglot/compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,8 @@ class SQLGlotCompiler(abc.ABC):
ops.IntervalSubtract,
)

NEEDS_PARENS = BINARY_INFIX_OPS + (ops.IsNull,)

def __init__(self) -> None:
self.agg = AggGen(aggfunc=self._aggregate)
self.f = FuncGen()
Expand Down Expand Up @@ -1192,7 +1194,7 @@ def visit_Aggregate(self, op, *, parent, groups, metrics):

@classmethod
def _add_parens(cls, op, sg_expr):
if isinstance(op, cls.BINARY_INFIX_OPS):
if isinstance(op, cls.NEEDS_PARENS):
return paren(sg_expr)
return sg_expr

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,8 @@ SELECT
`t0`.`b`
FROM `table` AS `t0`
WHERE
`t0`.`a` IS NULL = `t0`.`b` IS NULL
(
`t0`.`a` IS NULL
) = (
`t0`.`b` IS NULL
)
12 changes: 12 additions & 0 deletions ibis/backends/tests/test_generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -1846,3 +1846,15 @@ def test_select_mutate_with_dict(backend):

expr = t.select({"a": ibis.literal(1.0)}).limit(1)
backend.assert_frame_equal(result, expected)


@pytest.mark.broken(["mssql", "oracle"], reason="incorrect syntax")
def test_isnull_equality(con, backend, monkeypatch):
monkeypatch.setattr(ibis.options, "default_backend", con)
t = ibis.memtable({"x": ["a", "b", None], "y": ["c", None, None], "z": [1, 2, 3]})
expr = t.mutate(out=t.x.isnull() == t.y.isnull()).order_by("z").select("out")
result = expr.to_pandas()

expected = pd.DataFrame({"out": [True, False, True]})

backend.assert_frame_equal(result, expected)

0 comments on commit 247e2f7

Please sign in to comment.