JeonDaehong commented on code in PR #1145:
URL: https://github.com/apache/paimon-rust/pull/1145#discussion_r4306408883


##########
crates/paimon/src/spec/predicate.rs:
##########
@@ -284,6 +284,34 @@ pub enum PredicateOperator {
     NotBetween,
 }
 
+impl PredicateOperator {
+    /// The operator selecting the complement of the non-NULL rows, matching
+    /// Java `LeafFunction.negate`. `None` when there is no such operator.
+    fn negate(self) -> Option<Self> {
+        Some(match self {
+            Self::IsNull => Self::IsNotNull,
+            Self::IsNotNull => Self::IsNull,
+            Self::Eq => Self::NotEq,

Review Comment:
   Thanks, reproduced. Fixed in eff224e in two places:
   
   - `negate` now keeps `Not` for FLOAT/DOUBLE comparisons (see the NaN 
comment), so `NOT(f = 1)` stays out of the bitmap `NotEq` path.
   - The same wrong Top-1 also came from a plain `not_equal(f, 1)` or 
`greater_than(f, 1.5)`, with no negation involved. `is_exact_predicate` now 
treats FLOAT/DOUBLE leaves that the bitmap answers with every non-null row 
(`is_bitmap_floating_residual_sensitive_op`) as inexact, so they are refined 
before ranking.
   
   `floating_bitmap_candidates_are_refined_before_top_k` covers all three 
filters. Before the change it returned row 0, now row 1.
   



##########
crates/paimon/src/spec/predicate.rs:
##########
@@ -284,6 +284,34 @@ pub enum PredicateOperator {
     NotBetween,
 }
 
+impl PredicateOperator {
+    /// The operator selecting the complement of the non-NULL rows, matching
+    /// Java `LeafFunction.negate`. `None` when there is no such operator.
+    fn negate(self) -> Option<Self> {
+        Some(match self {
+            Self::IsNull => Self::IsNotNull,
+            Self::IsNotNull => Self::IsNull,
+            Self::Eq => Self::NotEq,
+            Self::NotEq => Self::Eq,
+            Self::Lt => Self::GtEq,

Review Comment:
   Thanks, reproduced: the partitioned read returned `[2]`. I kept `Not` for 
FLOAT/DOUBLE comparisons in `negate` rather than changing `datum_cmp`, since 
that would also change plain `<` / `>` on NaN partitions, which seems like a 
separate change. `Not(leaf)` is false only for NULL, and `NaN < 1.0` is false, 
so the NaN row is kept, as in Java.
   
   `negated_float_comparison_keeps_nan_rows` checks the read and 
`delete_by_predicate`, partitioned and not.
   



-- 
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]

Reply via email to