rdblue commented on a change in pull request #1474:
URL: https://github.com/apache/iceberg/pull/1474#discussion_r490617559
##########
File path: parquet/src/main/java/org/apache/iceberg/parquet/ReadConf.java
##########
@@ -165,12 +165,12 @@ ParquetFileReader reader() {
return shouldSkip;
}
- private Map<Long, Long> generateOffsetToStartPos() {
- ParquetFileReader fileReader = newReader(this.file,
ParquetReadOptions.builder().build());
Review comment:
The reason for a new reader is to create one that doesn't have a filter
pushed down through `ParquetReadOptions`.
When building a reader, a [file range can be added to
`ParquetReadOptions`](https://github.com/apache/iceberg/blob/master/parquet/src/main/java/org/apache/iceberg/parquet/Parquet.java#L590),
which will filter Parquet row groups as they are read. Iceberg uses this to
handle file split ranges, and Parquet avoids reading the row group metadata in
case there are a lot of row groups in the file.
There are a couple options to fix this. We could stop pushing the file range
to Parquet so that it reads all the row groups and use your implementation
here. That would also require applying the range in `ReadConf` and setting
`shouldSkip` appropriately.
A second option is to properly close the file that is opened here, like this:
```java
private Map<Long, Long> generateOffsetToStartPos() {
try (ParquetFileReader fileReader = newReader(file,
ParquetReadOptions.builder().build())) {
Map<Long, Long> offsetToStartPos = Maps.newHashMap();
long curRowCount = 0;
for (int i = 0; i < fileReader.getRowGroups().size(); i += 1) {
BlockMetaData meta = fileReader.getRowGroups().get(i);
offsetToStartPos.put(meta.getStartingPos(), curRowCount);
curRowCount += meta.getRowCount();
}
return offsetToStartPos;
} catch (IOException e) {
throw new UncheckedIOException("Failed to close reader for file: " +
file, e);
}
}
```
I think it's a good idea to go with the first option, but we might want to
fix this with the second option in the mean time.
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
