Parquet dictionary filter

[danielcweeks]

This isn't complete, but I wanted to get it out there for initial review.

I borrowed some from #270, which we can better define using this as a use case.

[danielcweeks]

I don't really love this signature due to exposing the data stream as a parameter. We could reduce the entire interface to a single FilterContext, which we could then use to differentiate between the hadoop implementation and the core parquet packages. However, the DictionaryFilter does rely on seek, which is specific to the HDFS api.

[rdblue]

I think a cleaner way to do this is to pass in a dictionary page reader that returns an uncompressed dictionary page when passed a ColumnDescriptor. The DictionaryPageReader implementation would be RowGroupDictionaryPageReader and could be constructed by the ParquetFileReader. That way, the file reader can keep its place and optionally provide dictionaries for its current row group.

Getting access to an already-open ParquetFileReader would mean pushing the row group filtering into the InternalParquetRecordReader class, which I think makes sense. The ParquetRecordReader class could do Split-based row group filtering and the internal version could do data-based row group filtering. I can prototype what I'm talking about if this makes no sense.

[danielcweeks]

I think I understand what you're saying here. The only important thing to maintain is that we don't want to prematurely load data that we're not going to process. The most recent version of the dictionary read JIRA has something similar to what you're describing, but it requires that the row group be loaded in order to read the dictionary. This isn't effective because we want to read the dictionary prior to loading the data.

[rdblue]

@danielcweeks, I'm doing some 1.9.0 planning. Is this something you guys would like to get in for that release? If so, would you mark the JIRA a blocker for PARQUET-392?

[danielcweeks]

Just added it.

On Fri, Nov 20, 2015 at 2:52 PM, Ryan Blue notifications@github.com wrote:

@danielcweeks https://github.com/danielcweeks, I'm doing some 1.9.0
planning. Is this something you guys would like to get in for that release?
If so, would you mark the JIRA a blocker for PARQUET-392
https://issues.apache.org/jira/browse/PARQUET-392?

—
Reply to this email directly or view it on GitHub
#286 (comment).

[julienledem]

@isnotinvain: any comment?

[rdblue]

This no longer calls readAsBytesInput, which gets the page data out of an existing buffer. It also decompresses the dictionary page, which I don't think was happening before. I've been editing this a bit, and I think things end up cleaner if we don't reuse the readDictionary method here.

[danielcweeks]

Yeah, I think I mentioned before that this probably doesn't work. We had some discussion about trying to consolidate the dictionary read logic so it can be exposed and used by other tools.

We can probably rework this to do both.

[rdblue]

I just had a look at the page reading part of this and will follow up tomorrow with comments on the dictionary filter.

[rdblue]

@danielcweeks: I implemented some of the suggestions I made in my branch: https://github.com/rdblue/parquet-mr/commits/PARQUET-384-add-dictionary-filtering

That makes dictionaries accessible through a DictionaryPageReader that you get from the ParquetFileReader. The row group filter gets a dictionary reader for each block and hands that to the filter test. I think this is a bit cleaner, but I got lazy and didn't move it into the InternalParquetRecordReader where there is already a file reader. I just pass in a new file reader that's reused.

I also noticed that there are two cases that this doesn't work well for. If there is no dictionary we have to handle that in the dictionary test. Also, I think we still need to account for the case where the encoding falls back to plain. In that case, we can't eliminate the row group.

[danielcweeks]

Closed in favor of: PR #330