[GitHub] [arrow] westonpace commented on a change in pull request #9947: ARROW-12288: [C++] Create Scanner interface

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

westonpace commented on a change in pull request #9947:
URL: https://github.com/apache/arrow/pull/9947#discussion_r610798990



##########
File path: cpp/src/arrow/dataset/scanner.h
##########
@@ -138,47 +179,147 @@ ARROW_DS_EXPORT Result<ScanTaskIterator> ScanTaskIteratorFromRecordBatch(
     std::vector<std::shared_ptr<RecordBatch>> batches,
     std::shared_ptr<ScanOptions> options);
 
-/// \brief Scanner is a materialized scan operation with context and options
-/// bound. A scanner is the class that glues ScanTask, Fragment,
-/// and Dataset. In python pseudo code, it performs the following:
+template <typename T>
+struct Enumerated {
+  T value;
+  int index;
+  bool last;
+};
+
+/// \brief Combines a record batch with the fragment that the record batch originated
+/// from
+///
+/// Knowing the source fragment can be useful for debugging & understanding loaded data
+struct TaggedRecordBatch {
+  std::shared_ptr<RecordBatch> record_batch;
+  std::shared_ptr<Fragment> fragment;
+};

Review comment:
       Yes, that's why I ended up using visit in `FileSystemDataset::Write`.  However, the `Iterator` is exposed externally so it might be nice to add support for it.  Unfortunately, neither comparing record batches or comparing fragments is simple (consider in memory fragments) or cheap.
   
   I could use reference equality again (I rely on fragments having stable references in `Scanner::AddPositioningToInOrderScan`) and that should work but I don't see it used much elsewhere so I doubt myself.
   
   What do you think?




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