Dandandan commented on a change in pull request #380: URL: https://github.com/apache/arrow-datafusion/pull/380#discussion_r638136080
########## File path: datafusion/src/physical_optimizer/pruning.rs ########## @@ -28,50 +28,75 @@ //! https://github.com/apache/arrow-datafusion/issues/363 it will //! be genericized. -use std::{collections::HashSet, sync::Arc}; +use std::{collections::HashSet, convert::TryInto, sync::Arc}; use arrow::{ - array::{ - make_array, new_null_array, ArrayData, ArrayRef, BooleanArray, - BooleanBufferBuilder, - }, - buffer::MutableBuffer, - datatypes::{DataType, Field, Schema}, + array::{ArrayRef, BooleanArray}, + datatypes::{Field, Schema, SchemaRef}, record_batch::RecordBatch, }; -use parquet::file::{ - metadata::RowGroupMetaData, statistics::Statistics as ParquetStatistics, -}; - use crate::{ error::{DataFusionError, Result}, execution::context::ExecutionContextState, logical_plan::{Expr, Operator}, optimizer::utils, physical_plan::{planner::DefaultPhysicalPlanner, ColumnarValue, PhysicalExpr}, + scalar::ScalarValue, }; +/// Interface to pass statistics information to [`PruningPredicates`] +pub trait PruningStatistics { + /// return the minimum value for the named column, if known + fn min_value(&self, column: &str) -> Option<ScalarValue>; Review comment: One concern I have (in general with current state of DataFusion) is that we use `ScalarValue` a lot in code which can detrimental to performance in some cases (compared to a typed array). Also in this case, if we would have a large dataset with 1000s of statistics values, calculating statistics might be slower than it could be when it is stored in contiguous memory. Just a thought - not something we should block this PR for. -- 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. For queries about this service, please contact Infrastructure at: [email protected]
