[GitHub] [iceberg] samredai commented on a diff in pull request #4816: remove operations from base expressions; add in expression

[GitBox <gi...@apache.org>]

samredai commented on code in PR #4816:
URL: https://github.com/apache/iceberg/pull/4816#discussion_r915192163


##########
python/pyiceberg/expressions/base.py:
##########
@@ -131,11 +75,115 @@ def __ge__(self, other):
 
 
 class BooleanExpression(ABC):
-    """base class for all boolean expressions"""
+    """Represents a boolean expression tree."""
 
     @abstractmethod
     def __invert__(self) -> "BooleanExpression":
-        ...
+        """Transform the Expression into its negated version."""
+
+
+class Bound(Generic[T], ABC):
+    """Represents a bound value expression."""
+
+    def eval(self, struct: StructProtocol):  # pylint: disable=W0613
+        ...  # pragma: no cover
+
+
+class Unbound(Generic[T, B], ABC):
+    """Represents an unbound expression node."""
+
+    @abstractmethod
+    def bind(self, schema: Schema, case_sensitive: bool) -> B:
+        ...  # pragma: no cover
+
+
+class Term(ABC):
+    """An expression that evaluates to a value."""
+
+
+class Reference(Generic[T], ABC):
+    """Represents a variable reference in an expression."""
+
+
+class BoundTerm(Bound[T], Term):
+    """Represents a bound term."""
+
+
+class UnboundTerm(Unbound[T, BoundTerm[T]]):
+    """Represents an unbound term."""
+
+
+@dataclass(frozen=True)
+class BoundReference(BoundTerm[T], Reference[T]):
+    """A reference bound to a field in a schema
+
+    Args:
+        field (NestedField): A referenced field in an Iceberg schema
+        accessor (Accessor): An Accessor object to access the value at the field's position
+    """
+
+    field: NestedField
+    accessor: Accessor
+
+    def eval(self, struct: StructProtocol) -> Any:

Review Comment:
   @rdblue in [this](https://github.com/apache/iceberg/pull/4816#discussion_r884327792) comment did you mean that we shouldn't include this method at all or just that we shouldn't include it as an `@abstractmethod` on `BoundPredicate`? From my understanding we need this here to retrieve the value of a reference given a row/struct as tested [here](https://github.com/apache/iceberg/blob/master/python/tests/expressions/test_expressions_base.py#L318-L320).



-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org