Copilot commented on code in PR #51362: URL: https://github.com/apache/arrow/pull/51362#discussion_r4030138811
########## 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 field indices (see `schema.h:139-145`), not Parquet leaf indices. In the nested regression case it returns `2` for `c`, while `ReadColumn` uses `metadata()->schema()->Column(i)` for its OpenTelemetry attributes and the leaf is `3`, so the span reports `b.y` instead of `c`. Also, `ReadColumn` no longer indexes `ColumnChunk(i)` after the earlier change, so the rationale in this comment is stale. Please make the trace use a consistent top-level/leaf mapping (or remove the leaf-index claim). -- 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]
