kbendick commented on a change in pull request #2062:
URL: https://github.com/apache/iceberg/pull/2062#discussion_r554910739



##########
File path: 
api/src/main/java/org/apache/iceberg/expressions/InclusiveMetricsEvaluator.java
##########
@@ -372,6 +372,38 @@ public Boolean or(Boolean leftResult, Boolean rightResult) 
{
       return ROWS_MIGHT_MATCH;
     }
 
+    @Override
+    public <T> Boolean notStartsWith(BoundReference<T> ref, Literal<T> lit) {

Review comment:
       One way to reduce the code bloat / repetition would be to define a 
function `private boolean startsWith(ByteBuffer value, ByteBuffer prefix, 
Comparator<ByteBuffer> cmp)` (or possibly returning int to allow it to be used 
inside of the `startsWith` code), but that would ultimately just remove the 
repetition of the three lines of code that truncate the upper / lower bound to 
match the byte length of the prefix and then compare. The functions would still 
need to be different from each other.
   
   I considered doing that in this PR, but I felt that changing the 
implementation of `startsWith` at the same time would possibly be a bit too 
many changes for one PR (as this PR is already large). I'd be happy to clean it 
up to make the code a bit cleaner, especially in a follow up PR, but I don't 
think that we can use `!startsWith(ref, lit)` for really any of these cases 
other than `ResidualEvaluator` unfortunately 🙁 .




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

Reply via email to