bodduv commented on code in PR #14500:
URL: https://github.com/apache/iceberg/pull/14500#discussion_r2792331346
##########
api/src/main/java/org/apache/iceberg/expressions/ManifestEvaluator.java:
##########
@@ -74,22 +86,37 @@ private ManifestEvaluator(PartitionSpec spec, Expression
partitionFilter, boolea
* @return false if the file cannot contain rows that match the expression,
true otherwise.
*/
public boolean eval(ManifestFile manifest) {
- return new ManifestEvalVisitor().eval(manifest);
+ boolean result = new ManifestEvalVisitor().eval(manifest, 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 manifest's partition field summaries
were written.
+ return new ManifestEvalVisitor().eval(manifest, true);
}
private static final boolean ROWS_MIGHT_MATCH = true;
private static final boolean ROWS_CANNOT_MATCH = false;
private class ManifestEvalVisitor extends BoundExpressionVisitor<Boolean> {
private List<PartitionFieldSummary> stats = null;
+ // Flag to use signed UUID comparator for backward compatibility.
+ // This is needed for the IN predicate because the comparator information
is lost
Review Comment:
Yes, for `NOT IN` we are only evaluating a specific case: when min and max
are same, we are checking if this value is one of (equality check) given values
in the list.
##########
api/src/main/java/org/apache/iceberg/expressions/Literals.java:
##########
@@ -622,10 +635,23 @@ public <T> Literal<T> to(Type type) {
return null;
}
+ @Override
+ public Comparator<UUID> comparator() {
+ return useSignedComparator ? SIGNED_CMP : RFC_CMP;
+ }
+
@Override
protected Type.TypeID typeId() {
return Type.TypeID.UUID;
}
+
+ /**
+ * Creates a new UUIDLiteral with the signed comparator for backward
compatibility with files
+ * written before RFC-compliant UUID comparisons were introduced.
+ */
+ UUIDLiteral withSignedComparator() {
Review Comment:
There is a `useSignedComparator:boolean` member.
--
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]
