Thanks Gidon to correct me. On Sun, Aug 4, 2019 at 10:37 PM Gidon Gershinsky <[email protected]> wrote: > > The main reason for bloom filter header encryption is tamper-proofing it > with AES GCM. > If not done, an attacker can alter the hash/algorithm fields, making the > readers lose relevant data on the one hand, and load/filter irrelevant data > on the other hand. > Bloom filters are encrypted differently from pages (GCM only, no page > ordinal, etc). > > On Sun, Aug 4, 2019 at 4:47 PM 俊杰陈 <[email protected]> wrote: > > > Many thanks for these useful comments！ > > > > I will create a PR to update format according to these comments. > > Reply some of them inline: > > > > The spec should give more detail on how to choose the number of blocks > > and on false positive rates. The sentence with “11.54 bits for each > > distinct value inserted into the filter” is vague: is this the > > multi-block filter? Why is a 1% false-positive rate “recommended”? I > > think it is okay to use 0.5% as each block’s false-positive rate, but > > then this should state how to achieve an overall false-positive rate > > as a function of the number of distinct values. > > [junjie]: IIUC, 0.5% is overall FPR. The reason to recommend 1% FPR > > is that it can use about 1.2MB bitset to store one million distinct > > values which should satisfy most cases, but I cannot guarantee that > > fit extreme cases such as only one column in a table, so I also > > recommend to use a configuration to set the FPR. In the implementation > > in the bloom-filter branch, I also set default max bloom filter size > > to 1MB. > > > > Should the bloom filters support compression? > > [junjie], The compression should help as you mentioned. I didn't add > > it because I think the size of a bloom filter should be small for a > > column chunk. Also, we may skip building bloom filter if there is an > > existing dictionary. > > > > Why is the bloom filter header encrypted? > > In a discussion with Gidon, we 'd like to follow the current way when > > encrypting pages. > > > > > > On Sun, Aug 4, 2019 at 4:42 AM Ryan Blue <[email protected]> wrote: > > > > > > Right now, I’m voting -1 on the spec as written because I don’t think > > that it is clear enough to be correctly implemented from just the spec. > > I’ll change that with a few corrections. > > > > > > Here are the items I think need to be fixed before I’ll change my vote > > to support this spec: > > > > > > The algorithm should be more clear: > > > > > > How is the bloom filter block selected from the 32 most-significant bits > > from of the hash function? These details must be in the spec and not in > > papers linked from the spec. > > > How is the number of blocks determined? From the overall filter size? > > > I think that the exact procedure for a lookup in each block should be > > covered in a section, followed by a section for how to perform a look up in > > the multi-block filter. The wording also needs to be cleaned up so that it > > is always clear whether the filter that is referenced is a block or the > > multi-block filter. > > > > > > The spec should give more detail on how to choose the number of blocks > > and on false positive rates. The sentence with “11.54 bits for each > > distinct value inserted into the filter” is vague: is this the multi-block > > filter? Why is a 1% false-positive rate “recommended”? > > > > > > I think it is okay to use 0.5% as each block’s false-positive rate, but > > then this should state how to achieve an overall false-positive rate as a > > function of the number of distinct values. > > > > > > This should be more clear about the scope of bloom filters. The only > > reference right now is “The Bloom filter data of a column chunk, …”. If > > each filter is for a column chunk, then this should be stated as a > > requirement. > > > The position of bloom filter headers and bloom filter bit sets is too > > vague: “The Bloom filter data of a column chunk, which contains the size of > > the filter in bytes, the algorithm, the hash function and the Bloom filter > > bitset, is stored near the footer.” > > > > > > This should state that the column chunk offset points to the start of > > the bloom filter header, which immediately precedes the bloom filter bytes. > > The number of bytes must be stored as numBytes in the header. Include > > layout diagrams like the ones in the encryption spec. > > > The position of all bloom filters in relation to the page indexes should > > be clear. Bloom filters should be written just before column indexes? > > > The position of each bloom filter within the bloom filter data section > > should be clear. Are bloom filters for a column located together, or bloom > > filters for a row group? (R1C1, R2C1, R1C2, R2C2 vs R1C1, R1C2, R2C1, R2C2) > > > Does the format allow locating bloom filter data anywhere else other > > than just before the header? Maybe the locations are recommendations and > > not requirements? > > > > > > Should the bloom filters support compression? If the strategy for a > > lower false-positive rate is to under-fill the multi-block filter, then > > would compression help reduce the storage cost? > > > Why is the bloom filter header encrypted? > > > > > > > > > On Wed, Jul 31, 2019 at 10:01 AM 俊杰陈 <[email protected]> wrote: > > >> > > >> Hi Wes, > > >> > > >> Thanks for the reply. It indeed does not have automation support in > > >> the test. Since now we have the repo parquet-testing for the > > >> integration test, I think we can do some automation base on that. I > > >> will take some time to look at this and update the integration plan in > > >> PARQUET-1326. > > >> > > >> Besides that, we can open a thread to discuss building automation > > >> integration test framework for features WIP and in the future. > > >> > > >> On Thu, Aug 1, 2019 at 12:10 AM Ryan Blue <[email protected]> > > wrote: > > >> > > > >> > Wes, > > >> > > > >> > Since the v2 format is unfinished, I don't think anyone should be > > writing > > >> > v2 pages. In fact, I don't think v2 pages would be recommended in the > > v2 > > >> > spec at this point. Do you know why Spark is writing this data? I > > didn't > > >> > know that Spark would write v2 by default. > > >> > > > >> > rb > > >> > > > >> > On Wed, Jul 31, 2019 at 8:28 AM Wes McKinney <[email protected]> > > wrote: > > >> > > > >> > > I'm not sure when I can have a more thorough look at this. > > >> > > > > >> > > To be honest, I'm personally struggling with the burden of > > supporting > > >> > > the existing feature set in the Parquet C++ library. The integration > > >> > > testing strategy for this as well as other features that are being > > >> > > added to both the Java and C++ libraries (e.g. encryption) make me > > >> > > uncomfortable due to the lack of automation. As an example, > > DataPageV2 > > >> > > support in parquet-cpp has been broken since the beginning of the > > >> > > project (see PARQUET-458) but it's only recently become an issue > > when > > >> > > people have been trying to read such files produced by Spark. More > > >> > > comprehensive integration testing would help ensure that the > > libraries > > >> > > remain compatible. > > >> > > > > >> > > On Tue, Jul 30, 2019 at 9:17 PM 俊杰陈 <[email protected]> wrote: > > >> > > > > > >> > > > Dear Parquet developers > > >> > > > > > >> > > > We still need your vote! > > >> > > > > > >> > > > > > >> > > > On Wed, Jul 24, 2019 at 9:30 PM 俊杰陈 <[email protected]> wrote: > > >> > > > > > > >> > > > > Hi @Ryan Blue @Wes McKinney > > >> > > > > > > >> > > > > We need your valuable vote, any feedback is welcome as well. > > >> > > > > > > >> > > > > On Tue, Jul 23, 2019 at 1:24 PM 俊杰陈 <[email protected]> wrote: > > >> > > > > > > > >> > > > > > Call for voting again. > > >> > > > > > > > >> > > > > > On Fri, Jul 19, 2019 at 1:17 PM 俊杰陈 <[email protected]> > > wrote: > > >> > > > > > > > > >> > > > > > > Dear Parquet developers > > >> > > > > > > > > >> > > > > > > We need more votes, please help to vote on this. > > >> > > > > > > > > >> > > > > > > On Wed, Jul 17, 2019 at 3:42 PM Gabor Szadovszky > > >> > > > > > > <[email protected]> wrote: > > >> > > > > > > > > > >> > > > > > > > After getting in PARQUET-1625 I vote again for having > > bloom > > >> > > filter spec and > > >> > > > > > > > the thrift file update as is in parquet-format master. > > >> > > > > > > > +1 (binding) > > >> > > > > > > > > > >> > > > > > > > On Mon, Jul 15, 2019 at 3:23 PM 俊杰陈 <[email protected]> > > wrote: > > >> > > > > > > > > > >> > > > > > > > > Thanks Gabor, It's never too late to make it better. We > > don't > > >> > > have to > > >> > > > > > > > > run it in a hurry, it has been developed for a long > > time yet.:) > > >> > > > > > > > > > > >> > > > > > > > > The thrift file is indeed a bit lag behind the spec. As > > the > > >> > > spec > > >> > > > > > > > > defined, the bloom filter data is stored near the > > footer which > > >> > > means > > >> > > > > > > > > we don't have to handle it like the page. Therefore, I > > just > > >> > > opened a > > >> > > > > > > > > jira to remove bloom_filter_page_header in PageHeader > > >> > > structure, while > > >> > > > > > > > > the BloomFitlerHeader is kept intentionally for > > convenience. > > >> > > Since the > > >> > > > > > > > > spec and the thrift should be aligned with each other > > >> > > eventually, so > > >> > > > > > > > > the vote target is both of them. > > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > On Mon, Jul 15, 2019 at 7:48 PM Gabor Szadovszky > > >> > > > > > > > > <[email protected]> wrote: > > >> > > > > > > > > > > > >> > > > > > > > > > Hi Junjie, > > >> > > > > > > > > > > > >> > > > > > > > > > Sorry for bringing up this a bit late but I have some > > >> > > problems with the > > >> > > > > > > > > > format update. The parquet.thrift file is updated to > > have > > >> > > the bloom > > >> > > > > > > > > filters > > >> > > > > > > > > > as a page (just as dictionaries and data pages). > > Meanwhile, > > >> > > the spec > > >> > > > > > > > > > (BloomFilter.md) says that the bloom filter is stored > > near > > >> > > the footer. > > >> > > > > > > > > So, > > >> > > > > > > > > > if the bloom filter is not part of the row-groups > > (like > > >> > > column indexes) I > > >> > > > > > > > > > would not add it as a page. See the struct > > ColumnIndex in > > >> > > the thrift > > >> > > > > > > > > file. > > >> > > > > > > > > > This struct is not referenced anywhere in it only > > declared. > > >> > > It was done > > >> > > > > > > > > > this way because we don't parse it in the same way as > > we > > >> > > parse the pages. > > >> > > > > > > > > > > > >> > > > > > > > > > Currently, I am not 100% sure about the target of > > this vote. > > >> > > If it is a > > >> > > > > > > > > > vote about adding bloom filters in general then it is > > a +1 > > >> > > (binding). If > > >> > > > > > > > > it > > >> > > > > > > > > > is about adding the bloom filters to parquet-format > > as is > > >> > > then, it is a > > >> > > > > > > > > -1 > > >> > > > > > > > > > (binding) until we fix the issue above. > > >> > > > > > > > > > > > >> > > > > > > > > > Regards, > > >> > > > > > > > > > Gabor > > >> > > > > > > > > > > > >> > > > > > > > > > On Mon, Jul 15, 2019 at 11:45 AM Gidon Gershinsky < > > >> > > [email protected]> > > >> > > > > > > > > wrote: > > >> > > > > > > > > > > > >> > > > > > > > > > > +1 (non-binding) > > >> > > > > > > > > > > > > >> > > > > > > > > > > On Mon, Jul 15, 2019 at 12:08 PM Zoltan Ivanfi > > >> > > <[email protected] > > >> > > > > > > > > > > > >> > > > > > > > > > > wrote: > > >> > > > > > > > > > > > > >> > > > > > > > > > > > +1 (binding) > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > On Mon, Jul 15, 2019 at 9:57 AM 俊杰陈 < > > [email protected]> > > >> > > wrote: > > >> > > > > > > > > > > > > > > >> > > > > > > > > > > > > Dear Parquet developers > > >> > > > > > > > > > > > > > > >> > > > > > > > > > > > > I'd like to resume this vote, you can start to > > vote > > >> > > now. Thanks for > > >> > > > > > > > > > > your > > >> > > > > > > > > > > > time. > > >> > > > > > > > > > > > > > > >> > > > > > > > > > > > > On Wed, Jul 10, 2019 at 9:29 PM 俊杰陈 < > > >> > > [email protected]> wrote: > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > I see, will resume this next week. Thanks. > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > On Wed, Jul 10, 2019 at 5:26 PM Zoltan Ivanfi > > >> > > > > > > > > > > <[email protected]> > > >> > > > > > > > > > > > wrote: > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > Hi Junjie, > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > Since there are ongoing improvements > > addressing > > >> > > review > > >> > > > > > > > > comments, I > > >> > > > > > > > > > > > would > > >> > > > > > > > > > > > > > > hold off with the vote for a few more days > > until > > >> > > the > > >> > > > > > > > > specification > > >> > > > > > > > > > > > settles. > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > Br, > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > Zoltan > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > On Wed, Jul 10, 2019 at 9:32 AM 俊杰陈 < > > >> > > [email protected]> > > >> > > > > > > > > wrote: > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > Hi Parquet committers and developers > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > We are waiting for your important ballot:) > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > On Tue, Jul 9, 2019 at 10:21 AM 俊杰陈 < > > >> > > [email protected]> > > >> > > > > > > > > wrote: > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > Yes, there are some public benchmark > > results, > > >> > > such as the > > >> > > > > > > > > > > > official > > >> > > > > > > > > > > > > > > > > benchmark from xxhash site ( > > >> > > http://www.xxhash.com/) and > > >> > > > > > > > > > > > published > > >> > > > > > > > > > > > > > > > > comparison from smhasher project > > >> > > > > > > > > > > > > > > > > (https://github.com/rurban/smhasher/). > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > On Tue, Jul 9, 2019 at 5:25 AM Wes > > McKinney < > > >> > > > > > > > > > > [email protected]> > > >> > > > > > > > > > > > wrote: > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > Do you have any benchmark data to > > support > > >> > > the choice of > > >> > > > > > > > > hash > > >> > > > > > > > > > > > function? > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > On Wed, Jul 3, 2019 at 8:41 AM 俊杰陈 < > > >> > > [email protected]> > > >> > > > > > > > > > > wrote: > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > Dear Parquet developers > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > To simplify the voting, I 'd like to > > >> > > update voting > > >> > > > > > > > > content > > >> > > > > > > > > > > > to the > > >> > > > > > > > > > > > > > > > spec > > >> > > > > > > > > > > > > > > > > > > with xxHash hash strategy. Now you > > can > > >> > > reply with +1 > > >> > > > > > > > > or -1. > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > Thanks for your participation. > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > On Tue, Jul 2, 2019 at 10:23 AM 俊杰陈 > > < > > >> > > > > > > > > [email protected]> > > >> > > > > > > > > > > > wrote: > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > Dear Parquet developers > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > Parquet Bloom filter has been > > developed > > >> > > for a while, > > >> > > > > > > > > per > > >> > > > > > > > > > > > the > > >> > > > > > > > > > > > > > > > discussion on the mail list, it's time to > > call a > > >> > > vote for > > >> > > > > > > > > spec to > > >> > > > > > > > > > > > move > > >> > > > > > > > > > > > > > > > forward. The current spec can be found at > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > https://github.com/apache/parquet-format/blob/master/BloomFilter.md > > . > > >> > > > > > > > > > > > > > > > There are some different options about the > > >> > > internal hash > > >> > > > > > > > > choice > > >> > > > > > > > > > > of > > >> > > > > > > > > > > > Bloom > > >> > > > > > > > > > > > > > > > filter and the PR is for that concern. > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > So I 'd like to propose to vote > > the spec > > >> > > + hash > > >> > > > > > > > > option, > > >> > > > > > > > > > > for > > >> > > > > > > > > > > > > > > > example: > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > +1 to spec and xxHash > > >> > > > > > > > > > > > > > > > > > > > +1 to spec and murmur3 > > >> > > > > > > > > > > > > > > > > > > > ... > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > Please help to vote, any feedback > > is > > >> > > also welcome in > > >> > > > > > > > > the > > >> > > > > > > > > > > > > > > > discussion thread. > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > Thanks & Best Regards > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > -- > > >> > > > > > > > > > > > > > > > > > > Thanks & Best Regards > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > -- > > >> > > > > > > > > > > > > > > > > Thanks & Best Regards > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > -- > > >> > > > > > > > > > > > > > > > Thanks & Best Regards > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > -- > > >> > > > > > > > > > > > > > Thanks & Best Regards > > >> > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > >> > > > > > > > > > > > > -- > > >> > > > > > > > > > > > > Thanks & Best Regards > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > -- > > >> > > > > > > > > Thanks & Best Regards > > >> > > > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > -- > > >> > > > > > > Thanks & Best Regards > > >> > > > > > > > >> > > > > > > > >> > > > > > > > >> > > > > > -- > > >> > > > > > Thanks & Best Regards > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > -- > > >> > > > > Thanks & Best Regards > > >> > > > > > >> > > > > > >> > > > > > >> > > > -- > > >> > > > Thanks & Best Regards > > >> > > > > >> > > > >> > > > >> > -- > > >> > Ryan Blue > > >> > Software Engineer > > >> > Netflix > > >> > > >> > > >> > > >> -- > > >> Thanks & Best Regards > > > > > > > > > > > > -- > > > Ryan Blue > > > Software Engineer > > > Netflix > > > > > > > > -- > > Thanks & Best Regards > >
-- Thanks & Best Regards
