kbendick commented on a change in pull request #2062:
URL: https://github.com/apache/iceberg/pull/2062#discussion_r589252718
##########
File path:
api/src/test/java/org/apache/iceberg/expressions/TestInclusiveManifestEvaluator.java
##########
@@ -439,6 +446,44 @@ public void testStringStartsWith() {
Assert.assertFalse("Should skip: range doesn't match", shouldRead);
}
+ @Test
+ public void testStringNotStartsWith() {
+ boolean shouldRead =
ManifestEvaluator.forRowFilter(notStartsWith("some_nulls", "a"), SPEC,
false).eval(FILE);
+ Assert.assertTrue("Should read: range matches", shouldRead);
+
+ shouldRead = ManifestEvaluator.forRowFilter(notStartsWith("some_nulls",
"aa"), SPEC, false).eval(FILE);
+ Assert.assertTrue("Should read: range matches", shouldRead);
+
+ shouldRead = ManifestEvaluator.forRowFilter(notStartsWith("some_nulls",
"dddd"), SPEC, false).eval(FILE);
+ Assert.assertTrue("Should read: range matches", shouldRead);
+
+ shouldRead = ManifestEvaluator.forRowFilter(notStartsWith("some_nulls",
"z"), SPEC, false).eval(FILE);
+ Assert.assertTrue("Should read: range matches", shouldRead);
+
+ shouldRead = ManifestEvaluator.forRowFilter(notStartsWith("no_nulls",
"a"), SPEC, false).eval(FILE);
+ Assert.assertTrue("Should read: range matches", shouldRead);
+
+ shouldRead = ManifestEvaluator.forRowFilter(notStartsWith("some_nulls",
"zzzz"), SPEC, false).eval(FILE);
+ Assert.assertTrue("Should read: range matches", shouldRead);
+
+ shouldRead = ManifestEvaluator.forRowFilter(notStartsWith("some_nulls",
"1"), SPEC, false).eval(FILE);
+ Assert.assertTrue("Should read: range matches", shouldRead);
+
+ shouldRead =
ManifestEvaluator.forRowFilter(notStartsWith("all_same_value_or_null", "a"),
SPEC, false).eval(FILE);
+ Assert.assertFalse("Should skip: range doesn't match", shouldRead);
Review comment:
I believe that either there's a code path that wasn't updated when we
made the decision to return files / row groups etc that evaluated as possibly
passing the predicate.
I will review those places.
The other thing that comes to mind is that possibly the file is not fully
mocked for the given situaton, but I don't think that's the case. I'm pretty
sure a path was not updated after we made the decision that a predicate x
notStartsWith a string literal for x is true (to deal with SQL 3-way boolean
logic which Iceberg does not implement).
I'll review this as soon as I have time and update.
Good catch! Thank you :)
----------------------------------------------------------------
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]
