PARQUET-2252: Make some methods public to allow external projects to … #1038
Conversation
…implement page skipping.
|
@gszadovszky @shangxinli Do you have any concern? |
|
Since these are already used in iceberg I think it is better to have them public and maintain backward compatibility. |
|
|
||
| BlockMetaData block = blocks.get(blockIndex); | ||
| if (block.getRowCount() == 0L) { | ||
| throw new ParquetEmptyBlockException("Illegal row group of 0 rows"); |
There was a problem hiding this comment.
Now the reader simply skips empty row groups instead of throw. Could you change this to be consistent?
There was a problem hiding this comment.
I checked PARQUET-2291, seems we only skip empty row group when using reader as a iterator, right? We skip empty row group in readNextRowGroup() , but not when the user passes in a blockIndex, and this newly introduced method also requires the user pass in a blockIndex.
There was a problem hiding this comment.
I don't see why this would throw an exception. This method is intended to allow building an external iterator. I don't think anyone would ever want to fail if there were an empty row group, even if the reader thinks it shouldn't have been written. I think this should return null.
There was a problem hiding this comment.
Currently, readRowGroup(int blockIndex) and readFilteredRowGroup(int blockIndex) also throw an exception when handling empty row groups, should we make them also return null instead of throwing an exception to be consistent?
rdblue
left a comment
rdblue
left a comment
There was a problem hiding this comment.
Overall looks fine, but I agree that the row group count check should just return null instead of throwing an exception.
|
Thanks, @zhongyujiang! |
4 participants
…implement page skipping.
Issue: PARQUET-2252
This PR makes some methods public(methods in
RowRangesandIndexIteratorfor calculatingRowRanges, andgetColumnIndexStoreinParquetFileReaderfor getting column index store), Iceberg needs these to build its own column index filtering. Since Iceberg is going to calculateRowRangesitself, this also adds a public method inParquetFileReaderthat allows users to pass inRowRangesto read filtered row group. Use of these changes can refer to this PR, currently it uses reflection as a workaround.