You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/11/11 12:28:54 UTC

[GitHub] [arrow-rs] alamb commented on a diff in pull request #3057: add bloom filter implementation based on split block (sbbf) spec

alamb commented on code in PR #3057:
URL: https://github.com/apache/arrow-rs/pull/3057#discussion_r1020166795


##########
parquet/src/bloom_filter/mod.rs:
##########
@@ -0,0 +1,212 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Bloom filter implementation specific to Parquet, as described
+//! in the [spec](https://github.com/apache/parquet-format/blob/master/BloomFilter.md)
+
+use crate::errors::ParquetError;
+use crate::file::metadata::ColumnChunkMetaData;
+use crate::format::{
+    BloomFilterAlgorithm, BloomFilterCompression, BloomFilterHash, BloomFilterHeader,
+};
+use std::hash::Hasher;
+use std::io::{Read, Seek, SeekFrom};
+use thrift::protocol::TCompactInputProtocol;
+use twox_hash::XxHash64;
+
+const SALT: [u32; 8] = [
+    0x47b6137b_u32,
+    0x44974d91_u32,
+    0x8824ad5b_u32,
+    0xa2b7289d_u32,
+    0x705495c7_u32,
+    0x2df1424b_u32,
+    0x9efc4947_u32,
+    0x5c6bfb31_u32,
+];
+
+/// Each block is 256 bits, broken up into eight contiguous "words", each consisting of 32 bits.
+/// Each word is thought of as an array of bits; each bit is either "set" or "not set".
+type Block = [u32; 8];
+
+/// takes as its argument a single unsigned 32-bit integer and returns a block in which each
+/// word has exactly one bit set.
+fn mask(x: u32) -> Block {
+    let mut result = [0_u32; 8];
+    for i in 0..8 {
+        // wrapping instead of checking for overflow

Review Comment:
   i don't know the implications of using wrapping mul here



##########
parquet/src/bloom_filter/mod.rs:
##########
@@ -0,0 +1,212 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Bloom filter implementation specific to Parquet, as described
+//! in the [spec](https://github.com/apache/parquet-format/blob/master/BloomFilter.md)
+
+use crate::errors::ParquetError;
+use crate::file::metadata::ColumnChunkMetaData;
+use crate::format::{
+    BloomFilterAlgorithm, BloomFilterCompression, BloomFilterHash, BloomFilterHeader,
+};
+use std::hash::Hasher;
+use std::io::{Read, Seek, SeekFrom};
+use thrift::protocol::TCompactInputProtocol;
+use twox_hash::XxHash64;
+
+const SALT: [u32; 8] = [
+    0x47b6137b_u32,
+    0x44974d91_u32,
+    0x8824ad5b_u32,
+    0xa2b7289d_u32,
+    0x705495c7_u32,
+    0x2df1424b_u32,
+    0x9efc4947_u32,
+    0x5c6bfb31_u32,
+];
+
+/// Each block is 256 bits, broken up into eight contiguous "words", each consisting of 32 bits.
+/// Each word is thought of as an array of bits; each bit is either "set" or "not set".
+type Block = [u32; 8];
+
+/// takes as its argument a single unsigned 32-bit integer and returns a block in which each
+/// word has exactly one bit set.
+fn mask(x: u32) -> Block {
+    let mut result = [0_u32; 8];
+    for i in 0..8 {
+        // wrapping instead of checking for overflow
+        let y = x.wrapping_mul(SALT[i]);
+        let y = y >> 27;
+        result[i] = 1 << y;
+    }
+    result
+}
+
+/// setting every bit in the block that was also set in the result from mask
+fn block_insert(block: &mut Block, hash: u32) {
+    let mask = mask(hash);
+    for i in 0..8 {
+        block[i] |= mask[i];
+    }
+}
+
+/// returns true when every bit that is set in the result of mask is also set in the block.
+fn block_check(block: &Block, hash: u32) -> bool {
+    let mask = mask(hash);
+    for i in 0..8 {
+        if block[i] & mask[i] == 0 {
+            return false;
+        }
+    }
+    true
+}
+
+/// A split block Bloom filter
+pub struct Sbbf(Vec<Block>);
+
+impl Sbbf {
+    fn new(bitset: &[u8]) -> Self {
+        let data = bitset
+            .chunks_exact(4 * 8)
+            .map(|chunk| {
+                let mut block = [0_u32; 8];
+                for (i, word) in chunk.chunks_exact(4).enumerate() {
+                    block[i] = u32::from_le_bytes(word.try_into().unwrap());
+                }
+                block
+            })
+            .collect::<Vec<Block>>();
+        Self(data)
+    }
+
+    pub fn read_from_column_chunk<R: Read + Seek>(

Review Comment:
   Is there any way to write a test for this function? Maybe we can do so eventually using the data in https://github.com/apache/parquet-testing/tree/master/data



##########
parquet/src/bloom_filter/mod.rs:
##########
@@ -0,0 +1,212 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Bloom filter implementation specific to Parquet, as described
+//! in the [spec](https://github.com/apache/parquet-format/blob/master/BloomFilter.md)
+
+use crate::errors::ParquetError;
+use crate::file::metadata::ColumnChunkMetaData;
+use crate::format::{
+    BloomFilterAlgorithm, BloomFilterCompression, BloomFilterHash, BloomFilterHeader,
+};
+use std::hash::Hasher;
+use std::io::{Read, Seek, SeekFrom};
+use thrift::protocol::TCompactInputProtocol;
+use twox_hash::XxHash64;
+
+const SALT: [u32; 8] = [
+    0x47b6137b_u32,
+    0x44974d91_u32,
+    0x8824ad5b_u32,
+    0xa2b7289d_u32,
+    0x705495c7_u32,
+    0x2df1424b_u32,
+    0x9efc4947_u32,
+    0x5c6bfb31_u32,
+];
+
+/// Each block is 256 bits, broken up into eight contiguous "words", each consisting of 32 bits.
+/// Each word is thought of as an array of bits; each bit is either "set" or "not set".
+type Block = [u32; 8];
+
+/// takes as its argument a single unsigned 32-bit integer and returns a block in which each
+/// word has exactly one bit set.
+fn mask(x: u32) -> Block {
+    let mut result = [0_u32; 8];
+    for i in 0..8 {
+        // wrapping instead of checking for overflow
+        let y = x.wrapping_mul(SALT[i]);
+        let y = y >> 27;
+        result[i] = 1 << y;
+    }
+    result
+}
+
+/// setting every bit in the block that was also set in the result from mask
+fn block_insert(block: &mut Block, hash: u32) {
+    let mask = mask(hash);
+    for i in 0..8 {
+        block[i] |= mask[i];
+    }
+}
+
+/// returns true when every bit that is set in the result of mask is also set in the block.
+fn block_check(block: &Block, hash: u32) -> bool {
+    let mask = mask(hash);
+    for i in 0..8 {
+        if block[i] & mask[i] == 0 {
+            return false;
+        }
+    }
+    true
+}
+
+/// A split block Bloom filter
+pub struct Sbbf(Vec<Block>);
+
+impl Sbbf {
+    fn new(bitset: &[u8]) -> Self {
+        let data = bitset
+            .chunks_exact(4 * 8)
+            .map(|chunk| {
+                let mut block = [0_u32; 8];
+                for (i, word) in chunk.chunks_exact(4).enumerate() {
+                    block[i] = u32::from_le_bytes(word.try_into().unwrap());
+                }
+                block
+            })
+            .collect::<Vec<Block>>();
+        Self(data)
+    }
+
+    pub fn read_from_column_chunk<R: Read + Seek>(
+        column_metadata: &ColumnChunkMetaData,
+        mut reader: &mut R,
+    ) -> Result<Self, ParquetError> {
+        let offset = column_metadata.bloom_filter_offset().ok_or_else(|| {
+            ParquetError::General("Bloom filter offset is not set".to_string())
+        })? as u64;
+        reader.seek(SeekFrom::Start(offset))?;
+        // deserialize header
+        let mut prot = TCompactInputProtocol::new(&mut reader);
+        let header = BloomFilterHeader::read_from_in_protocol(&mut prot)?;
+
+        match header.algorithm {
+            BloomFilterAlgorithm::BLOCK(_) => {
+                // this match exists to future proof the singleton algorithm enum
+            }
+        }
+        match header.compression {
+            BloomFilterCompression::UNCOMPRESSED(_) => {
+                // this match exists to future proof the singleton compression enum
+            }
+        }
+        match header.hash {
+            BloomFilterHash::XXHASH(_) => {
+                // this match exists to future proof the singleton hash enum
+            }
+        }
+        let length: usize = header.num_bytes.try_into().map_err(|_| {
+            ParquetError::General("Bloom filter length is invalid".to_string())
+        })?;
+        let mut buffer = vec![0_u8; length];
+        reader.read_exact(&mut buffer).map_err(|e| {
+            ParquetError::General(format!("Could not read bloom filter: {}", e))
+        })?;
+        Ok(Self::new(&buffer))
+    }
+
+    #[inline]
+    fn hash_to_block_index(&self, hash: u64) -> usize {
+        // unchecked_mul is unstable, but in reality this is safe, we'd just use saturating mul
+        // but it will not saturate
+        (((hash >> 32).saturating_mul(self.0.len() as u64)) >> 32) as usize
+    }
+
+    /// Insert a hash into the filter
+    pub fn insert(&mut self, hash: u64) {
+        let block_index = self.hash_to_block_index(hash);
+        let block = &mut self.0[block_index];
+        block_insert(block, hash as u32);
+    }
+
+    /// Check if a hash is in the filter
+    pub fn check(&self, hash: u64) -> bool {
+        let block_index = self.hash_to_block_index(hash);
+        let block = &self.0[block_index];
+        block_check(block, hash as u32)
+    }
+}
+
+// per spec we use xxHash with seed=0
+const SEED: u64 = 0;
+
+pub fn hash_bytes<A: AsRef<[u8]>>(value: A) -> u64 {
+    let mut hasher = XxHash64::with_seed(SEED);
+    hasher.write(value.as_ref());
+    hasher.finish()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_hash_bytes() {
+        assert_eq!(hash_bytes(b""), 17241709254077376921);
+    }
+
+    #[test]
+    fn test_mask_set_quick_check() {
+        for i in 0..1_000_000 {
+            let result = mask(i);
+            assert!(result.iter().all(|&x| x.count_ones() == 1));
+        }
+    }
+
+    #[test]
+    fn test_block_insert_and_check() {
+        for i in 0..1_000_000 {
+            let mut block = [0_u32; 8];
+            block_insert(&mut block, i);
+            assert!(block_check(&block, i));
+        }
+    }
+
+    #[test]
+    fn test_sbbf_insert_and_check() {
+        let mut sbbf = Sbbf(vec![[0_u32; 8]; 1_000]);
+        for i in 0..1_000_000 {
+            sbbf.insert(i);
+            assert!(sbbf.check(i));
+        }
+    }
+
+    #[test]
+    fn test_with_fixture() {

Review Comment:
   I am not sure where this data came from -- it might help to add comments such as are in https://github.com/jorgecarleitao/parquet2/blob/main/src/bloom_filter/mod.rs#L14-L69
   
   Also, there is a second test dataset in `basics()` that might be good to bring over too



##########
parquet/src/bloom_filter/mod.rs:
##########
@@ -0,0 +1,212 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Bloom filter implementation specific to Parquet, as described
+//! in the [spec](https://github.com/apache/parquet-format/blob/master/BloomFilter.md)
+
+use crate::errors::ParquetError;
+use crate::file::metadata::ColumnChunkMetaData;
+use crate::format::{
+    BloomFilterAlgorithm, BloomFilterCompression, BloomFilterHash, BloomFilterHeader,
+};
+use std::hash::Hasher;
+use std::io::{Read, Seek, SeekFrom};
+use thrift::protocol::TCompactInputProtocol;
+use twox_hash::XxHash64;
+
+const SALT: [u32; 8] = [
+    0x47b6137b_u32,
+    0x44974d91_u32,
+    0x8824ad5b_u32,
+    0xa2b7289d_u32,
+    0x705495c7_u32,
+    0x2df1424b_u32,
+    0x9efc4947_u32,
+    0x5c6bfb31_u32,
+];
+
+/// Each block is 256 bits, broken up into eight contiguous "words", each consisting of 32 bits.
+/// Each word is thought of as an array of bits; each bit is either "set" or "not set".
+type Block = [u32; 8];
+
+/// takes as its argument a single unsigned 32-bit integer and returns a block in which each
+/// word has exactly one bit set.
+fn mask(x: u32) -> Block {
+    let mut result = [0_u32; 8];
+    for i in 0..8 {
+        // wrapping instead of checking for overflow
+        let y = x.wrapping_mul(SALT[i]);
+        let y = y >> 27;
+        result[i] = 1 << y;
+    }
+    result
+}
+
+/// setting every bit in the block that was also set in the result from mask
+fn block_insert(block: &mut Block, hash: u32) {
+    let mask = mask(hash);
+    for i in 0..8 {
+        block[i] |= mask[i];
+    }
+}
+
+/// returns true when every bit that is set in the result of mask is also set in the block.
+fn block_check(block: &Block, hash: u32) -> bool {
+    let mask = mask(hash);
+    for i in 0..8 {
+        if block[i] & mask[i] == 0 {
+            return false;
+        }
+    }
+    true
+}
+
+/// A split block Bloom filter
+pub struct Sbbf(Vec<Block>);
+
+impl Sbbf {
+    fn new(bitset: &[u8]) -> Self {
+        let data = bitset
+            .chunks_exact(4 * 8)
+            .map(|chunk| {
+                let mut block = [0_u32; 8];
+                for (i, word) in chunk.chunks_exact(4).enumerate() {
+                    block[i] = u32::from_le_bytes(word.try_into().unwrap());
+                }
+                block
+            })
+            .collect::<Vec<Block>>();
+        Self(data)
+    }
+
+    pub fn read_from_column_chunk<R: Read + Seek>(
+        column_metadata: &ColumnChunkMetaData,
+        mut reader: &mut R,
+    ) -> Result<Self, ParquetError> {
+        let offset = column_metadata.bloom_filter_offset().ok_or_else(|| {
+            ParquetError::General("Bloom filter offset is not set".to_string())
+        })? as u64;
+        reader.seek(SeekFrom::Start(offset))?;
+        // deserialize header
+        let mut prot = TCompactInputProtocol::new(&mut reader);
+        let header = BloomFilterHeader::read_from_in_protocol(&mut prot)?;
+
+        match header.algorithm {
+            BloomFilterAlgorithm::BLOCK(_) => {
+                // this match exists to future proof the singleton algorithm enum
+            }
+        }
+        match header.compression {
+            BloomFilterCompression::UNCOMPRESSED(_) => {
+                // this match exists to future proof the singleton compression enum
+            }
+        }
+        match header.hash {
+            BloomFilterHash::XXHASH(_) => {
+                // this match exists to future proof the singleton hash enum
+            }
+        }
+        let length: usize = header.num_bytes.try_into().map_err(|_| {
+            ParquetError::General("Bloom filter length is invalid".to_string())
+        })?;
+        let mut buffer = vec![0_u8; length];
+        reader.read_exact(&mut buffer).map_err(|e| {
+            ParquetError::General(format!("Could not read bloom filter: {}", e))
+        })?;
+        Ok(Self::new(&buffer))
+    }
+
+    #[inline]
+    fn hash_to_block_index(&self, hash: u64) -> usize {
+        // unchecked_mul is unstable, but in reality this is safe, we'd just use saturating mul
+        // but it will not saturate
+        (((hash >> 32).saturating_mul(self.0.len() as u64)) >> 32) as usize
+    }
+
+    /// Insert a hash into the filter
+    pub fn insert(&mut self, hash: u64) {
+        let block_index = self.hash_to_block_index(hash);
+        let block = &mut self.0[block_index];
+        block_insert(block, hash as u32);
+    }
+
+    /// Check if a hash is in the filter

Review Comment:
   ```suggestion
       /// Check if a hash is in the filter. May return
       /// true ("false positive") for values that was never inserted
       /// but will always return false if a hash has not been inserted. 
   ```



##########
parquet/src/bloom_filter/mod.rs:
##########
@@ -0,0 +1,212 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Bloom filter implementation specific to Parquet, as described
+//! in the [spec](https://github.com/apache/parquet-format/blob/master/BloomFilter.md)
+
+use crate::errors::ParquetError;
+use crate::file::metadata::ColumnChunkMetaData;
+use crate::format::{
+    BloomFilterAlgorithm, BloomFilterCompression, BloomFilterHash, BloomFilterHeader,
+};
+use std::hash::Hasher;
+use std::io::{Read, Seek, SeekFrom};
+use thrift::protocol::TCompactInputProtocol;
+use twox_hash::XxHash64;
+
+const SALT: [u32; 8] = [

Review Comment:
   ```suggestion
   /// Salt as defined in the [spec](https://github.com/apache/parquet-format/blob/master/BloomFilter.md#technical-approach)
   const SALT: [u32; 8] = [
   ```



##########
parquet/src/bloom_filter/mod.rs:
##########
@@ -0,0 +1,212 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Bloom filter implementation specific to Parquet, as described
+//! in the [spec](https://github.com/apache/parquet-format/blob/master/BloomFilter.md)
+
+use crate::errors::ParquetError;
+use crate::file::metadata::ColumnChunkMetaData;
+use crate::format::{
+    BloomFilterAlgorithm, BloomFilterCompression, BloomFilterHash, BloomFilterHeader,
+};
+use std::hash::Hasher;
+use std::io::{Read, Seek, SeekFrom};
+use thrift::protocol::TCompactInputProtocol;
+use twox_hash::XxHash64;
+
+const SALT: [u32; 8] = [
+    0x47b6137b_u32,
+    0x44974d91_u32,
+    0x8824ad5b_u32,
+    0xa2b7289d_u32,
+    0x705495c7_u32,
+    0x2df1424b_u32,
+    0x9efc4947_u32,
+    0x5c6bfb31_u32,
+];
+
+/// Each block is 256 bits, broken up into eight contiguous "words", each consisting of 32 bits.
+/// Each word is thought of as an array of bits; each bit is either "set" or "not set".
+type Block = [u32; 8];
+
+/// takes as its argument a single unsigned 32-bit integer and returns a block in which each
+/// word has exactly one bit set.
+fn mask(x: u32) -> Block {
+    let mut result = [0_u32; 8];
+    for i in 0..8 {
+        // wrapping instead of checking for overflow
+        let y = x.wrapping_mul(SALT[i]);
+        let y = y >> 27;
+        result[i] = 1 << y;
+    }
+    result
+}
+
+/// setting every bit in the block that was also set in the result from mask
+fn block_insert(block: &mut Block, hash: u32) {
+    let mask = mask(hash);
+    for i in 0..8 {
+        block[i] |= mask[i];
+    }
+}
+
+/// returns true when every bit that is set in the result of mask is also set in the block.
+fn block_check(block: &Block, hash: u32) -> bool {
+    let mask = mask(hash);
+    for i in 0..8 {
+        if block[i] & mask[i] == 0 {
+            return false;
+        }
+    }
+    true
+}
+
+/// A split block Bloom filter
+pub struct Sbbf(Vec<Block>);
+
+impl Sbbf {
+    fn new(bitset: &[u8]) -> Self {
+        let data = bitset
+            .chunks_exact(4 * 8)
+            .map(|chunk| {
+                let mut block = [0_u32; 8];
+                for (i, word) in chunk.chunks_exact(4).enumerate() {
+                    block[i] = u32::from_le_bytes(word.try_into().unwrap());
+                }
+                block
+            })
+            .collect::<Vec<Block>>();
+        Self(data)
+    }
+
+    pub fn read_from_column_chunk<R: Read + Seek>(
+        column_metadata: &ColumnChunkMetaData,
+        mut reader: &mut R,
+    ) -> Result<Self, ParquetError> {
+        let offset = column_metadata.bloom_filter_offset().ok_or_else(|| {
+            ParquetError::General("Bloom filter offset is not set".to_string())
+        })? as u64;
+        reader.seek(SeekFrom::Start(offset))?;
+        // deserialize header
+        let mut prot = TCompactInputProtocol::new(&mut reader);
+        let header = BloomFilterHeader::read_from_in_protocol(&mut prot)?;
+
+        match header.algorithm {
+            BloomFilterAlgorithm::BLOCK(_) => {
+                // this match exists to future proof the singleton algorithm enum
+            }
+        }
+        match header.compression {
+            BloomFilterCompression::UNCOMPRESSED(_) => {
+                // this match exists to future proof the singleton compression enum
+            }
+        }
+        match header.hash {
+            BloomFilterHash::XXHASH(_) => {
+                // this match exists to future proof the singleton hash enum
+            }
+        }
+        let length: usize = header.num_bytes.try_into().map_err(|_| {

Review Comment:
   ```suggestion
           // length in bytes
           let length: usize = header.num_bytes.try_into().map_err(|_| {
   ```



##########
parquet/Cargo.toml:
##########
@@ -57,6 +57,7 @@ seq-macro = { version = "0.3", default-features = false }
 futures = { version = "0.3", default-features = false, features = ["std"], optional = true }
 tokio = { version = "1.0", optional = true, default-features = false, features = ["macros", "rt", "io-util"] }
 hashbrown = { version = "0.13", default-features = false }
+twox-hash = { version = "1.6", optional = true }

Review Comment:
   👍 



##########
parquet/src/bloom_filter/mod.rs:
##########
@@ -0,0 +1,212 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Bloom filter implementation specific to Parquet, as described
+//! in the [spec](https://github.com/apache/parquet-format/blob/master/BloomFilter.md)
+
+use crate::errors::ParquetError;
+use crate::file::metadata::ColumnChunkMetaData;
+use crate::format::{
+    BloomFilterAlgorithm, BloomFilterCompression, BloomFilterHash, BloomFilterHeader,
+};
+use std::hash::Hasher;
+use std::io::{Read, Seek, SeekFrom};
+use thrift::protocol::TCompactInputProtocol;
+use twox_hash::XxHash64;
+
+const SALT: [u32; 8] = [
+    0x47b6137b_u32,
+    0x44974d91_u32,
+    0x8824ad5b_u32,
+    0xa2b7289d_u32,
+    0x705495c7_u32,
+    0x2df1424b_u32,
+    0x9efc4947_u32,
+    0x5c6bfb31_u32,
+];
+
+/// Each block is 256 bits, broken up into eight contiguous "words", each consisting of 32 bits.
+/// Each word is thought of as an array of bits; each bit is either "set" or "not set".
+type Block = [u32; 8];
+
+/// takes as its argument a single unsigned 32-bit integer and returns a block in which each
+/// word has exactly one bit set.
+fn mask(x: u32) -> Block {
+    let mut result = [0_u32; 8];
+    for i in 0..8 {
+        // wrapping instead of checking for overflow
+        let y = x.wrapping_mul(SALT[i]);
+        let y = y >> 27;
+        result[i] = 1 << y;
+    }
+    result
+}
+
+/// setting every bit in the block that was also set in the result from mask
+fn block_insert(block: &mut Block, hash: u32) {
+    let mask = mask(hash);
+    for i in 0..8 {
+        block[i] |= mask[i];
+    }
+}
+
+/// returns true when every bit that is set in the result of mask is also set in the block.
+fn block_check(block: &Block, hash: u32) -> bool {
+    let mask = mask(hash);
+    for i in 0..8 {
+        if block[i] & mask[i] == 0 {
+            return false;
+        }
+    }
+    true
+}
+
+/// A split block Bloom filter
+pub struct Sbbf(Vec<Block>);
+
+impl Sbbf {
+    fn new(bitset: &[u8]) -> Self {
+        let data = bitset
+            .chunks_exact(4 * 8)
+            .map(|chunk| {
+                let mut block = [0_u32; 8];
+                for (i, word) in chunk.chunks_exact(4).enumerate() {
+                    block[i] = u32::from_le_bytes(word.try_into().unwrap());
+                }
+                block
+            })
+            .collect::<Vec<Block>>();
+        Self(data)
+    }
+
+    pub fn read_from_column_chunk<R: Read + Seek>(
+        column_metadata: &ColumnChunkMetaData,
+        mut reader: &mut R,
+    ) -> Result<Self, ParquetError> {
+        let offset = column_metadata.bloom_filter_offset().ok_or_else(|| {
+            ParquetError::General("Bloom filter offset is not set".to_string())
+        })? as u64;
+        reader.seek(SeekFrom::Start(offset))?;
+        // deserialize header
+        let mut prot = TCompactInputProtocol::new(&mut reader);
+        let header = BloomFilterHeader::read_from_in_protocol(&mut prot)?;
+
+        match header.algorithm {
+            BloomFilterAlgorithm::BLOCK(_) => {
+                // this match exists to future proof the singleton algorithm enum
+            }
+        }
+        match header.compression {
+            BloomFilterCompression::UNCOMPRESSED(_) => {
+                // this match exists to future proof the singleton compression enum
+            }
+        }
+        match header.hash {
+            BloomFilterHash::XXHASH(_) => {
+                // this match exists to future proof the singleton hash enum
+            }
+        }
+        let length: usize = header.num_bytes.try_into().map_err(|_| {
+            ParquetError::General("Bloom filter length is invalid".to_string())
+        })?;
+        let mut buffer = vec![0_u8; length];
+        reader.read_exact(&mut buffer).map_err(|e| {
+            ParquetError::General(format!("Could not read bloom filter: {}", e))
+        })?;
+        Ok(Self::new(&buffer))
+    }
+
+    #[inline]
+    fn hash_to_block_index(&self, hash: u64) -> usize {
+        // unchecked_mul is unstable, but in reality this is safe, we'd just use saturating mul
+        // but it will not saturate
+        (((hash >> 32).saturating_mul(self.0.len() as u64)) >> 32) as usize
+    }
+
+    /// Insert a hash into the filter
+    pub fn insert(&mut self, hash: u64) {
+        let block_index = self.hash_to_block_index(hash);
+        let block = &mut self.0[block_index];
+        block_insert(block, hash as u32);
+    }
+
+    /// Check if a hash is in the filter
+    pub fn check(&self, hash: u64) -> bool {
+        let block_index = self.hash_to_block_index(hash);
+        let block = &self.0[block_index];
+        block_check(block, hash as u32)
+    }
+}
+
+// per spec we use xxHash with seed=0
+const SEED: u64 = 0;
+
+pub fn hash_bytes<A: AsRef<[u8]>>(value: A) -> u64 {
+    let mut hasher = XxHash64::with_seed(SEED);
+    hasher.write(value.as_ref());
+    hasher.finish()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_hash_bytes() {
+        assert_eq!(hash_bytes(b""), 17241709254077376921);
+    }
+
+    #[test]
+    fn test_mask_set_quick_check() {
+        for i in 0..1_000_000 {
+            let result = mask(i);
+            assert!(result.iter().all(|&x| x.count_ones() == 1));
+        }
+    }
+
+    #[test]
+    fn test_block_insert_and_check() {
+        for i in 0..1_000_000 {
+            let mut block = [0_u32; 8];
+            block_insert(&mut block, i);
+            assert!(block_check(&block, i));
+        }
+    }
+
+    #[test]
+    fn test_sbbf_insert_and_check() {
+        let mut sbbf = Sbbf(vec![[0_u32; 8]; 1_000]);
+        for i in 0..1_000_000 {
+            sbbf.insert(i);
+            assert!(sbbf.check(i));
+        }
+    }
+
+    #[test]
+    fn test_with_fixture() {

Review Comment:
   ```suggestion
       fn test_with_fixture() {
               // bloom filter produced by parquet-mr/spark for a column of i64 f"a{i}" for i in 0..10
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org