Fokko commented on code in PR #6127: URL: https://github.com/apache/iceberg/pull/6127#discussion_r1015073328
########## python/pyiceberg/expressions/visitors.py: ########## @@ -417,6 +422,75 @@ def visit_bound_predicate(self, predicate) -> BooleanExpression: return predicate +def expression_evaluator(schema: Schema, unbound: BooleanExpression, case_sensitive=True) -> Callable[[StructProtocol], bool]: Review Comment: ```suggestion def expression_evaluator(schema: Schema, unbound: BooleanExpression, case_sensitive: bool = True) -> Callable[[StructProtocol], bool]: ``` ########## python/pyiceberg/expressions/visitors.py: ########## @@ -417,6 +422,75 @@ def visit_bound_predicate(self, predicate) -> BooleanExpression: return predicate +def expression_evaluator(schema: Schema, unbound: BooleanExpression, case_sensitive=True) -> Callable[[StructProtocol], bool]: + return _ExpressionEvaluator(schema, unbound, case_sensitive).eval + + +class _ExpressionEvaluator(BoundBooleanExpressionVisitor[bool]): + bound: BooleanExpression + struct: StructProtocol + + def __init__(self, schema: Schema, unbound: BooleanExpression, case_sensitive=True): Review Comment: ```suggestion def __init__(self, schema: Schema, unbound: BooleanExpression, case_sensitive: bool = True): ``` ########## python/pyiceberg/expressions/visitors.py: ########## @@ -417,6 +422,75 @@ def visit_bound_predicate(self, predicate) -> BooleanExpression: return predicate +def expression_evaluator(schema: Schema, unbound: BooleanExpression, case_sensitive=True) -> Callable[[StructProtocol], bool]: + return _ExpressionEvaluator(schema, unbound, case_sensitive).eval + + +class _ExpressionEvaluator(BoundBooleanExpressionVisitor[bool]): + bound: BooleanExpression + struct: StructProtocol + + def __init__(self, schema: Schema, unbound: BooleanExpression, case_sensitive=True): + self.bound = bind(schema, unbound, case_sensitive) + + def eval(self, struct: StructProtocol): Review Comment: ```suggestion def eval(self, struct: StructProtocol) -> bool: ``` ########## python/pyiceberg/expressions/__init__.py: ########## @@ -281,6 +289,30 @@ def __invert__(self) -> BoundIsNull: return BoundIsNull(self.term) +def coerce_unary_arguments(data_class: Union[type, None]) -> Union[type, Callable[[type], type]]: Review Comment: I'm not sold on this, mostly because it adds another layer of complexity. These things should normally just happen in the init. I've created a PR https://github.com/apache/iceberg/pull/6127 That does exactly this, well tested 👍🏻 ########## python/pyiceberg/expressions/__init__.py: ########## @@ -351,17 +386,56 @@ def bind(self, schema: Schema, case_sensitive: bool = True) -> BooleanExpression @dataclass(frozen=True) -class BoundSetPredicate(BoundPredicate[T]): - literals: set[Literal[T]] +class BoundSetPredicate(BoundPredicate[C]): + literals: set[Literal[C]] + + @property Review Comment: ```suggestion @cached_property ``` This way we don't have to build the caching mechanism ourselves. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org