alamb commented on code in PR #3454:
URL: https://github.com/apache/arrow-datafusion/pull/3454#discussion_r971176547
##########
datafusion/core/src/physical_optimizer/pruning.rs:
##########
@@ -168,38 +170,51 @@ impl PruningPredicate {
/// simplified version `b`. The predicates are simplified via the
/// ConstantFolding optimizer pass
pub fn prune<S: PruningStatistics>(&self, statistics: &S) ->
Result<Vec<bool>> {
- // build statistics record batch
- let predicate_array =
- build_statistics_record_batch(statistics, &self.required_columns)
- .and_then(|statistics_batch| {
- // execute predicate expression
- self.predicate_expr.evaluate(&statistics_batch)
- })
- .and_then(|v| match v {
- ColumnarValue::Array(array) => Ok(array),
- ColumnarValue::Scalar(_) => Err(DataFusionError::Internal(
- "predicate expression didn't return an
array".to_string(),
- )),
- })?;
-
- let predicate_array = predicate_array
- .as_any()
- .downcast_ref::<BooleanArray>()
- .ok_or_else(|| {
- DataFusionError::Internal(format!(
- "Expected pruning predicate evaluation to be BooleanArray,
\
- but was {:?}",
- predicate_array
- ))
- })?;
-
- // when the result of the predicate expression for a row group is null
/ undefined,
- // e.g. due to missing statistics, this row group can't be filtered
out,
- // so replace with true
- Ok(predicate_array
- .into_iter()
- .map(|x| x.unwrap_or(true))
- .collect::<Vec<_>>())
+ // build a RecordBatch that contains the min/max values in the
+ // appropriate statistics columns
+ let statistics_batch =
+ build_statistics_record_batch(statistics, &self.required_columns)?;
+
+ // Evaluate the pruning predicate on that record batch.
+ //
+ // Use true when the result of evaluating a predicate
+ // expression on a row group is null (aka `None`). Null can
+ // arise when the statistics are unknown or some calculation
+ // in the predicate means we don't know for sure if the row
+ // group can be filtered out or not. To maintain correctness
+ // the row group must be kept and thus `true` is returned.
+ match self.predicate_expr.evaluate(&statistics_batch)? {
+ ColumnarValue::Array(array) => {
+ let predicate_array = array
+ .as_any()
+ .downcast_ref::<BooleanArray>()
+ .ok_or_else(|| {
+ DataFusionError::Internal(format!(
+ "Expected pruning predicate evaluation to be
BooleanArray, \
+ but was {:?}",
+ array
+ ))
+ })?;
Review Comment:
Great idea -- done in 5ad465cec
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
