You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/07/10 23:04:11 UTC

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

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


##########
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 BaseReference(Generic[T], Term, ABC):
+    """Represents a variable reference in an expression."""
+
+
+class BoundTerm(Bound[T], Term):
+    """Represents a bound term."""
+
+
+class UnboundTerm(Unbound[T, BoundTerm[T]], Term):
+    """Represents an unbound term."""
+
+
+@dataclass(frozen=True)
+class BoundReference(BoundTerm[T], BaseReference[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:
   I think that this should return `T`. That's the type that this reference produces. That may be a little difficult since `Accessor` is not parameterized by the type that it returns. If it is hard, we can fix it later.



-- 
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