Copilot commented on code in PR #51362:
URL: https://github.com/apache/arrow/pull/51362#discussion_r4029794348


##########
cpp/src/parquet/arrow/reader.cc:
##########
@@ -1379,11 +1379,18 @@ Future<std::shared_ptr<Table>> 
FileReaderImpl::DecodeRowGroups(
   // OptionalParallelForAsync requires an executor
   if (!cpu_executor) cpu_executor = ::arrow::internal::GetCpuThreadPool();
 
-  auto read_column = [row_groups, self, this](size_t i,
-                                              
std::shared_ptr<ColumnReaderImpl> reader)
+  // `readers` only holds the requested columns, so its index `i` is a 
position within
+  // the selection, not the column's actual index in the row group. Map back 
to the
+  // real column index (as GetFieldReaders does internally) before calling 
ReadColumn,
+  // which indexes RowGroupMetaData::ColumnChunk() by the latter.
+  ARROW_ASSIGN_OR_RAISE(std::vector<int> field_indices,
+                        manifest_.GetFieldIndices(column_indices));

Review Comment:
   `GetFieldIndices` returns top-level/root field positions, not Parquet 
leaf-column indices (see `schema.h:139-144`), while `ReadColumn` uses its 
argument in `RowGroup::ColumnChunk(i)` and the public API defines requested 
indices as leaves (`reader.h:202-218`). For a schema such as `struct a {x, y}, 
c`, selecting `c` yields `column_indices=[2]` and `field_indices=[1]`, so this 
still reads `ColumnChunk(1)` (`a.y`), potentially decrypting the wrong column 
or using its value count. Build one selected leaf index per root reader and add 
a nested-schema regression instead of passing `field_indices` here.



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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to