rdblue commented on code in PR #4816:
URL: https://github.com/apache/iceberg/pull/4816#discussion_r907943759
##########
python/src/iceberg/expressions/base.py:
##########
@@ -138,6 +75,24 @@ def __invert__(self) -> "BooleanExpression":
...
+class BoundPredicate(ABC):
+ def __init__(self, term, literal):
+ """A concrete predicate must have a `term` and `literal`"""
+ self._term = term
+ self._literal = literal
+
+
+class UnboundPredicate:
+ def __init__(self, term, literal):
+ """A concrete predicate must have a `term` and `literal`"""
+ self._term = term
+ self._literal = literal
Review Comment:
Is there value in having this `__init__` method? I don't think `In`
delegates to it.
Also, every `UnboundPredicate` will have a `term`, but not all will have one
or more literals. `IsNull`, `NotNull`, `IsNan`, and `NotNan` are all unary and
have no term. `In` and `NotIn` have more than one literal. And the comparison
predicates, like `LessThan` and `Equal` have exactly one literal. In the Java
version, we model this as a list or iterable of literals and ensure
requirements are handled when binding. Then the bound versions are more strict
about the literals.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
