clesaec commented on code in PR #6893:
URL: https://github.com/apache/iceberg/pull/6893#discussion_r1113984216
##########
parquet/src/main/java/org/apache/iceberg/parquet/ParquetBloomRowGroupFilter.java:
##########
@@ -116,89 +124,99 @@ private boolean eval(
// If the filter's column set doesn't overlap with any bloom filter
columns, exit early with
// ROWS_MIGHT_MATCH
if (filterRefs.size() > 0 && Sets.intersection(fieldsWithBloomFilter,
filterRefs).isEmpty()) {
- return ROWS_MIGHT_MATCH;
+ return expr;
}
- return ExpressionVisitors.visitEvaluator(expr, this);
+ return ExpressionVisitors.visit(expr, this);
}
@Override
- public Boolean alwaysTrue() {
- return ROWS_MIGHT_MATCH; // all rows match
+ public Expression alwaysTrue() {
+ return ROWS_ALL_MATCH; // all rows match
}
@Override
- public Boolean alwaysFalse() {
+ public Expression alwaysFalse() {
return ROWS_CANNOT_MATCH; // all rows fail
}
@Override
- public Boolean not(Boolean result) {
+ public Expression not(Expression result) {
// not() should be rewritten by RewriteNot
// bloom filter is based on hash and cannot eliminate based on not
throw new UnsupportedOperationException("This path shouldn't be
reached.");
}
@Override
- public Boolean and(Boolean leftResult, Boolean rightResult) {
- return leftResult && rightResult;
+ public Expression and(Supplier<Expression> left, Supplier<Expression>
right) {
+ Expression leftResult = left.get();
Review Comment:
As on methods "and" & "or", you always evaluate, at least left expression,
it's like using Suppliers is not relevant here; and just having
`BloomEvalVisitor extends BoundExpressionVisitor<Boolean>` to
`BloomEvalVisitor extends BoundExpressionVisitor<Expression>` would be
enough to delay expression evaluation itself.
(or just have `public abstract static class FindsResidualVisitor extends
BoundExpressionVisitor<Expression>`)
WDYT ?
--
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]
