PARQUET-41: Add bloom filters to parquet statistics #215
Conversation
| @@ -57,6 +58,7 @@ | |||
| private ValuesWriter dataColumn; | |||
| private int valueCount; | |||
| private int valueCountForNextSizeCheck; | |||
| private BloomFilterOpts opts; | |||
There was a problem hiding this comment.
Could we rename 'opts' to 'bloomFilterOpts' to increase readability along the code?
|
It's looking good Ferd. Here are some questions I have.
|
| org.apache.parquet.column.statistics.Statistics statistics) { | ||
| if (!(statistics instanceof BloomFilterStatistics)) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
I don't follow why we need this test and define the signature of this method this way.
can't we just convert from one type to the other?
There was a problem hiding this comment.
In the stage of converting, it constructs a statistics used in parquet-format and updates the data retrieving from the statistics from parquet-mr.
|
Hi @spena
At this point, I didn't add the support for fall back. If it's really useful, I think we could do it in a follow-up ticket.
The bloom filter is used to filter a entire row group in the same way as min/max statistics. I am not very familiar with dictionary encoding in parquet. But I think it should be used before dictionary encoding.
I tried to think about a way to calculate it but didn't come up with a good idea. But I think nobody understands the data better than the person who uses it. |
winningsix
force-pushed
the
PARQUET-41
branch
2 times, most recently
from
a347660
to
4ba507c
Compare
winningsix
force-pushed
the
PARQUET-41
branch
from
5c220da
to
ac15839
Compare
PARQUET-41: Update patch addressing comments Parquet-41: Adding other data types support and enable Unit tests Change the bitset from arraylist to array Add statistics option and enable tests for bloom filter Fix failed unit tests Remove the page level bloom filter bit set Rebase code
It's the PR in mr part.