Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
133 changes: 130 additions & 3 deletions datafusion/core/src/physical_plan/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use arrow::error::Result as ArrowResult;
use arrow::record_batch::RecordBatch;
use datafusion_expr::Operator;
use datafusion_physical_expr::expressions::BinaryExpr;
use datafusion_physical_expr::split_conjunction;
use datafusion_physical_expr::{split_conjunction, AnalysisContext};

use log::debug;

Expand Down Expand Up @@ -168,9 +168,27 @@ impl ExecutionPlan for FilterExec {
Some(self.metrics.clone_inner())
}

/// The output statistics of a filtering operation are unknown
/// The output statistics of a filtering operation can be estimated if the
/// predicate's selectivity value can be determined for the incoming data.
fn statistics(&self) -> Statistics {
Statistics::default()
let input_stats = self.input.statistics();
let analysis_ctx =
AnalysisContext::from_statistics(self.input.schema().as_ref(), &input_stats);

let predicate_selectivity = self
.predicate
.boundaries(&analysis_ctx)
.and_then(|bounds| bounds.selectivity);

match predicate_selectivity {
Some(selectivity) => Statistics {
num_rows: input_stats
.num_rows
.map(|num_rows| (num_rows as f64 * selectivity).ceil() as usize),
..Default::default()
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should explicitly list out is_exact: false here? Default::default() gets the same result but maybe being explicit would be better 🤔

},
None => Statistics::default(),
}
}
}

