YinZheng-Sun commented on PR #956: URL: https://github.com/apache/iceberg-cpp/pull/956#issuecomment-6093522662
> I think you're right that this indeed is a problem and different impls are not consistent: > > impl `x < v` on null NOT > Java true (`Literals` nulls-first) rewritten in every pruning evaluator > Go true (`nullsFirstCmp`) rewritten in every pruning evaluator (`RewriteNotExpr`) > Rust false rewritten once in the scan; evaluator errors on a leftover `Not` > Python false metrics evaluator raises on a leftover `Not` > Perhaps in the short term (in this PR), we can keep `RewriteNot` in the pruning filter and make the evaluator use the same rule. Apply `RewriteNot` when building the `Evaluator`, or error on a leftover `Not` like iceberg-rust. Then `not(key < 100)` is `key >= 100` for every value, so pruning keeps full strength on nullable columns. > > In the long run, we can give the evaluator three states for null comparisons (null-first, null-last, unknown), so `not(unknown)` stays unknown. Pruning stays sound and semantics match SQL exactly. > > WDYT? @YinZheng-Sun Thanks for the suggestion. I will apply the short-term approach firstly: Evaluator::Make() applies RewriteNot, so row evaluation and row-group pruning use the same normalized predicates. -- 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]
