emkornfield commented on a change in pull request #7704:
URL: https://github.com/apache/arrow/pull/7704#discussion_r453149789



##########
File path: cpp/src/parquet/arrow/reader.cc
##########
@@ -301,62 +302,50 @@ class FileReaderImpl : public FileReader {
 
 class RowGroupRecordBatchReader : public ::arrow::RecordBatchReader {
  public:
-  RowGroupRecordBatchReader(std::vector<std::unique_ptr<ColumnReaderImpl>> 
field_readers,
-                            std::shared_ptr<::arrow::Schema> schema, int64_t 
batch_size)
-      : field_readers_(std::move(field_readers)),
-        schema_(std::move(schema)),
-        batch_size_(batch_size) {}
-
   ~RowGroupRecordBatchReader() override {}
 
   std::shared_ptr<::arrow::Schema> schema() const override { return schema_; }
 
-  static Status Make(const std::vector<int>& row_groups,
-                     const std::vector<int>& column_indices, FileReaderImpl* 
reader,
-                     int64_t batch_size,
-                     std::unique_ptr<::arrow::RecordBatchReader>* out) {
-    std::vector<int> field_indices;
-    if (!reader->manifest_.GetFieldIndices(column_indices, &field_indices)) {
-      return Status::Invalid("Invalid column index");
-    }
+  static ::arrow::Result<std::unique_ptr<RowGroupRecordBatchReader>> Make(
+      const std::vector<int>& row_group_indices, const std::vector<int>& 
column_indices,
+      int64_t batch_size, FileReaderImpl* reader) {
+    std::unique_ptr<RowGroupRecordBatchReader> out(new 
RowGroupRecordBatchReader);
 
-    std::vector<std::unique_ptr<ColumnReaderImpl>> 
field_readers(field_indices.size());
-    std::vector<std::shared_ptr<Field>> fields;
+    RETURN_NOT_OK(FromParquetSchema(
+        reader->parquet_reader()->metadata()->schema(), 
default_arrow_reader_properties(),
+        /*key_value_metadata=*/nullptr, column_indices, &out->schema_));
 
-    auto included_leaves = VectorToSharedSet(column_indices);
-    for (size_t i = 0; i < field_indices.size(); ++i) {
-      RETURN_NOT_OK(reader->GetFieldReader(field_indices[i], included_leaves, 
row_groups,
-                                           &field_readers[i]));
-      fields.push_back(field_readers[i]->field());
-    }
-    out->reset(new RowGroupRecordBatchReader(std::move(field_readers),
-                                             ::arrow::schema(fields), 
batch_size));
-    return Status::OK();
+    using ::arrow::RecordBatchIterator;
+
+    auto row_group_index_to_batch_iterator =
+        [=](const int* i) -> ::arrow::Result<RecordBatchIterator> {

Review comment:
       this binding everything by value here intentional?  If so could you add 
a comment why?




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


Reply via email to