Fokko commented on code in PR #6892:
URL: https://github.com/apache/iceberg/pull/6892#discussion_r1116079239
##########
python/tests/expressions/test_parser.py:
##########
@@ -149,3 +151,13 @@ def test_and_or_with_parens() -> None:
assert Or(IsNull("x"), And(GreaterThanOrEqual("x", 5), Not(LessThan("x",
10)))) == parser.parse(
"(x is null) or (5 <= x) and not(x < 10)"
)
+
+
+def test_starts_with() -> None:
+ assert StartsWith("x", "data") == parser.parse("x starts_with 'data'")
Review Comment:
I don't think `starts_with` is common in SQL. How about:
```suggestion
assert StartsWith("x", "data") == parser.parse("x LIKE 'data*'")
```
##########
python/pyiceberg/expressions/visitors.py:
##########
@@ -943,6 +987,12 @@ def visit_less_than(self, term: BoundTerm[L], literal:
Literal[L]) -> List[Tuple
def visit_less_than_or_equal(self, term: BoundTerm[L], literal:
Literal[L]) -> List[Tuple[str, str, Any]]:
return [(term.ref().field.name, "<=",
self._cast_if_necessary(term.ref().field.field_type, literal.value))]
+ def visit_starts_with(self, term: BoundTerm[L], literal: Literal[L]) ->
List[Tuple[str, str, Any]]:
+ return [(term.ref().field.name, "starts_with",
self._cast_if_necessary(term.ref().field.field_type, literal.value))]
Review Comment:
I don't think that this is a supported predicate:
https://github.com/apache/arrow/blob/45918a90a6ca1cf3fd67c256a7d6a240249e555a/python/pyarrow/parquet/core.py#L126-L131
We can just return an empty predicate for now:
```suggestion
return []
```
This visitor isn't used right now but might be in the future for Dask to
implement page skipping. Keeping it empty will mean that we don't use it for
skipping pages, but we have to filter on a row level afterward anyway.
##########
python/pyiceberg/expressions/visitors.py:
##########
@@ -675,6 +701,24 @@ def visit_less_than_or_equal(self, term: BoundTerm[L],
literal: Literal[L]) -> b
return ROWS_MIGHT_MATCH
+ def visit_starts_with(self, term: BoundTerm[L], literal: Literal[L]) ->
bool:
Review Comment:
Same for the ManifestEvaluator, can you copy the tests from Java:
https://github.com/apache/iceberg/blob/master/api/src/test/java/org/apache/iceberg/expressions/TestInclusiveManifestEvaluator.java#L538-L627
--
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]
