feat: Add StrictMetricsEvaluator
#963
Conversation
|
Tests will be added tomorrow. |
…athanc-n/iceberg-rust into add-strict-metrics-evaluator
…athanc-n/iceberg-rust into add-strict-metrics-evaluator
|
Tests have been added, this implementation follows the pyiceberg implementation. cc @Fokko @liurenjie1024 @ZENOTME |
| return ROWS_MUST_MATCH; | ||
| } | ||
|
|
||
| let mut evaluator = Self::new(data_file, schema); |
There was a problem hiding this comment.
Still learning rust, why is this one mutable?
There was a problem hiding this comment.
The bound predicate visitor takes in a mutable reference of the evaluator. in rust you have to declare as mutable as well as pass it in with mut to let the compiler know the function can modify
Co-authored-by: Fokko Driesprong <[email protected]>
Co-authored-by: Fokko Driesprong <[email protected]>
Co-authored-by: Fokko Driesprong <[email protected]>
…athanc-n/iceberg-rust into add-strict-metrics-evaluator
|
@Fokko Thanks for the review. I got a bit mixed up with the logic with only nulls vs may contain null 😆. Should be good now |
There was a problem hiding this comment.
Thanks @jonathanc-n for this great pr, and all the tests! Just some minor suggestions, others looks great!
| data_file: &'a DataFile, | ||
| } | ||
|
|
||
| impl<'a> StrictMetricsEvaluator<'a> { |
There was a problem hiding this comment.
Seems it has a lot of duplication with InclusiveMetricsEvaluator, maybe we could unify them in future.
|
@liurenjie1024 @Fokko Thank you for the reviews, problems should be resolved. |
part of #735. Added
StrictMetricsEvaluator.