Expand Down Expand Up @@ -282,9 +300,14 @@ mod tests {
use crate::physical_plan::{collect, with_new_children_if_necessary};
use crate::prelude::SessionContext;
use crate::test;
use crate::test::exec::StatisticsExec;
use crate::test_util;
use arrow::datatypes::{DataType, Field, Schema};
use datafusion_common::ColumnStatistics;
use datafusion_common::ScalarValue;
use datafusion_expr::Operator;
use std::iter::Iterator;
use std::sync::Arc;

#[tokio::test]
async fn simple_predicate() -> Result<()> {
Expand Down Expand Up @@ -380,4 +403,108 @@ mod tests {

Ok(())
}

#[tokio::test]
async fn test_filter_statistics_basic_expr() -> Result<()> {
// Table:
// a: min=1, max=100
let schema = Schema::new(vec![Field::new("a", DataType::Int32, false)]);
let input = Arc::new(StatisticsExec::new(
Statistics {
num_rows: Some(100),
column_statistics: Some(vec![ColumnStatistics {
min_value: Some(ScalarValue::Int32(Some(1))),
max_value: Some(ScalarValue::Int32(Some(100))),
..Default::default()
}]),
..Default::default()
},
schema.clone(),
));

// a <= 25
let predicate: Arc<dyn PhysicalExpr> =
binary(col("a", &schema)?, Operator::LtEq, lit(25i32), &schema)?;

// WHERE a <= 25
let filter: Arc<dyn ExecutionPlan> =
Arc::new(FilterExec::try_new(predicate, input)?);

let statistics = filter.statistics();
assert_eq!(statistics.num_rows, Some(25));
Copy link
Contributor

Choose a reason for hiding this comment

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

👨‍🍳 👌

Very nice


Ok(())
}

#[tokio::test]
#[ignore]
// This test requires propagation of column boundaries from the comparison analysis
// to the analysis context. This is not yet implemented.
async fn test_filter_statistics_column_level_basic_expr() -> Result<()> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alamb while working on this, I've noticed the initial application of propagation of new column limits. Since we don't have an API to represent changes to the boundaries during an expression's analysis (like a becomes [1, 25] in the example below) we can't generate the column_statistics which is essentially rendering nested join optimizations unusable (and potentially any other analysis that needs column level stats).

This doesn't mean it is completely ineffecttive as is, since we can at least find the cardinality of filter itself and do the local filter <-> table switch in the case below. But I think it might make sense to at least investigate potential ways to deal with this.

image

Copy link
Contributor Author

@isidentical isidentical Nov 11, 2022

Choose a reason for hiding this comment

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

I have a simple solution for this problem (isidentical#5) that essentially implements a much more narrow-scoped version of the apply() API from the previous iteration. It doesn't add any new methods to the physical expressions, but it still shares a mutable context reference (I kind of resonate this with other similiar APIs in datafusion like expr_to_columns) so not sure if the same reservations still apply. I'd be really interested in your feedback on this.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand what about this test requires column level analysis -- your figure has a join in it, but thietest just seems to be the same as test_filter_statistics_basic_expr above it. I will look at isidentical#5 shortly

// Table:
// a: min=1, max=100
let schema = Schema::new(vec![Field::new("a", DataType::Int32, false)]);
let input = Arc::new(StatisticsExec::new(
Statistics {
num_rows: Some(100),
column_statistics: Some(vec![ColumnStatistics {
min_value: Some(ScalarValue::Int32(Some(1))),
max_value: Some(ScalarValue::Int32(Some(100))),
..Default::default()
}]),
..Default::default()
},
schema.clone(),
));

// a <= 25
let predicate: Arc<dyn PhysicalExpr> =
binary(col("a", &schema)?, Operator::LtEq, lit(25i32), &schema)?;

// WHERE a <= 25
let filter: Arc<dyn ExecutionPlan> =
Arc::new(FilterExec::try_new(predicate, input)?);

let statistics = filter.statistics();
assert_eq!(statistics.num_rows, Some(25));
assert_eq!(
statistics.column_statistics,
Some(vec![ColumnStatistics {
min_value: Some(ScalarValue::Int32(Some(1))),
max_value: Some(ScalarValue::Int32(Some(25))),
..Default::default()
}])
);

Ok(())
}

#[tokio::test]
async fn test_filter_statistics_when_input_stats_missing() -> Result<()> {
// Table:
// a: min=???, max=??? (missing)
let schema = Schema::new(vec![Field::new("a", DataType::Int32, false)]);
let input = Arc::new(StatisticsExec::new(
Statistics {
column_statistics: Some(vec![ColumnStatistics {
..Default::default()
}]),
..Default::default()
},
schema.clone(),
));

// a <= 25
let predicate: Arc<dyn PhysicalExpr> =
binary(col("a", &schema)?, Operator::LtEq, lit(25i32), &schema)?;

// WHERE a <= 25
let filter: Arc<dyn ExecutionPlan> =
Arc::new(FilterExec::try_new(predicate, input)?);

let statistics = filter.statistics();
assert_eq!(statistics.num_rows, None);

Ok(())
}
}
5 changes: 3 additions & 2 deletions datafusion/core/tests/statistics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,9 @@ async fn sql_filter() -> Result<()> {
.await
.unwrap();

// with a filtering condition we loose all knowledge about the statistics
assert_eq!(Statistics::default(), physical_plan.statistics());
let stats = physical_plan.statistics();
assert!(!stats.is_exact);
assert_eq!(stats.num_rows, Some(1));
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice


Ok(())
}
Expand Down
8 changes: 4 additions & 4 deletions datafusion/physical-expr/src/expressions/binary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2970,7 +2970,7 @@ mod tests {
];

for ((operator, rhs), (exp_selectivity, _, _)) in cases {
let context = AnalysisContext::from_statistics(&schema, statistics.clone());
let context = AnalysisContext::from_statistics(&schema, &statistics);
let left = col("a", &schema).unwrap();
let right = ScalarValue::Int64(Some(rhs));
let boundaries =
Expand Down Expand Up @@ -3039,7 +3039,7 @@ mod tests {
];

for ((operator, rhs), (exp_selectivity, _, _)) in cases {
let context = AnalysisContext::from_statistics(&schema, statistics.clone());
let context = AnalysisContext::from_statistics(&schema, &statistics);
let left = col("a", &schema).unwrap();
let right = ScalarValue::from(rhs);
let boundaries =
Expand Down Expand Up @@ -3085,7 +3085,7 @@ mod tests {
&schema,
);

let context = AnalysisContext::from_statistics(&schema, statistics);
let context = AnalysisContext::from_statistics(&schema, &statistics);
let predicate_boundaries = gt
.boundaries(&context)
.expect("boundaries should not be None");
Expand Down Expand Up @@ -3113,7 +3113,7 @@ mod tests {
&schema,
);

let context = AnalysisContext::from_statistics(&schema, statistics);
let context = AnalysisContext::from_statistics(&schema, &statistics);
let predicate_boundaries = gt
.boundaries(&context)
.expect("boundaries should not be None");
Expand Down
2 changes: 1 addition & 1 deletion datafusion/physical-expr/src/expressions/column.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ mod test {
#[test]
fn stats_bounds_analysis() -> Result<()> {
let (schema, statistics) = get_test_table_stats();
let context = AnalysisContext::from_statistics(&schema, statistics);
let context = AnalysisContext::from_statistics(&schema, &statistics);

let cases = [
// (name, index, expected boundaries)
Expand Down
4 changes: 2 additions & 2 deletions datafusion/physical-expr/src/physical_expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,11 @@ impl AnalysisContext {
}

/// Create a new analysis context from column statistics.
pub fn from_statistics(input_schema: &Schema, statistics: Statistics) -> Self {
pub fn from_statistics(input_schema: &Schema, statistics: &Statistics) -> Self {
// Even if the underlying statistics object doesn't have any column level statistics,
// we can still create an analysis context with the same number of columns and see whether
// we can infer it during the way.
let column_boundaries = match statistics.column_statistics {
let column_boundaries = match &statistics.column_statistics {
Some(columns) => columns
.iter()
.map(ExprBoundaries::from_column)
Expand Down