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 2021/05/07 17:34:15 UTC

[GitHub] [arrow] lidavidm opened a new pull request #10268: ARROW-12687: [C++][Dataset] Convert Scanner into a RecordBatchReader

lidavidm opened a new pull request #10268:
URL: https://github.com/apache/arrow/pull/10268


   This provides compatibility with APIs that expect regular RecordBatchReaders, e.g. exporting via the C Data Interface.


-- 
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] lidavidm closed pull request #10268: ARROW-12687: [C++][Python][Dataset] Convert Scanner into a RecordBatchReader

Posted by GitBox <gi...@apache.org>.
lidavidm closed pull request #10268:
URL: https://github.com/apache/arrow/pull/10268


   


-- 
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] westonpace commented on a change in pull request #10268: ARROW-12687: [C++][Python][Dataset] Convert Scanner into a RecordBatchReader

Posted by GitBox <gi...@apache.org>.
westonpace commented on a change in pull request #10268:
URL: https://github.com/apache/arrow/pull/10268#discussion_r628458974



##########
File path: cpp/src/arrow/dataset/scanner.cc
##########
@@ -164,6 +164,40 @@ Result<int64_t> Scanner::CountRows() {
   return count;
 }
 
+namespace {
+class ScannerRecordBatchReader : public RecordBatchReader {
+ public:
+  explicit ScannerRecordBatchReader(std::shared_ptr<Schema> schema,
+                                    std::shared_ptr<Scanner> scanner,
+                                    TaggedRecordBatchIterator delegate)
+      : schema_(std::move(schema)),
+        scanner_(std::move(scanner)),
+        delegate_(std::move(delegate)) {}
+
+  std::shared_ptr<Schema> schema() const override { return schema_; }
+  Status ReadNext(std::shared_ptr<RecordBatch>* batch) override {
+    ARROW_ASSIGN_OR_RAISE(auto next, delegate_.Next());
+    if (IsIterationEnd(next)) {
+      *batch = nullptr;
+    } else {
+      *batch = std::move(next.record_batch);
+    }
+    return Status::OK();
+  }
+
+ private:
+  std::shared_ptr<Schema> schema_;
+  std::shared_ptr<Scanner> scanner_;

Review comment:
       Hmm...I'd hope the `TaggedRecordBatchIterator` keeps the `Scanner` alive.  Is that not the case?

##########
File path: python/pyarrow/_dataset.pyx
##########
@@ -2928,6 +2928,13 @@ cdef class Scanner(_Weakrefable):
             result = self.scanner.CountRows()
         return GetResultValue(result)
 
+    def to_reader(self):
+        """Consume this scanner as a RecordBatchReader."""

Review comment:
       What is this adding to the python API?  Are there places that accept a `RecordBatchReader`?




-- 
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] lidavidm commented on a change in pull request #10268: ARROW-12687: [C++][Python][Dataset] Convert Scanner into a RecordBatchReader

Posted by GitBox <gi...@apache.org>.
lidavidm commented on a change in pull request #10268:
URL: https://github.com/apache/arrow/pull/10268#discussion_r628462966



##########
File path: cpp/src/arrow/dataset/scanner.cc
##########
@@ -164,6 +164,40 @@ Result<int64_t> Scanner::CountRows() {
   return count;
 }
 
+namespace {
+class ScannerRecordBatchReader : public RecordBatchReader {
+ public:
+  explicit ScannerRecordBatchReader(std::shared_ptr<Schema> schema,
+                                    std::shared_ptr<Scanner> scanner,
+                                    TaggedRecordBatchIterator delegate)
+      : schema_(std::move(schema)),
+        scanner_(std::move(scanner)),
+        delegate_(std::move(delegate)) {}
+
+  std::shared_ptr<Schema> schema() const override { return schema_; }
+  Status ReadNext(std::shared_ptr<RecordBatch>* batch) override {
+    ARROW_ASSIGN_OR_RAISE(auto next, delegate_.Next());
+    if (IsIterationEnd(next)) {
+      *batch = nullptr;
+    } else {
+      *batch = std::move(next.record_batch);
+    }
+    return Status::OK();
+  }
+
+ private:
+  std::shared_ptr<Schema> schema_;
+  std::shared_ptr<Scanner> scanner_;

Review comment:
       I added this on reflex; I've removed it.

##########
File path: python/pyarrow/_dataset.pyx
##########
@@ -2928,6 +2928,13 @@ cdef class Scanner(_Weakrefable):
             result = self.scanner.CountRows()
         return GetResultValue(result)
 
+    def to_reader(self):
+        """Consume this scanner as a RecordBatchReader."""

Review comment:
       In this case, to call RecordBatchReader._export_to_c.




-- 
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 #10268: ARROW-12687: [C++][Dataset] Convert Scanner into a RecordBatchReader

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10268:
URL: https://github.com/apache/arrow/pull/10268#issuecomment-834643542


   https://issues.apache.org/jira/browse/ARROW-12687


-- 
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