You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@arrow.apache.org by "raulcd (via GitHub)" <gi...@apache.org> on 2024/11/22 10:10:04 UTC
[I] [C++][Parquet] Add `arrow::Result` version of `parquet::arrow::FileReader::Make()` [arrow]
raulcd opened a new issue, #44810:
URL: https://github.com/apache/arrow/issues/44810
### Describe the enhancement requested
### Describe the enhancement requested
The current version uses `arrow::Status` and `std::unique_ptr<FileReader>*`: https://github.com/apache/arrow/blob/ae497bf11a4078f48b02f53b8dc843e3c0579d76/cpp/src/parquet/arrow/reader.h#L119-L127
### Component(s)
C++, Parquet
--
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: issues-unsubscribe@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Add `arrow::Result` version of `parquet::arrow::FileReader::Make()` [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on issue #44810:
URL: https://github.com/apache/arrow/issues/44810#issuecomment-3640035719
Issue resolved by pull request 48285
https://github.com/apache/arrow/pull/48285
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Add `arrow::Result` version of `parquet::arrow::FileReader::Make()` [arrow]
Posted by "raulcd (via GitHub)" <gi...@apache.org>.
raulcd commented on issue #44810:
URL: https://github.com/apache/arrow/issues/44810#issuecomment-3586819986
@hiroyuki-sato you have examples of the old API being used on tests on this file, here an example:
https://github.com/apache/arrow/blob/fa41b2674bfab627ec1c69b4b4fb52d9caff8c15/cpp/src/parquet/arrow/arrow_reader_writer_test.cc#L4168-L4173
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Add `arrow::Result` version of `parquet::arrow::FileReader::Make()` [arrow]
Posted by "malinjawi (via GitHub)" <gi...@apache.org>.
malinjawi commented on issue #44810:
URL: https://github.com/apache/arrow/issues/44810#issuecomment-2493570880
Hey @raulcd @pitrou I would be happy to work on this issue. I just got my first PR merged and happy to continue making contributions to Arrow.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Add `arrow::Result` version of `parquet::arrow::FileReader::Make()` [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on issue #44810:
URL: https://github.com/apache/arrow/issues/44810#issuecomment-3570243791
This issue has been marked as stale because it has had no activity in the past 365 days. Please remove the stale label or comment below, or this issue will be closed in 14 days. If this improvement is still desired but has no current owner, please add the 'Status: needs champion' label.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Add `arrow::Result` version of `parquet::arrow::FileReader::Make()` [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou closed issue #44810: [C++][Parquet] Add `arrow::Result` version of `parquet::arrow::FileReader::Make()`
URL: https://github.com/apache/arrow/issues/44810
--
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: issues-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Add `arrow::Result` version of `parquet::arrow::FileReader::Make()` [arrow]
Posted by "hiroyuki-sato (via GitHub)" <gi...@apache.org>.
hiroyuki-sato commented on issue #44810:
URL: https://github.com/apache/arrow/issues/44810#issuecomment-3589022485
@raulcd Thank you for your advice.
I heard that the `Status` version of `FileReader::Make` would be deprecated once the `Result` version was created.
Therefore, I changed the body of the `Status` version to call the `Result` version, and the tests also use the `Result` version.
I'll change other `FileReader::Make ` parts on another PR in the `arrow_reader_writer_test.cc`.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Add `arrow::Result` version of `parquet::arrow::FileReader::Make()` [arrow]
Posted by "malinjawi (via GitHub)" <gi...@apache.org>.
malinjawi commented on issue #44810:
URL: https://github.com/apache/arrow/issues/44810#issuecomment-2495611900
Hey @raulcd @pitrou thanks for raising this issue. I have raised a PR and addressed other related changes found with the current versions of `arrow::Status` and `std::unique_ptr<FileReader>*`. Please let me know i there are anything else I can address to solve this issue.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Add `arrow::Result` version of `parquet::arrow::FileReader::Make()` [arrow]
Posted by "hiroyuki-sato (via GitHub)" <gi...@apache.org>.
hiroyuki-sato commented on issue #44810:
URL: https://github.com/apache/arrow/issues/44810#issuecomment-3585598638
I wrote the code with the help of @kou 's advice. Where should I write the tests?
```diff
diff --git a/cpp/src/parquet/arrow/reader.cc b/cpp/src/parquet/arrow/reader.cc
index c32e588688..b067e52b6f 100644
--- a/cpp/src/parquet/arrow/reader.cc
+++ b/cpp/src/parquet/arrow/reader.cc
@@ -1353,6 +1353,23 @@ Status FileReader::Make(::arrow::MemoryPool* pool,
return Make(pool, std::move(reader), default_arrow_reader_properties(), out);
}
+Result<std::unique_ptr<FileReader>> FileReader::Make(
+ ::arrow::MemoryPool* pool, std::unique_ptr<ParquetFileReader> parquet_reader,
+ const ArrowReaderProperties& properties) {
+ std::unique_ptr<FileReader> reader =
+ std::make_unique<FileReaderImpl>(pool, std::move(parquet_reader), properties);
+ RETURN_NOT_OK(static_cast<FileReaderImpl*>(reader.get())->Init());
+ return reader;
+}
+
+Result<std::unique_ptr<FileReader>> FileReader::Make(
+ ::arrow::MemoryPool* pool, std::unique_ptr<ParquetFileReader> parquet_reader) {
+ std::unique_ptr<FileReader> reader = std::make_unique<FileReaderImpl>(
+ pool, std::move(parquet_reader), default_arrow_reader_properties());
+ RETURN_NOT_OK(static_cast<FileReaderImpl*>(reader.get())->Init());
+ return reader;
+}
+
FileReaderBuilder::FileReaderBuilder()
: pool_(::arrow::default_memory_pool()),
properties_(default_arrow_reader_properties()) {}
diff --git a/cpp/src/parquet/arrow/reader.h b/cpp/src/parquet/arrow/reader.h
index 9753fe47ba..b4a13fd3b3 100644
--- a/cpp/src/parquet/arrow/reader.h
+++ b/cpp/src/parquet/arrow/reader.h
@@ -126,6 +126,14 @@ class PARQUET_EXPORT FileReader {
std::unique_ptr<ParquetFileReader> reader,
std::unique_ptr<FileReader>* out);
+ /// Factory function to create a FileReader from a ParquetFileReader and properties
+ static ::arrow::Result<std::unique_ptr<FileReader>> Make(
+ ::arrow::MemoryPool* pool, std::unique_ptr<ParquetFileReader> reader,
+ const ArrowReaderProperties& properties);
+
+ /// Factory function to create a FileReader from a ParquetFileReader
+ static ::arrow::Result<std::unique_ptr<FileReader>> Make(
+ ::arrow::MemoryPool* pool, std::unique_ptr<ParquetFileReader> reader);
// Since the distribution of columns amongst a Parquet file's row groups may
// be uneven (the number of values in each column chunk can be different), we
// provide a column-oriented read interface. The ColumnReader hides the
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Add `arrow::Result` version of `parquet::arrow::FileReader::Make()` [arrow]
Posted by "raulcd (via GitHub)" <gi...@apache.org>.
raulcd commented on issue #44810:
URL: https://github.com/apache/arrow/issues/44810#issuecomment-2493396069
@kou @pitrou from my understanding those are things that we want to keep doing and migrating to `::arrow::Result`, right? I could do some of those :)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Add `arrow::Result` version of `parquet::arrow::FileReader::Make()` [arrow]
Posted by "raulcd (via GitHub)" <gi...@apache.org>.
raulcd commented on issue #44810:
URL: https://github.com/apache/arrow/issues/44810#issuecomment-2493770532
Thanks @malinjawi for your contributions!
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++][Parquet] Add `arrow::Result` version of `parquet::arrow::FileReader::Make()` [arrow]
Posted by "raulcd (via GitHub)" <gi...@apache.org>.
raulcd commented on issue #44810:
URL: https://github.com/apache/arrow/issues/44810#issuecomment-3580695180
I am removing the assignee in case someone looking for a `good-first-issue` wants to tackle this one.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org