-
Notifications
You must be signed in to change notification settings - Fork 4.2k
ARROW-11074: [Rust][DataFusion] Implement predicate push-down for parquet tables #9064
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.
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
Changes from 8 commits
a646088
0f28e8d
b00daec
bbaf809
5b2b6f2
a03ccf9
2e78552
5db2c6f
3f619d8
d1af10d
5d7dda7
61f5656
30d6d7d
750b8a2
0f4e18e
6e0f409
2de3d6c
bd22f64
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,8 @@ use crate::logical_plan::Expr; | |
| use crate::physical_plan::parquet::ParquetExec; | ||
| use crate::physical_plan::ExecutionPlan; | ||
|
|
||
| use super::datasource::TableProviderFilterPushDown; | ||
|
|
||
| /// Table-based representation of a `ParquetFile`. | ||
| pub struct ParquetTable { | ||
| path: String, | ||
|
|
@@ -41,7 +43,7 @@ pub struct ParquetTable { | |
| impl ParquetTable { | ||
| /// Attempt to initialize a new `ParquetTable` from a file path. | ||
| pub fn try_new(path: &str, max_concurrency: usize) -> Result<Self> { | ||
| let parquet_exec = ParquetExec::try_from_path(path, None, 0, 1)?; | ||
| let parquet_exec = ParquetExec::try_from_path(path, None, None, 0, 1)?; | ||
| let schema = parquet_exec.schema(); | ||
| Ok(Self { | ||
| path: path.to_string(), | ||
|
|
@@ -62,17 +64,37 @@ impl TableProvider for ParquetTable { | |
| self.schema.clone() | ||
| } | ||
|
|
||
| fn supports_filter_pushdown( | ||
| &self, | ||
| _filter: &Expr, | ||
| ) -> Result<TableProviderFilterPushDown> { | ||
| Ok(TableProviderFilterPushDown::Inexact) | ||
| } | ||
|
|
||
| /// Scan the file(s), using the provided projection, and return one BatchIterator per | ||
| /// partition. | ||
| fn scan( | ||
| &self, | ||
| projection: &Option<Vec<usize>>, | ||
| batch_size: usize, | ||
| _filters: &[Expr], | ||
| filters: &[Expr], | ||
| ) -> Result<Arc<dyn ExecutionPlan>> { | ||
| let predicate = if filters.is_empty() { | ||
| None | ||
| } else { | ||
| Some( | ||
| filters | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add a comment explaining the logic here? It isn't immediately obvious to me.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Immediately after posting that comment I see how it works now, but I think a comment would still be helpful |
||
| .iter() | ||
| .skip(1) | ||
| .fold(filters[0].clone(), |acc, filter| { | ||
| crate::logical_plan::and(acc, (*filter).to_owned()) | ||
| }), | ||
| ) | ||
| }; | ||
| Ok(Arc::new(ParquetExec::try_from_path( | ||
| &self.path, | ||
| projection.clone(), | ||
| predicate, | ||
| batch_size, | ||
| self.max_concurrency, | ||
| )?)) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, I feel we should have a proper filter API defined in data fusion which can be shared among various data sources. On the other hand, the actual filtering logic should be implemented by different data sources / formats, probably via converting the data fusion's filter API to the corresponding ones from the latter.
But this is a very good start and we can probably do them as follow ups (if we don't care much for API changes).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that starting with this PR and then extending the approach to be something more generic is a good approach.