huan233usc commented on code in PR #16531:
URL: https://github.com/apache/iceberg/pull/16531#discussion_r3320787290
##########
spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/source/BaseSparkScanBuilder.java:
##########
@@ -164,9 +164,19 @@ public Predicate[] pushPredicates(Predicate[] predicates) {
Binder.bind(projection.asStruct(), expr, caseSensitive);
expressions.add(expr);
pushablePredicates.add(predicate);
+ } else {
+ // if the full predicate can't be converted, try to extract
convertible conjuncts
+ // from compound AND predicates for file-level pruning
+ Expression partialExpr = convertAndConjuncts(predicate);
Review Comment:
Should we actually move the logic to `SparkV2Filters.convert`
e.g. make this line
https://github.com/apache/iceberg/blob/main/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkV2Filters.java#L281-L290
to
```
case AND: {
And andPredicate = (And) predicate;
Expression left = convert(andPredicate.left());
Expression right = convert(andPredicate.right());
if (left != null && right != null) {
return and(left, right);
} else if (left != null) {
return left;
} else if (right != null) {
return right;
}
return null;
}
```
By doing so, we still have a single place to handle the spark->iceberg
expression conversion so all caller could benefit this change
--
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]
