fix: fix NotEq case in anticipation of non-literals (#1260)

The code as written fails when the "other" [min, max] interval has size
greater than 1.

Call the left-hand-side's interval: `[lmin, lmax]` and the
right-hand-side's interval: `[rmin, rmax]`. The current expression is:

    (lmin == rmin) && (rmin == lmax)
    ||
    (lmin == rmax) && (rmax == lmax)

By transitivity, in either conjunction, lmin == lmax. That means the
column has a single known value: lmin (equivalently: lmax).

If the right-hand-side is only ever literals or expressions thereof, its
interval is always `[x, x]`. In that case, this expression is tantamount
to: is the column constant and equal to `x`?

If the right-hand-side is either another column or some
non-deterministic expression, the interval could be, for example: `[10,
11]`. If the column is known to be the constant value `10` (i.e. its min
and max are 10), we cannot prune this chunk! The rows where the
right-hand-side is 11 would satisfy the inequality.

I'm also open to: `(lmin == lmax) && (lmin == rmin)` with a comment
indicating that, because we only have literals, rmin is necessarily
equal to rmax and thus rmin _is_ the, known, right-hand-side value.

Author: danking

vortex-serde/src/layouts/pruning.rs

@@ -200,23 +200,28 @@ impl<'a> PruningPredicateRewriter<'a> {
                 let replaced_max = self.rewrite_other_exp(Stat::Max);
                 let replaced_min = self.rewrite_other_exp(Stat::Min);
 
-                // In case of other_exp is literal both sides of AND will be the same expression
+                let column_value_is_single_known_value = Arc::new(BinaryExpr::new(
+                    min_col.clone(),
+                    Operator::Eq,
+                    max_col.clone(),
+                ));
+                let column_value = min_col;
+
+                let other_value_is_single_known_value = Arc::new(BinaryExpr::new(
+                    replaced_min.clone(),
+                    Operator::Eq,
+                    replaced_max.clone(),
+                ));
+                let other_value = replaced_min;
+
                 Some(Arc::new(BinaryExpr::new(
                     Arc::new(BinaryExpr::new(
-                        Arc::new(BinaryExpr::new(
-                            min_col.clone(),
-                            Operator::Eq,
-                            replaced_min.clone(),
-                        )),
-                        Operator::And,
-                        Arc::new(BinaryExpr::new(replaced_min, Operator::Eq, max_col.clone())),
-                    )),
-                    Operator::Or,
-                    Arc::new(BinaryExpr::new(
-                        Arc::new(BinaryExpr::new(min_col, Operator::Eq, replaced_max.clone())),
+                        column_value_is_single_known_value,
                         Operator::And,
-                        Arc::new(BinaryExpr::new(replaced_max, Operator::Eq, max_col)),
+                        other_value_is_single_known_value,
                     )),
+                    Operator::And,
+                    Arc::new(BinaryExpr::new(column_value, Operator::Eq, other_value)),
                 )))
             }
             Operator::Gt | Operator::Gte => {
@@ -394,30 +399,23 @@ mod tests {
                 Arc::new(BinaryExpr::new(
                     Arc::new(Column::new(stat_column_name(&column, Stat::Min))),
                     Operator::Eq,
-                    Arc::new(Column::new(stat_column_name(&other_col, Stat::Min))),
+                    Arc::new(Column::new(stat_column_name(&column, Stat::Max))),
                 )),
                 Operator::And,
                 Arc::new(BinaryExpr::new(
                     Arc::new(Column::new(stat_column_name(&other_col, Stat::Min))),
                     Operator::Eq,
-                    Arc::new(Column::new(stat_column_name(&column, Stat::Max))),
+                    Arc::new(Column::new(stat_column_name(&other_col, Stat::Max))),
                 )),
             )),
-            Operator::Or,
+            Operator::And,
             Arc::new(BinaryExpr::new(
-                Arc::new(BinaryExpr::new(
-                    Arc::new(Column::new(stat_column_name(&column, Stat::Min))),
-                    Operator::Eq,
-                    Arc::new(Column::new(stat_column_name(&other_col, Stat::Max))),
-                )),
-                Operator::And,
-                Arc::new(BinaryExpr::new(
-                    Arc::new(Column::new(stat_column_name(&other_col, Stat::Max))),
-                    Operator::Eq,
-                    Arc::new(Column::new(stat_column_name(&column, Stat::Max))),
-                )),
+                Arc::new(Column::new(stat_column_name(&column, Stat::Min))),
+                Operator::Eq,
+                Arc::new(Column::new(stat_column_name(&other_col, Stat::Min))),
             )),
         ));
+
         assert_eq!(*converted, *expected_expr.as_any());
     }