Skip to content

Commit

Permalink
Add tests for conjunction/disjunction, make API uniform
Browse files Browse the repository at this point in the history
  • Loading branch information
alamb committed Oct 15, 2022
1 parent 34155f8 commit 09f3cf4
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 7 deletions.
2 changes: 1 addition & 1 deletion benchmarks/src/bin/parquet_filter_pushdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ async fn run_benchmarks(
col("response_status").eq(lit(403_u16)),
)),
// Many filters
disjunction(&[
disjunction([
col("request_method").not_eq(lit("GET")),
col("response_status").eq(lit(400_u16)),
// TODO this fails in the FilterExec with Error: Internal("The type of Dictionary(Int32, Utf8) = Utf8 of binary physical should be same")
Expand Down
42 changes: 36 additions & 6 deletions datafusion/optimizer/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,12 +150,8 @@ pub fn conjunction(filters: impl IntoIterator<Item = Expr>) -> Option<Expr> {
/// logical OR.
///
/// Returns None if the filters array is empty.
pub fn disjunction(filters: &[Expr]) -> Option<Expr> {
if filters.is_empty() {
return None;
}

filters.iter().cloned().reduce(datafusion_expr::or)
pub fn disjunction(filters: impl IntoIterator<Item = Expr>) -> Option<Expr> {
filters.into_iter().reduce(|accum, expr| accum.or(expr))
}

/// Recursively un-alias an expressions
Expand Down Expand Up @@ -532,6 +528,40 @@ mod tests {
);
}

#[test]
fn test_conjunction_empty() {
assert_eq!(conjunction(vec![]), None);
}

#[test]
fn test_conjunction() {
// `[A, B, C]`
let expr = conjunction(vec![col("a"), col("b"), col("c")]);

// --> `(A AND B) AND C`
assert_eq!(expr, Some(col("a").and(col("b")).and(col("c"))));

// which is different than `A AND (B AND C)`
assert_ne!(expr, Some(col("a").and(col("b").and(col("c")))));
}

#[test]
fn test_disjunction_empty() {
assert_eq!(disjunction(vec![]), None);
}

#[test]
fn test_disjunction() {
// `[A, B, C]`
let expr = disjunction(vec![col("a"), col("b"), col("c")]);

// --> `(A OR B) OR C`
assert_eq!(expr, Some(col("a").or(col("b")).or(col("c"))));

// which is different than `A OR (B OR C)`
assert_ne!(expr, Some(col("a").or(col("b").or(col("c")))));
}

#[test]
fn test_split_conjunction_owned_or() {
let expr = col("a").eq(lit(5)).or(col("b"));
Expand Down

0 comments on commit 09f3cf4

Please sign in to comment.