You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/09/29 15:53:22 UTC
[GitHub] [arrow] jorisvandenbossche opened a new pull request #8301: ARROW-10100: [C++][Python][Dataset] Add ParquetFileFragment::Subset method
jorisvandenbossche opened a new pull request #8301:
URL: https://github.com/apache/arrow/pull/8301
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a change in pull request #8301: ARROW-10100: [C++][Python][Dataset] Add ParquetFileFragment::Subset method
Posted by GitBox <gi...@apache.org>.
bkietz commented on a change in pull request #8301:
URL: https://github.com/apache/arrow/pull/8301#discussion_r502461050
##########
File path: cpp/src/arrow/dataset/file_parquet.cc
##########
@@ -631,6 +631,39 @@ Result<FragmentVector> ParquetFileFragment::SplitByRowGroup(
return fragments;
}
+Result<std::shared_ptr<Fragment>> ParquetFileFragment::Subset(
+ const std::shared_ptr<Expression>& predicate) {
+ RETURN_NOT_OK(EnsureCompleteMetadata());
+ ARROW_ASSIGN_OR_RAISE(auto row_groups, FilterRowGroups(*predicate));
+
+ ARROW_ASSIGN_OR_RAISE(auto new_fragment,
+ parquet_format_.MakeFragment(source_, partition_expression(),
+ row_groups, physical_schema_));
+ return new_fragment;
Review comment:
```suggestion
return checked_pointer_cast<ParquetFileFragment>(std::move(new_fragment));
```
##########
File path: cpp/src/arrow/dataset/file_parquet.h
##########
@@ -232,6 +232,10 @@ class ARROW_DS_EXPORT ParquetFileFragment : public FileFragment {
/// \brief Ensure attached statistics are complete and the physical schema is cached.
Status EnsureCompleteMetadata(parquet::arrow::FileReader* reader = NULLPTR);
+ /// \brief Return a filtered subset of the ParquetFileFragment.
+ Result<std::shared_ptr<Fragment>> Subset(const std::shared_ptr<Expression>& predicate);
+ Result<std::shared_ptr<Fragment>> Subset(const std::vector<int> row_group_ids);
Review comment:
```suggestion
Result<std::shared_ptr<ParquetFileFragment>> Subset(const std::shared_ptr<Expression>& predicate);
Result<std::shared_ptr<ParquetFileFragment>> Subset(const std::vector<int> row_group_ids);
```
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] jorisvandenbossche commented on pull request #8301: ARROW-10100: [C++][Python][Dataset] Add ParquetFileFragment::Subset method
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on pull request #8301:
URL: https://github.com/apache/arrow/pull/8301#issuecomment-706508669
@bkietz Thanks for the update! I further expanded the tests somewhat for the empty case.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] jorisvandenbossche commented on pull request #8301: ARROW-10100: [C++][Python][Dataset] Add ParquetFileFragment::Subset method
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on pull request #8301:
URL: https://github.com/apache/arrow/pull/8301#issuecomment-706084228
@bkietz so I was now thinking it is actually fine to return a fragment with 0 row groups (it can then be the responsibility of the user to check for this, if they want to filter out the empty fragments out of their list of fragments). But, the problem is that an empty row group vector currently already means "all row groups" ...
Would it be fine to change empty vector to nullptr to indicate "all row groups" ?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] kszucs closed pull request #8301: ARROW-10100: [C++][Python][Dataset] Add ParquetFileFragment::Subset method
Posted by GitBox <gi...@apache.org>.
kszucs closed pull request #8301:
URL: https://github.com/apache/arrow/pull/8301
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] jorisvandenbossche commented on a change in pull request #8301: ARROW-10100: [C++][Python][Dataset] Add ParquetFileFragment::Subset method
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on a change in pull request #8301:
URL: https://github.com/apache/arrow/pull/8301#discussion_r496842872
##########
File path: cpp/src/arrow/dataset/file_parquet.cc
##########
@@ -575,6 +575,39 @@ Result<FragmentVector> ParquetFileFragment::SplitByRowGroup(
return fragments;
}
+Result<std::shared_ptr<Fragment>> ParquetFileFragment::Subset(
+ const std::shared_ptr<Expression>& predicate) {
+ RETURN_NOT_OK(EnsureCompleteMetadata());
+ ARROW_ASSIGN_OR_RAISE(auto row_groups, FilterRowGroups(*predicate));
+
+ ARROW_ASSIGN_OR_RAISE(auto new_fragment,
+ parquet_format_.MakeFragment(source_, partition_expression(),
+ row_groups, physical_schema_));
+ return new_fragment;
+}
+
+Result<std::shared_ptr<Fragment>> ParquetFileFragment::Subset(
+ const std::vector<int> row_group_ids) {
+ std::vector<RowGroupInfo> row_groups;
+
Review comment:
Here I would still need to check that all row_group_ids exist in the fragment
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] jorisvandenbossche commented on pull request #8301: ARROW-10100: [C++][Python][Dataset] Add ParquetFileFragment::Subset method
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on pull request #8301:
URL: https://github.com/apache/arrow/pull/8301#issuecomment-700800046
@bkietz this is what I had in mind for https://issues.apache.org/jira/browse/ARROW-10100. Thoughts?
(only POC, if we add it, need to add proper tests etc)
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] jorisvandenbossche commented on pull request #8301: ARROW-10100: [C++][Python][Dataset] Add ParquetFileFragment::Subset method
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on pull request #8301:
URL: https://github.com/apache/arrow/pull/8301#issuecomment-706084228
@bkietz so I was now thinking it is actually fine to return a fragment with 0 row groups (it can then be the responsibility of the user to check for this, if they want to filter out the empty fragments out of their list of fragments). But, the problem is that an empty row group vector currently already means "all row groups" ...
Would it be fine to change empty vector to nullptr to indicate "all row groups" ?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #8301: ARROW-10100: [C++][Python][Dataset] Add ParquetFileFragment::Subset method
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #8301:
URL: https://github.com/apache/arrow/pull/8301#issuecomment-700808359
https://issues.apache.org/jira/browse/ARROW-10100
----------------------------------------------------------------
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:
users@infra.apache.org