EnricoMi commented on code in PR #51362:
URL: https://github.com/apache/arrow/pull/51362#discussion_r4034548054
##########
cpp/src/parquet/arrow/reader.cc:
##########
@@ -271,13 +271,13 @@ class FileReaderImpl : public FileReader {
Status ReadColumn(int i, const std::vector<int>& row_groups, ColumnReader*
reader,
std::shared_ptr<ChunkedArray>* out) {
BEGIN_PARQUET_CATCH_EXCEPTIONS
- // TODO(wesm): This calculation doesn't make much sense when we have
repeated
- // schema nodes
+ // NextBatch()'s size is a number of records (rows), not leaf values, so
use the
+ // row group's own row count directly rather than some column's
num_values() (which
+ // (a) requires picking a column, a prior source of index-confusion bugs,
and
+ // (b) over-counts for repeated schema nodes, counting elements rather
than rows).
int64_t records_to_read = 0;
for (auto row_group : row_groups) {
- // Can throw exception
- records_to_read +=
-
reader_->metadata()->RowGroup(row_group)->ColumnChunk(i)->num_values();
+ records_to_read += reader_->metadata()->RowGroup(row_group)->num_rows();
Review Comment:
With "this" you mean the new code or the old code?
--
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]
