Skip to content

Commit

Permalink
ARROW-8186: [Python] Fix dataset expression operation with invalid sc…
Browse files Browse the repository at this point in the history
…alar

When the expression operation is invalid, we should raise an error rather than returning a non-expression object.

Closes #6700 from jorisvandenbossche/ARROW-8186

Authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
Signed-off-by: Krisztián Szűcs <szucs.krisztian@gmail.com>
  • Loading branch information
jorisvandenbossche authored and kszucs committed Mar 24, 2020
1 parent 8294254 commit 0449ea7
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 14 deletions.
17 changes: 3 additions & 14 deletions python/pyarrow/_dataset.pyx
Expand Up @@ -1165,20 +1165,12 @@ cdef class Scanner:
def _binop(fn, left, right):
# cython doesn't support reverse operands like __radd__ just passes the
# arguments in the same order as the binary operator called

if isinstance(left, Expression) and isinstance(right, Expression):
pass
elif isinstance(left, Expression):
try:
right = ScalarExpression(right)
except TypeError:
return NotImplemented

right = ScalarExpression(right)
elif isinstance(right, Expression):
try:
left = ScalarExpression(left)
except TypeError:
return NotImplemented
left = ScalarExpression(left)
else:
raise TypeError('Neither left nor right arguments are Expressions')

Expand Down Expand Up @@ -1282,10 +1274,7 @@ cdef class Expression:
}

if not isinstance(other, Expression):
try:
other = ScalarExpression(other)
except TypeError:
return NotImplemented
other = ScalarExpression(other)

return ComparisonExpression(operator_mapping[op], self, other)

Expand Down
13 changes: 13 additions & 0 deletions python/pyarrow/tests/test_dataset.py
Expand Up @@ -444,6 +444,19 @@ def test_expression_ergonomics():
with pytest.raises(TypeError):
field.isin(1)

# operations with non-scalar values
with pytest.raises(TypeError):
field == [1]

with pytest.raises(TypeError):
field != {1}

with pytest.raises(TypeError):
field & [1]

with pytest.raises(TypeError):
field | [1]


@pytest.mark.parametrize('paths_or_selector', [
fs.FileSelector('subdir', recursive=True),
Expand Down

0 comments on commit 0449ea7

Please sign in to comment.