pvary commented on code in PR #14500:
URL: https://github.com/apache/iceberg/pull/14500#discussion_r2704800758
##########
api/src/main/java/org/apache/iceberg/expressions/ManifestEvaluator.java:
##########
@@ -74,22 +89,53 @@ 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, 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 manifest's partition field summaries
were written.
+ // The signedUuidExpr has literals with signed comparators for lt/gt/eq
predicates.
+ // For IN predicates, we pass signedUuidMode=true so comparatorForIn()
returns signed
+ // comparator.
+ return new ManifestEvalVisitor().eval(manifest, signedUuidExpr, 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
+ // when binding converts literals to a Set<T> of raw values.
+ private boolean useSignedUuidComparator = false;
- private boolean eval(ManifestFile manifest) {
+ private boolean eval(ManifestFile manifest, Expression expression, boolean
signedUuidMode) {
this.stats = manifest.partitions();
+ this.useSignedUuidComparator = signedUuidMode;
if (stats == null) {
return ROWS_MIGHT_MATCH;
}
- return ExpressionVisitors.visitEvaluator(expr, this);
+ return ExpressionVisitors.visitEvaluator(expression, this);
+ }
+
+ /**
+ * Returns the appropriate comparator for the given reference. This is
needed for the IN
+ * predicate because the comparator information is lost when binding
converts literals to a Set
+ * of raw values. For other predicates, the literal carries the comparator.
+ */
+ @SuppressWarnings("unchecked")
+ private <T> Comparator<T> comparatorForIn(BoundReference<T> ref) {
+ if (useSignedUuidComparator && ref.type().typeId() == Type.TypeID.UUID) {
+ return (Comparator<T>) Comparators.signedUUIDs();
+ }
+ return ref.comparator();
Review Comment:
nit: newline
--
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]
