pvary commented on code in PR #14500:
URL: https://github.com/apache/iceberg/pull/14500#discussion_r2716608787


##########
api/src/main/java/org/apache/iceberg/expressions/InclusiveMetricsEvaluator.java:
##########
@@ -74,7 +88,20 @@ public InclusiveMetricsEvaluator(Schema schema, Expression 
unbound, boolean case
    */
   public boolean eval(ContentFile<?> file) {
     // TODO: detect the case where a column is missing from the file using 
file's max field id.
-    return new MetricsEvalVisitor().eval(file);
+    boolean result = new MetricsEvalVisitor().eval(file, expr, false);
+
+    // If the RFC-compliant evaluation says rows might match, or there's no 
signed UUID expression,
+    // return the result.
+    if (result || signedUuidExpr == null) {
+      return result;
+    }
+
+    // Always try with signed UUID comparator as a fallback. There is no 
reliable way to detect
+    // which comparator was used when the file's column metrics were written.

Review Comment:
   Sure - another PR.
   This is a correctness fix, we can improve on the performance later



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

Reply via email to