TGooch44 commented on a change in pull request #2399:
URL: https://github.com/apache/iceberg/pull/2399#discussion_r606005443
##########
File path: python/iceberg/api/expressions/predicate.py
##########
@@ -70,72 +86,152 @@ def __str__(self):
class BoundPredicate(Predicate):
- def __init__(self, op, ref, lit=None):
- super(BoundPredicate, self).__init__(op, ref, lit)
+ def __init__(self, op: Operation, term: BoundTerm, lit: Literal = None,
literals: List[Literal] = None,
+ is_unary_predicate: bool = False, is_literal_predicate: bool
= False,
+ is_set_predicate: bool = False):
+ super(BoundPredicate, self).__init__(op, term)
+ ValidationException.check(sum([is_unary_predicate,
is_literal_predicate, is_set_predicate]) == 1,
+ "Only a single predicate type may be set:
%s=%s, %s=%s, %s=%s",
+ ("is_unary_predicate", is_unary_predicate,
+ "is_literal_predicate",
is_literal_predicate,
+ "is_set_predicate", is_set_predicate))
+
+ self._literals: Union[None, List[Literal]] = None
+ if is_unary_predicate:
+ ValidationException.check(lit is None, "Unary Predicates may not
have a literal", ())
+
+ elif is_literal_predicate:
+ ValidationException.check(lit is not None, "Literal Predicates
must have a literal set", ())
+ self._literals = [lit] # type: ignore
+
+ elif is_set_predicate:
+ ValidationException.check(literals is not None, "Set Predicates
must have literals set", ())
+ self._literals = literals
+ else:
+ raise ValueError(f"Unable to instantiate {op} -> (lit={lit},
literal={literals}")
- def negate(self):
- return BoundPredicate(self.op.negate(), self.ref, self.lit)
+ @property
+ def lit(self) -> Union[None, Literal]:
+ if self._literals is None or len(self._literals) == 0:
+ return None
+ return self._literals[0]
+
+ def eval(self, struct: StructLike) -> bool:
+ raise NotImplementedError("eval not implemented for BoundPredicate")
+
+ def test(self, struct) -> bool:
+ raise NotImplementedError("TO:DO implement test")
class UnboundPredicate(Predicate):
- def __init__(self, op, named_ref, value=None, lit=None):
- if value is None and lit is None:
- super(UnboundPredicate, self).__init__(op, named_ref, None)
+ def __init__(self, op, term, value=None, lit=None, values=None,
literals=None):
+ self._literals = None
+ num_set_args = sum([1 for x in [value, lit, values, literals] if x is
not None])
+
+ if num_set_args > 1:
+ raise ValueError(f"Only one of value={value}, lit={lit},
values={values}, literals={literals} may be set")
+ super(UnboundPredicate, self).__init__(op, term)
if isinstance(value, Literal):
lit = value
value = None
if value is not None:
- super(UnboundPredicate, self).__init__(op, named_ref,
Literals.from_(value))
+ self._literals = [Literals.from_(value)]
elif lit is not None:
- super(UnboundPredicate, self).__init__(op, named_ref, lit)
+ self._literals = [lit]
+ elif values is not None:
+ self._literals = map(Literals.from_, values)
+ elif literals is not None:
+ self._literals = literals
+
+ @property
+ def literals(self):
+ return self._literals
+
+ @property
+ def lit(self):
+ if self.op in [Operation.IN, Operation.NOT_IN]:
+ raise ValueError(f"{self.op} predicate cannot return a literal")
+
+ return None if self.literals is None else self.literals[0]
def negate(self):
- return UnboundPredicate(self.op.negate(), self.ref, self.lit)
+ return UnboundPredicate(self.op.negate(), self.term,
literals=self.literals)
- def bind(self, struct, case_sensitive=True): # noqa: C901
- if case_sensitive:
- field = struct.field(self.ref.name)
- else:
- field = struct.case_insensitive_field(self.ref.name.lower())
-
- ValidationException.check(field is not None,
- "Cannot find field '%s' in struct %s",
(self.ref.name, struct))
-
- if self.lit is None:
- if self.op == Operation.IS_NULL:
- if field.is_required:
- return FALSE
- return BoundPredicate(Operation.IS_NULL,
BoundReference(struct, field.field_id))
- elif self.op == Operation.NOT_NULL:
- if field.is_required:
- return TRUE
- return BoundPredicate(Operation.NOT_NULL,
BoundReference(struct, field.field_id))
+ def bind(self, struct, case_sensitive=True):
+ bound = self.term.bind(struct, case_sensitive=case_sensitive)
+
+ if self.literals is None:
+ return self.bind_unary_operation(bound)
+ elif self.op in [Operation.IN, Operation.NOT_IN]:
+ return self.bind_in_operation(bound)
+
+ return self.bind_literal_operation(bound)
+
+ def bind_unary_operation(self, bound_term: BoundTerm) -> BoundPredicate:
+ from .expressions import Expressions
Review comment:
If it's at the top it introduces a circular import because of all the
static methods in Expressions that expose shorthand's for the various predicate
type, eg. `Expressions.equal`, `Expressions.not_equal`, etc. These could
probably be moved into the predicate file itself, but leaving it in the
expressions keeps it a bit more consistent with the Java API. We could also
move it to the predicates file and then expose it in the expressions module
init file.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
