Skip to content

Commit ca6de5e

Browse files
jackylee-chclaude
andcommitted
fix(io): keep nulls when pushing NotEqualTo down to Arrow
An Arrow comparison yields null for a null input, so `field != value` dropped every row where the column is null. All the other evaluators treat a null as satisfying NotEqualTo: `_ExpressionEvaluationVisitor.visit_not_equal` returns `None != value`, `_StrictMetricsEvaluationVisitor` reports ROWS_MUST_MATCH for a column proven to hold only nulls, and the Arrow NOT IN path already keeps them because `~isin(...)` is false for a null. The reference implementation agrees: `Evaluator.notEq` is `!eq`, and `eq` compares with a nulls-first comparator, so a null never equals the literal. Keep the null explicitly. NOT IN needs no change. Co-Authored-By: Claude Code <noreply@anthropic.com>
1 parent 75b74cf commit ca6de5e

2 files changed

Lines changed: 19 additions & 2 deletions

File tree

‎pyiceberg/io/pyarrow.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -919,7 +919,10 @@ def visit_equal(self, term: BoundTerm, literal: Literal[Any]) -> pc.Expression:
919919
return pc.field(self._get_field_name(term)) == _convert_scalar(literal.value, term.ref().field.field_type)
920920

921921
def visit_not_equal(self, term: BoundTerm, literal: Literal[Any]) -> pc.Expression:
922-
return pc.field(self._get_field_name(term)) != _convert_scalar(literal.value, term.ref().field.field_type)
922+
# A null is not equal to the literal, but an Arrow comparison yields null for it and
923+
# the row would be dropped. Keep it explicitly to match the other evaluators.
924+
ref = pc.field(self._get_field_name(term))
925+
return ref.is_null(nan_is_null=False) | (ref != _convert_scalar(literal.value, term.ref().field.field_type))
923926

924927
def visit_greater_than_or_equal(self, term: BoundTerm, literal: Literal[Any]) -> pc.Expression:
925928
return pc.field(self._get_field_name(term)) >= _convert_scalar(literal.value, term.ref().field.field_type)

‎tests/io/test_pyarrow.py‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
BoundStartsWith,
6161
GreaterThan,
6262
Not,
63+
NotEqualTo,
6364
Or,
6465
)
6566
from pyiceberg.expressions.literals import literal
@@ -785,10 +786,23 @@ def test_expr_equal_to_pyarrow(bound_reference: BoundReference) -> None:
785786
def test_expr_not_equal_to_pyarrow(bound_reference: BoundReference) -> None:
786787
assert (
787788
repr(expression_to_pyarrow(BoundNotEqualTo(bound_reference, literal("hello"))))
788-
== '<pyarrow.compute.Expression (foo != "hello")>'
789+
== '<pyarrow.compute.Expression (is_null(foo, {nan_is_null=false}) or (foo != "hello"))>'
789790
)
790791

791792

793+
def test_expr_not_equal_to_pyarrow_keeps_nulls(bound_reference: BoundReference) -> None:
794+
"""A null is not equal to the literal, so it satisfies NotEqualTo, as it does in the other evaluators."""
795+
from pyiceberg.expressions.visitors import expression_evaluator
796+
797+
values = ["hello", "world", None]
798+
pushed_down = pa.table({"foo": values}).filter(expression_to_pyarrow(BoundNotEqualTo(bound_reference, literal("hello"))))
799+
schema = Schema(NestedField(field_id=1, name="foo", field_type=StringType(), required=False))
800+
in_memory = expression_evaluator(schema, NotEqualTo("foo", "hello"), True)
801+
802+
assert pushed_down.column("foo").to_pylist() == ["world", None]
803+
assert [value for value in values if in_memory(Record(value))] == ["world", None]
804+
805+
792806
def test_expr_greater_than_or_equal_equal_to_pyarrow(bound_reference: BoundReference) -> None:
793807
assert (
794808
repr(expression_to_pyarrow(BoundGreaterThanOrEqual(bound_reference, literal("hello"))))

0 commit comments

Comments
 (0)