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.

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

ARROW-11435: [Datafusion] allow creating ParquetPartition from external crate, make combine_filters public #9369

Closed
wants to merge 1 commit into from

Conversation

houqp
Copy link
Member

@houqp houqp commented Jan 30, 2021

Without this, it's not possible to create TableProvider in external crate that tagets parquet format because ParquetExec::new takes ParquetPartition as argument.

@github-actions
Copy link

@codecov-io
Copy link

Codecov Report

Merging #9369 (96485ec) into master (f05b49b) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9369      +/-   ##
==========================================
- Coverage   81.94%   81.94%   -0.01%     
==========================================
  Files         231      231              
  Lines       53374    53375       +1     
==========================================
- Hits        43739    43738       -1     
- Misses       9635     9637       +2     
Impacted Files Coverage Δ
rust/datafusion/src/physical_plan/parquet.rs 88.10% <0.00%> (-0.14%) ⬇️
rust/parquet/src/encodings/encoding.rs 94.86% <0.00%> (-0.20%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f05b49b...c46ff83. Read the comment docs.

…al crate

also move combine_filters from physical_plan/parquet.rs into
logical_plan/expr.rs.
Copy link
Member

@andygrove andygrove left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @houqp

@alamb alamb changed the title ARROW-11435: [Datafusion] allow creating ParquetPartition from external crate ARROW-11435: [Datafusion] allow creating ParquetPartition from external crate, make combine_filters public Jan 31, 2021
@houqp
Copy link
Member Author

houqp commented Feb 2, 2021

@andygrove @alamb gentle ping to help with the merge if everything looks good on your end ;P

@alamb
Copy link
Contributor

alamb commented Feb 2, 2021

Looks good to me -- I'll merge it in!

@alamb alamb closed this in 05b3656 Feb 2, 2021
@houqp houqp deleted the qp_parquetpartition branch February 2, 2021 19:48
nevi-me pushed a commit to nevi-me/arrow that referenced this pull request Feb 13, 2021
…al crate, make combine_filters public

Without this, it's not possible to create TableProvider in external crate that tagets parquet format because `ParquetExec::new` takes `ParquetPartition` as argument.

Closes apache#9369 from houqp/qp_parquetpartition

Authored-by: Qingping Hou <qph@scribd.com>
Signed-off-by: Andrew Lamb <andrew@nerdnetworks.org>
GeorgeAp pushed a commit to sirensolutions/arrow that referenced this pull request Jun 7, 2021
…al crate, make combine_filters public

Without this, it's not possible to create TableProvider in external crate that tagets parquet format because `ParquetExec::new` takes `ParquetPartition` as argument.

Closes apache#9369 from houqp/qp_parquetpartition

Authored-by: Qingping Hou <qph@scribd.com>
Signed-off-by: Andrew Lamb <andrew@nerdnetworks.org>
michalursa pushed a commit to michalursa/arrow that referenced this pull request Jun 13, 2021
…al crate, make combine_filters public

Without this, it's not possible to create TableProvider in external crate that tagets parquet format because `ParquetExec::new` takes `ParquetPartition` as argument.

Closes apache#9369 from houqp/qp_parquetpartition

Authored-by: Qingping Hou <qph@scribd.com>
Signed-off-by: Andrew Lamb <andrew@nerdnetworks.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants