Treat!= filtering of numerical column as the inverse of == (aka. outer-NOT and ALL semantics) (#11372)

Author: abey79

crates/viewer/re_dataframe_ui/src/filters/numerical.rs

@@ -7,7 +7,7 @@ use arrow::datatypes::DataType;
 use datafusion::common::{Column, Result as DataFusionResult, exec_err};
 use datafusion::logical_expr::{
     ArrayFunctionArgument, ArrayFunctionSignature, ColumnarValue, Expr, ScalarFunctionArgs,
-    ScalarUDF, ScalarUDFImpl, Signature, TypeSignature, Volatility, col, lit,
+    ScalarUDF, ScalarUDFImpl, Signature, TypeSignature, Volatility, col, lit, not,
 };
 use strum::VariantArray as _;
 
@@ -57,8 +57,9 @@ impl ComparisonOperator {
         T: PartialOrd + PartialEq + Copy,
     {
         match self {
-            Self::Eq => left == right,
-            Self::Ne => left != right,
+            // Consistent with other column types, we handle `Ne` as an outer-NOT on `Eq`, so the
+            // `NOT` is applied in the `as_filter_expression` functions.
+            Self::Eq | Self::Ne => left == right,
             Self::Lt => left < right,
             Self::Le => left <= right,
             Self::Gt => left > right,
@@ -144,8 +145,17 @@ impl IntFilter {
         };
 
         let udf = ScalarUDF::new_from_impl(IntFilterUdf::new(self.operator, rhs_value));
+        let expr = udf.call(vec![col(column.clone())]);
 
-        udf.call(vec![col(column.clone())])
+        // Consistent with other column types, we treat `Ne` as an outer-NOT, so we applies it here
+        // while the UDF handles `Ne` and `Eq` in the same way (see `ComparisonOperator::apply`).
+        let apply_should_invert_expression_semantics = self.operator == ComparisonOperator::Ne;
+
+        if apply_should_invert_expression_semantics {
+            not(expr.clone()).or(expr.is_null())
+        } else {
+            expr
+        }
     }
 }
 
@@ -227,7 +237,17 @@ impl FloatFilter {
 
         let udf = ScalarUDF::new_from_impl(FloatFilterUdf::new(self.operator, rhs_value));
 
-        udf.call(vec![col(column.clone())])
+        let expr = udf.call(vec![col(column.clone())]);
+
+        // Consistent with other column types, we treat `Ne` as an outer-NOT, so we applies it here
+        // while the UDF handles `Ne` and `Eq` in the same way (see `ComparisonOperator::apply`).
+        let apply_should_invert_expression_semantics = self.operator == ComparisonOperator::Ne;
+
+        if apply_should_invert_expression_semantics {
+            not(expr.clone()).or(expr.is_null())
+        } else {
+            expr
+        }
     }
 }
 

crates/viewer/re_dataframe_ui/src/filters/string.rs

@@ -73,9 +73,10 @@ impl StringFilter {
 
         // The udf treats `DoesNotContains` in the same way as `Contains`, so we must apply an
         // outer `NOT` (or null) operation. This way, both operators yield complementary results.
-        let apply_any_or_null_semantics = self.operator() == StringOperator::DoesNotContain;
+        let apply_should_invert_expression_semantics =
+            self.operator() == StringOperator::DoesNotContain;
 
-        if apply_any_or_null_semantics {
+        if apply_should_invert_expression_semantics {
             not(expr.clone()).or(expr.is_null())
         } else {
             expr

crates/viewer/re_dataframe_ui/tests/snapshots/filter_tests__float_compares@nulls_ne_4.snap

@@ -31,6 +31,7 @@ TestResult {
     [
       1.0,
       2.0,
+      null,
       5.0,
     ],
 }

crates/viewer/re_dataframe_ui/tests/snapshots/filter_tests__float_lists@ne_2.0.snap

@@ -67,12 +67,6 @@ TestResult {
     filtered: ListArray
     [
       PrimitiveArray<Float64>
-    [
-      1.0,
-      2.0,
-      3.0,
-    ],
-      PrimitiveArray<Float64>
     [
       4.0,
       5.0,
@@ -83,12 +77,6 @@ TestResult {
       7.0,
       4.0,
       9.0,
-    ],
-      PrimitiveArray<Float64>
-    [
-      5.0,
-      2.0,
-      1.0,
     ],
     ],
 }

crates/viewer/re_dataframe_ui/tests/snapshots/filter_tests__float_lists@nulls_ne_2.0.snap

@@ -86,12 +86,6 @@ TestResult {
     filtered: ListArray
     [
       PrimitiveArray<Float64>
-    [
-      1.0,
-      2.0,
-      3.0,
-    ],
-      PrimitiveArray<Float64>
     [
       4.0,
       5.0,
@@ -103,24 +97,9 @@ TestResult {
       4.0,
       9.0,
     ],
-      PrimitiveArray<Float64>
-    [
-      5.0,
-      2.0,
-      1.0,
-    ],
-      PrimitiveArray<Float64>
-    [
       null,
-      1.0,
-      2.0,
-      3.0,
-    ],
       PrimitiveArray<Float64>
     [
-      1.0,
-      2.0,
-      3.0,
       null,
     ],
     ],

crates/viewer/re_dataframe_ui/tests/snapshots/filter_tests__int_compares@nulls_ne_4.snap

@@ -31,6 +31,7 @@ TestResult {
     [
       1,
       2,
+      null,
       5,
     ],
 }

crates/viewer/re_dataframe_ui/tests/snapshots/filter_tests__int_lists@ne_2.snap

@@ -67,12 +67,6 @@ TestResult {
     filtered: ListArray
     [
       PrimitiveArray<Int64>
-    [
-      1,
-      2,
-      3,
-    ],
-      PrimitiveArray<Int64>
     [
       4,
       5,
@@ -83,12 +77,6 @@ TestResult {
       7,
       4,
       9,
-    ],
-      PrimitiveArray<Int64>
-    [
-      5,
-      2,
-      1,
     ],
     ],
 }

crates/viewer/re_dataframe_ui/tests/snapshots/filter_tests__int_lists@nulls_ne_2.snap

@@ -86,12 +86,6 @@ TestResult {
     filtered: ListArray
     [
       PrimitiveArray<Int64>
-    [
-      1,
-      2,
-      3,
-    ],
-      PrimitiveArray<Int64>
     [
       4,
       5,
@@ -103,24 +97,9 @@ TestResult {
       4,
       9,
     ],
-      PrimitiveArray<Int64>
-    [
-      5,
-      2,
-      1,
-    ],
-      PrimitiveArray<Int64>
-    [
       null,
-      1,
-      2,
-      3,
-    ],
       PrimitiveArray<Int64>
     [
-      1,
-      2,
-      3,
       null,
     ],
     ],