pvary commented on code in PR #14500:
URL: https://github.com/apache/iceberg/pull/14500#discussion_r2716740464
##########
api/src/main/java/org/apache/iceberg/expressions/InclusiveMetricsEvaluator.java:
##########
@@ -86,8 +113,12 @@ private class MetricsEvalVisitor extends
ExpressionVisitors.BoundVisitor<Boolean
private Map<Integer, Long> nanCounts = null;
private Map<Integer, ByteBuffer> lowerBounds = null;
private Map<Integer, ByteBuffer> upperBounds = null;
+ // Flag to use signed UUID comparator for backward compatibility.
+ // This is needed for the IN predicate because the comparator information
is lost
+ // when binding converts literals to a Set<T> of raw values.
+ private boolean useSignedUuidComparator = false;
- private boolean eval(ContentFile<?> file) {
+ private boolean eval(ContentFile<?> file, Expression expression, boolean
signedUuidMode) {
Review Comment:
We can add the `signedUuidMode` to the constructor as well.
It seems a bit more natural to me, than adding it as a parameter for the
`eval` method.
It doesn't seem a big deal, just a bit nicer code
##########
api/src/main/java/org/apache/iceberg/expressions/InclusiveMetricsEvaluator.java:
##########
@@ -86,8 +113,12 @@ private class MetricsEvalVisitor extends
ExpressionVisitors.BoundVisitor<Boolean
private Map<Integer, Long> nanCounts = null;
private Map<Integer, ByteBuffer> lowerBounds = null;
private Map<Integer, ByteBuffer> upperBounds = null;
+ // Flag to use signed UUID comparator for backward compatibility.
+ // This is needed for the IN predicate because the comparator information
is lost
+ // when binding converts literals to a Set<T> of raw values.
+ private boolean useSignedUuidComparator = false;
- private boolean eval(ContentFile<?> file) {
+ private boolean eval(ContentFile<?> file, Expression expression, boolean
signedUuidMode) {
Review Comment:
What if we add a parameter to the `MetricsEvalVisitor` constructor?
Then we can have something like this:
```
boolean result = new MetricsEvalVisitor(expr).eval(file);
if (false) {
boolean result = new MetricsEvalVisitor(signedUuidExpr).eval(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.
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]
