Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix pruning on not equal predicate #561

Merged
merged 4 commits into from
Jun 14, 2021
Merged

Fix pruning on not equal predicate #561

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
539b81b
Add test for not eq pruning
alamb Jun 14, 2021
840cb0f
Revert "#420: Support for not_eq predicate in pruning predicates (#544)"
alamb Jun 14, 2021
b1d348f
fix test
alamb Jun 14, 2021
54c4270
fix logic
alamb Jun 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 28 additions & 28 deletions datafusion/src/physical_optimizer/pruning.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -552,14 +552,6 @@ fn build_predicate_expression(
};
let corrected_op = expr_builder.correct_operator(op);
let statistics_expr = match corrected_op {
Operator::NotEq => {
// column != literal => (min, max) = literal => min > literal || literal > max
let min_column_expr = expr_builder.min_column_expr()?;
let max_column_expr = expr_builder.max_column_expr()?;
min_column_expr
.gt(expr_builder.scalar_expr().clone())
.or(expr_builder.scalar_expr().clone().gt(max_column_expr))
}
Operator::Eq => {
// column = literal => (min, max) = literal => min <= literal && literal <= max
// (column / 2) = 4 => (column_min / 2) <= 4 && 4 <= (column_max / 2)
Expand Down Expand Up @@ -937,26 +929,6 @@ mod tests {
Ok(())
}

#[test]
fn row_group_predicate_not_eq() -> Result<()> {
let schema = Schema::new(vec![Field::new("c1", DataType::Int32, false)]);
let expected_expr = "#c1_min Gt Int32(1) Or Int32(1) Gt #c1_max";

// test column on the left
let expr = col("c1").not_eq(lit(1));
let predicate_expr =
build_predicate_expression(&expr, &schema, &mut RequiredStatColumns::new())?;
assert_eq!(format!("{:?}", predicate_expr), expected_expr);

// test column on the right
let expr = lit(1).not_eq(col("c1"));
let predicate_expr =
build_predicate_expression(&expr, &schema, &mut RequiredStatColumns::new())?;
assert_eq!(format!("{:?}", predicate_expr), expected_expr);

Ok(())
}

#[test]
fn row_group_predicate_gt() -> Result<()> {
let schema = Schema::new(vec![Field::new("c1", DataType::Int32, false)]);
Expand Down Expand Up @@ -1190,6 +1162,34 @@ mod tests {
assert_eq!(result, expected);
}

#[test]
fn prune_not_eq_data() {
let schema = Arc::new(Schema::new(vec![Field::new("s1", DataType::Utf8, true)]));

// Prune using s2 != 'M'
let expr = col("s1").not_eq(lit("M"));

let statistics = TestStatistics::new().with(
"s1",
ContainerStats::new_utf8(
vec![Some("A"), Some("A"), Some("N"), None, Some("A")], // min
vec![Some("Z"), Some("L"), Some("Z"), None, None], // max
),
);

// s1 [A, Z] ==> might have values that pass predicate
// s1 [A, L] ==> all rows pass the predicate
// s1 [N, Z] ==> all rows pass the predicate
// No stats for s2 ==> some rows could pass
// s2 [3, None] (null max) ==> some rows could pass

let p = PruningPredicate::try_new(&expr, schema).unwrap();
let result = p.prune(&statistics).unwrap();
let expected = vec![true, true, true, true, true];
Copy link
Contributor Author

@alamb alamb Jun 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On master this test fails thusly:

---- physical_optimizer::pruning::tests::prune_not_eq_data stdout ----
thread 'physical_optimizer::pruning::tests::prune_not_eq_data' panicked at 'assertion failed: `(left == right)`
  left: `[false, true, true, false, true, true]`,
 right: `[true, true, true, false, true, true]`', datafusion/src/physical_optimizer/pruning.rs:1218:9

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(aka it incorrectly prunes out the range of A -> Z)


assert_eq!(result, expected);
}

/// Creates setup for boolean chunk pruning
///
/// For predicate "b1" (boolean expr)
Expand Down