Skip to content

Commit

Permalink
fix: respect order of ops in FloorDivide
Browse files Browse the repository at this point in the history
  • Loading branch information
NickCrews committed Oct 23, 2024
1 parent fbf214d commit fb85936
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 2 deletions.
2 changes: 1 addition & 1 deletion ibis/backends/sql/compilers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -847,7 +847,7 @@ def visit_Clip(self, op, *, arg, lower, upper):
return arg

def visit_FloorDivide(self, op, *, left, right):
return self.cast(self.f.floor(left / right), op.dtype)
return self.cast(self.f.floor(sge.paren(left) / sge.paren(right)), op.dtype)

def visit_Ceil(self, op, *, arg):
return self.cast(self.f.ceil(arg), op.dtype)
Expand Down
2 changes: 1 addition & 1 deletion ibis/backends/sql/compilers/flink.py
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ def visit_TryCast(self, op, *, arg, to):
return sge.TryCast(this=arg, to=type_mapper.from_ibis(to))

def visit_FloorDivide(self, op, *, left, right):
return self.f.floor(left / right)
return self.f.floor(sge.paren(left) / sge.paren(right))

Check warning on line 333 in ibis/backends/sql/compilers/flink.py

View check run for this annotation

Codecov / codecov/patch

ibis/backends/sql/compilers/flink.py#L333

Added line #L333 was not covered by tests

def visit_JSONGetItem(self, op, *, arg, index):
assert isinstance(op.index, ops.Literal)
Expand Down
7 changes: 7 additions & 0 deletions ibis/backends/tests/test_numeric.py
Original file line number Diff line number Diff line change
Expand Up @@ -902,6 +902,13 @@ def test_simple_math_functions_columns(
backend.assert_series_equal(result, expected)


def test_floor_divide_precedence(con):
# Check that we compile to 16 / (4 / 2) and not 16 / 4 / 2
expr = ibis.literal(16) // (4 / ibis.literal(2))
result = int(con.execute(expr))
assert result == 8


# we add one to double_col in this test to make sure the common case works (no
# domain errors), and we test the backends' various failure modes in each
# backend's test suite
Expand Down

0 comments on commit fb85936

Please sign in to comment.