fix rewrite_not to process complex nested not

Author: ZENOTME

crates/iceberg/src/expr/predicate.rs

@@ -28,6 +28,8 @@ use itertools::Itertools;
 use serde::{Deserialize, Serialize};
 
 use crate::error::Result;
+use crate::expr::visitors::predicate_visitor::visit;
+use crate::expr::visitors::rewrite_not::RewriteNotVisitor;
 use crate::expr::{Bind, BoundReference, PredicateOperator, Reference};
 use crate::spec::{Datum, PrimitiveLiteral, SchemaRef};
 use crate::{Error, ErrorKind};
@@ -652,29 +654,8 @@ impl Predicate {
     /// assert_eq!(&format!("{result}"), "a >= 5");
     /// ```
     pub fn rewrite_not(self) -> Predicate {
-        match self {
-            Predicate::And(expr) => {
-                let [left, right] = expr.inputs;
-                let new_left = Box::new(left.rewrite_not());
-                let new_right = Box::new(right.rewrite_not());
-                Predicate::And(LogicalExpression::new([new_left, new_right]))
-            }
-            Predicate::Or(expr) => {
-                let [left, right] = expr.inputs;
-                let new_left = Box::new(left.rewrite_not());
-                let new_right = Box::new(right.rewrite_not());
-                Predicate::Or(LogicalExpression::new([new_left, new_right]))
-            }
-            Predicate::Not(expr) => {
-                let [inner] = expr.inputs;
-                inner.negate()
-            }
-            Predicate::Unary(expr) => Predicate::Unary(expr),
-            Predicate::Binary(expr) => Predicate::Binary(expr),
-            Predicate::Set(expr) => Predicate::Set(expr),
-            Predicate::AlwaysTrue => Predicate::AlwaysTrue,
-            Predicate::AlwaysFalse => Predicate::AlwaysFalse,
-        }
+        visit(&mut RewriteNotVisitor::new(), &self)
+            .expect("RewriteNotVisitor guarantees always success")
     }
 }
 
@@ -1466,4 +1447,29 @@ mod tests {
         assert_eq!(&format!("{bound_expr}"), r#"True"#);
         test_bound_predicate_serialize_diserialize(bound_expr);
     }
+
+    #[test]
+    fn test_rewrite_not_deeply_nested() {
+        // Test nested expression: not((not((not(ref(name="bar") < 40) and ref(name="bar") < 40)) and ref(name="bar") < 40))
+        // Expected rewrite not result: ((bar >= 40) AND (bar < 40)) OR (bar >= 40)
+        let complex_expression = Reference::new("bar")
+            .less_than(Datum::int(40))
+            .not()
+            .and(Reference::new("bar").less_than(Datum::int(40)))
+            .not()
+            .and(Reference::new("bar").less_than(Datum::int(40)))
+            .not();
+
+        let expected = Reference::new("bar")
+            .greater_than_or_equal_to(Datum::int(40))
+            .and(Reference::new("bar").less_than(Datum::int(40)))
+            .or(Reference::new("bar").greater_than_or_equal_to(Datum::int(40)));
+
+        let result = complex_expression.rewrite_not();
+
+        assert_eq!(result, expected);
+
+        let result_str = format!("{result}");
+        assert_eq!(&result_str, "((bar >= 40) AND (bar < 40)) OR (bar >= 40)");
+    }
 }

crates/iceberg/src/expr/visitors/mod.rs

@@ -21,6 +21,8 @@ pub(crate) mod inclusive_metrics_evaluator;
 pub(crate) mod inclusive_projection;
 pub(crate) mod manifest_evaluator;
 pub(crate) mod page_index_evaluator;
+pub(crate) mod predicate_visitor;
+pub(crate) mod rewrite_not;
 pub(crate) mod row_group_metrics_evaluator;
 pub(crate) mod strict_metrics_evaluator;
 pub(crate) mod strict_projection;