[GitHub] [arrow] pitrou commented on a change in pull request #10509: ARROW-12996: Add bytes_read() to StreamingReader

[GitBox <gi...@apache.org>]

pitrou commented on a change in pull request #10509:
URL: https://github.com/apache/arrow/pull/10509#discussion_r651769022



##########
File path: cpp/src/arrow/csv/reader.h
##########
@@ -73,6 +73,9 @@ class ARROW_EXPORT StreamingReader : public RecordBatchReader {
 
   virtual Future<std::shared_ptr<RecordBatch>> ReadNextAsync() = 0;
 
+  /// \brief Returns the number of bytes which have been read by this reader
+  virtual int64_t bytes_read() const = 0;

Review comment:
       The docstring may be a bit imprecise here. If there is some readahead going on, is it included in the result? Or is it the number of bytes corresponding to the batches already consumed by the caller?

##########
File path: cpp/src/arrow/csv/reader_test.cc
##########
@@ -216,6 +216,27 @@ TEST(StreamingReaderTests, NestedParallelism) {
   TestNestedParallelism(thread_pool, table_factory);
 }
 
+TEST(StreamingReaderTest, BytesRead) {
+  ASSERT_OK_AND_ASSIGN(auto thread_pool, internal::ThreadPool::Make(1));
+  auto table_buffer =
+      std::make_shared<Buffer>("a,b,c\n123,456,789\n101,112,131\n415,161,718\n");
+  auto input = std::make_shared<io::BufferReader>(table_buffer);
+  auto read_options = ReadOptions::Defaults();
+  read_options.block_size = 20;
+  ASSERT_OK_AND_ASSIGN(
+      auto streaming_reader,
+      StreamingReader::Make(io::default_io_context(), input, read_options,
+                            ParseOptions::Defaults(), ConvertOptions::Defaults()));
+  std::shared_ptr<RecordBatch> batch;
+  int64_t bytes = 6;  // Size of header
+  do {
+    ASSERT_EQ(bytes, streaming_reader->bytes_read());
+    ASSERT_OK(streaming_reader->ReadNext(&batch));
+    bytes += 12;  // Add size of each row
+  } while (batch);
+  ASSERT_EQ(42, streaming_reader->bytes_read());
+}

Review comment:
       Can you also add a test where the `skip_rows` and/or `skip_rows_after_names` options are set? What should be the semantics there?




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