Support Page Skipping in Iceberg Parquet Reader

[aokolnychyi]

I think we should evaluate the feasibility of page skipping in Iceberg Parquet reader.

See here for more details.

[manishmalhotrawork]

@aokolnychyi this is interesting. is this still valid ?

[chenjunjiedada]

I 'm also interested in this. Currently, the page indexes feature has not released yet. See PARQUET-1434.

[chenjunjiedada]

@aokolnychyi @rdblue IIUC, we don't need to do extra work to support this as long as the predicate pushdown works well. The read property parquet.filter.columnindex.enabled defaults set to true and column indexes are built by default as well. Please correct me if I 'm wrong.

[rdblue]

We would need to implement something to take advantage of the column indexes in some engines. Any client using an Iceberg table uses its own readers for Parquet data. Spark readers are in the iceberg-spark module and use a different read path to construct records than the one built into Parquet, so we would need to add support for page indexes there.

In the long term, we want this optimization to be part of the vectorized reads because we think that reading first into Arrow (vectorized) and then converting to an in-memory row format is going to be faster than deserializing directly to the row format. That means we should invest in adding page skipping to the Parquet to Arrow conversion.

[chenjunjiedada]

The column index feature provides a help functionParquetFileReader.readNextFilteredRowGroup which should contain column index filtering logic. Does that mean we can just replace the readNextRowGroup In ParquetReader to utilize the column index filtering?

[rdblue]

@chenjunjiedada, probably not. That may read just the required pages, but Iceberg doesn't use Parquet filtering, so it wouldn't be correct without converting and pushing those filters. Record construction also needs to change to reconstruct rows correctly because the filtered pages start at different row offsets.

[shangxinli]

I don't see any progress on this feature for a year! Anybody is still actively working on it? If not, I am interested in working on it.

I look at the Icerberg code briefly. Yes, although the flag parquet.filter.columnindex.enabled is enabled by default, Iceberg is not using that reading path. It seems we can borrow something in org.apache.parquet.internal.filter2.columnindex.ColumnIndexFilter and implement the filtering in Vectorized Parquet reader.

[aokolnychyi]

I think this feature was blocked by the vectorization work. Since we merged it into master, I think we should be good to go.

[aokolnychyi]

I don't know anyone working on this.

[shangxinli]

Thanks Anton! I will have a look when I get time.

[rdblue]

@shangxinli, it would be great to have your help implementing this!

You're right that Iceberg doesn't use the read path from Parquet. Iceberg has its own set of readers to reconstruct Parquet rows, and it also has its own set of expressions that are used directly in row group and record filters.

We're here to help out with questions if you have them.

[shangxinli]

I have the code working and I can see many pages filtered out now. I am going to add unit tests and create PR. I have one question for you. Does it make sense to change idToAlias(...) to findField(...).name() in the line
https://github.com/apache/iceberg/blob/master/parquet/src/main/java/org/apache/iceberg/parquet/ParquetFilters.java#L120? Do you think it will work for nested fields? The reason for that is the variable 'aliasToId' in Schema class could be null.

[rdblue]

@shangxinli, idToAlias is correct. That accesses the full Parquet name for the field in the a schema converted from a Parquet file. There is no guarantee that Iceberg field names correspond to the names used by Parquet when the file was written, which is why we maintain the alias map when converting from Parquet.

[shangxinli]

@rdblue, idToAlias could be null. In that case, how do we find the field name which is required by the following Parquet API(FilterApi)?

BTW, how do you resolve the imports of 'relocated' in Intellij? I am able to Gradle build in the terminal but Intellij keeps complaining those relocated imports like below. This blocks me to run unit test in Intellij.

import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;

[rdblue]

idToAlias is not null when the schema was converted from a Parquet schema. So you just have to ensure that the schema you're using was converted from the schema of the file you're working with.

Try building the gradle bundle through IntelliJ's gradle plugin. That usually fixes it for me.