From 6eef9dd651c463f6aa2bbe97ca03f356bc0e4057 Mon Sep 17 00:00:00 2001 From: Ayoub Date: Sun, 20 Sep 2026 19:19:31 +0100 Subject: [PATCH 1/2] perf(expressions): avoid building literal sets twice for In / NotIn In.__new__ and NotIn.__new__ already build a literal set to check whether the predicate can collapse to AlwaysFalse / EqualTo. SetPredicate.__init__ was then building the same set again. That meant every set predicate could allocate 2K Literal models instead of K. Reuse the set created by __new__ and return early from SetPredicate.__init__ when the instance is already initialized, following the same pattern used by And.__init__. Also resolve bound_term.ref().field.field_type once per bind() instead of once for every literal in the set comprehension. --- pyiceberg/expressions/__init__.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/pyiceberg/expressions/__init__.py b/pyiceberg/expressions/__init__.py index ece0db82db..4414d831c7 100644 --- a/pyiceberg/expressions/__init__.py +++ b/pyiceberg/expressions/__init__.py @@ -698,6 +698,11 @@ class SetPredicate(UnboundPredicate, ABC): def __init__( self, term: str | UnboundTerm, literals: Iterable[Any] | Iterable[LiteralValue] | None = None, **kwargs: Any ) -> None: + # `In.__new__` and `NotIn.__new__` have to build the literal set to pick the predicate + # class, so they initialize the instance with it. Without this guard the set is built twice. + if hasattr(self, "literals"): + return + if literals is None and "values" in kwargs: literals = kwargs["values"] @@ -709,8 +714,8 @@ def __init__( def bind(self, schema: Schema, case_sensitive: bool = True) -> BoundSetPredicate: bound_term = self.term.bind(schema, case_sensitive) - literal_set = self.literals - return self.as_bound(bound_term, {lit.to(bound_term.ref().field.field_type) for lit in literal_set}) # type: ignore + field_type = bound_term.ref().field.field_type + return self.as_bound(bound_term, {lit.to(field_type) for lit in self.literals}) # type: ignore def __str__(self) -> str: """Return the string representation of the SetPredicate class.""" @@ -844,7 +849,9 @@ def __new__( # pylint: disable=W0221 elif count == 1: return EqualTo(term, next(iter(literals_set))) else: - return super().__new__(cls) + predicate = super().__new__(cls) + SetPredicate.__init__(predicate, term, literals_set) + return predicate def __invert__(self) -> NotIn: """Transform the Expression into its negated version.""" @@ -879,7 +886,9 @@ def __new__( # pylint: disable=W0221 elif count == 1: return NotEqualTo(term, next(iter(literals_set))) else: - return super().__new__(cls) + predicate = super().__new__(cls) + SetPredicate.__init__(predicate, term, literals_set) + return predicate def __invert__(self) -> In: """Transform the Expression into its negated version.""" From a555b1a6e59221498e06479aa39214cb6be7d313 Mon Sep 17 00:00:00 2001 From: Ayoub Date: Tue, 22 Sep 2026 10:22:06 +0100 Subject: [PATCH 2/2] perf(expressions): reuse literal set from new Pass the set built by __new__ directly to __init__ instead of rebuilding it ...This also fixes generator inputs: rebuilding the set could consume an already-exhausted iterator and create an empty predicate Add regression coverage for generator input --- pyiceberg/expressions/__init__.py | 22 ++++++++-------------- tests/expressions/test_expressions.py | 8 ++++++++ 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/pyiceberg/expressions/__init__.py b/pyiceberg/expressions/__init__.py index 4414d831c7..f8529d7df4 100644 --- a/pyiceberg/expressions/__init__.py +++ b/pyiceberg/expressions/__init__.py @@ -698,18 +698,12 @@ class SetPredicate(UnboundPredicate, ABC): def __init__( self, term: str | UnboundTerm, literals: Iterable[Any] | Iterable[LiteralValue] | None = None, **kwargs: Any ) -> None: - # `In.__new__` and `NotIn.__new__` have to build the literal set to pick the predicate - # class, so they initialize the instance with it. Without this guard the set is built twice. - if hasattr(self, "literals"): - return - - if literals is None and "values" in kwargs: - literals = kwargs["values"] - - if literals is None: - literal_set: set[LiteralValue] = set() - else: - literal_set = _to_literal_set(literals) + # __new__ already built the set, and may have used up a one-shot iterator doing it. + literal_set = self.__dict__.pop("_literals_from_new", None) + if literal_set is None: + if literals is None and "values" in kwargs: + literals = kwargs["values"] + literal_set = set() if literals is None else _to_literal_set(literals) super().__init__(term=_to_unbound_term(term), values=literal_set) def bind(self, schema: Schema, case_sensitive: bool = True) -> BoundSetPredicate: @@ -850,7 +844,7 @@ def __new__( # pylint: disable=W0221 return EqualTo(term, next(iter(literals_set))) else: predicate = super().__new__(cls) - SetPredicate.__init__(predicate, term, literals_set) + object.__setattr__(predicate, "_literals_from_new", literals_set) return predicate def __invert__(self) -> NotIn: @@ -887,7 +881,7 @@ def __new__( # pylint: disable=W0221 return NotEqualTo(term, next(iter(literals_set))) else: predicate = super().__new__(cls) - SetPredicate.__init__(predicate, term, literals_set) + object.__setattr__(predicate, "_literals_from_new", literals_set) return predicate def __invert__(self) -> In: diff --git a/tests/expressions/test_expressions.py b/tests/expressions/test_expressions.py index 8ce48a6897..a12726ecbe 100644 --- a/tests/expressions/test_expressions.py +++ b/tests/expressions/test_expressions.py @@ -327,6 +327,14 @@ def test_in_list() -> None: assert In(Reference("foo"), ["a", "bc", "def"]).literals == {literal("a"), literal("bc"), literal("def")} +def test_in_generator() -> None: + assert In(Reference("foo"), (v for v in ["a", "bc", "def"])).literals == {literal("a"), literal("bc"), literal("def")} + + +def test_not_in_generator() -> None: + assert NotIn(Reference("foo"), (v for v in ["a", "bc", "def"])).literals == {literal("a"), literal("bc"), literal("def")} + + def test_not_in_empty() -> None: assert NotIn(Reference("foo"), ()) == AlwaysTrue()