wgtmac commented on code in PR #956:
URL: https://github.com/apache/iceberg-cpp/pull/956#discussion_r4250211166
##########
src/iceberg/expression/evaluator.cc:
##########
@@ -140,6 +141,7 @@ Result<std::unique_ptr<Evaluator>> Evaluator::Make(const
Schema& schema,
std::shared_ptr<Expression>
unbound,
bool case_sensitive) {
ICEBERG_ASSIGN_OR_RAISE(auto bound_expr, Binder::Bind(schema, unbound,
case_sensitive));
+ ICEBERG_ASSIGN_OR_RAISE(bound_expr, RewriteNot::Visit(bound_expr));
Review Comment:
These two evaluators now disagree on the same row.
Suppose a file uses `identity(key)` partitioning. Its partition value is
null, and it contains this row:
| key | value |
| --- | --- |
| null | 7 |
The filter is `NOT(key < 100) OR value = 5`.
`Evaluator` rewrites the filter to `key >= 100 OR value = 5`. Both checks
return false for this row, so the row does not match.
`ResidualEvaluator` still checks the original filter:
1. `key < 100` returns false for the null partition value.
2. `NOT(false)` becomes true.
3. `true OR value = 5` becomes true for the whole file.
This drops the `value = 5` condition. If file statistics cannot reject the
file, the reader gets a `true` residual. Any later filter using that residual
will keep the row, even though `Evaluator` rejects it.
--
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]
