westonpace commented on a change in pull request #9644:
URL: https://github.com/apache/arrow/pull/9644#discussion_r598045677
##########
File path: cpp/src/arrow/record_batch.h
##########
@@ -207,6 +208,14 @@ class ARROW_EXPORT RecordBatchReader {
/// \return Status
virtual Status ReadNext(std::shared_ptr<RecordBatch>* batch) = 0;
+ // Fallback to sync implementation until all other readers are
converted(ARROW-11770)
+ // and then this could become pure virtual with ReadNext falling back to
async impl.
+ virtual Future<std::shared_ptr<RecordBatch>> ReadNextAsync() {
+ std::shared_ptr<RecordBatch> batch;
+ ARROW_RETURN_NOT_OK(ReadNext(&batch));
+ return
Future<std::shared_ptr<RecordBatch>>::MakeFinished(std::move(batch));
+ }
+
Review comment:
It was already different. The streaming CSV reader has a `ReadBatch`
method and it was used to create an iterator of batches. I think keeping the
generator logic out of the readers would be ideal.
##########
File path: cpp/src/arrow/record_batch.h
##########
@@ -207,6 +208,14 @@ class ARROW_EXPORT RecordBatchReader {
/// \return Status
virtual Status ReadNext(std::shared_ptr<RecordBatch>* batch) = 0;
+ // Fallback to sync implementation until all other readers are
converted(ARROW-11770)
+ // and then this could become pure virtual with ReadNext falling back to
async impl.
+ virtual Future<std::shared_ptr<RecordBatch>> ReadNextAsync() {
+ std::shared_ptr<RecordBatch> batch;
+ ARROW_RETURN_NOT_OK(ReadNext(&batch));
+ return
Future<std::shared_ptr<RecordBatch>>::MakeFinished(std::move(batch));
+ }
+
Review comment:
It was already different. The streaming CSV reader has a "read batch"
method and it was used to create an iterator of batches. I think keeping the
generator logic out of the readers would be ideal.
--
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:
us...@infra.apache.org
