GH-34785: [C++][Parquet] Add bloom filter write support #37400

mapleFU · 2023-08-26T14:26:49Z

Rationale for this change

Currently we allow reading bloom filter for specific column and row group. Now this patch allows writing bloom filters to Parquet files.

What changes are included in this PR?

Allow writing bf:

Add WriterProperties config for writing bloom filter, including bf and (per-rowgroup) ndv estimation.
Add BloomFilterBuilder for parquet
From FileSerializer to ColumnWriter, adding bloomfilter
Ensure Bloom Filter info is written to the file
Testing logic for BloomFilterBuilder
Testing logic for BloomFilter and ColumnWriter
Testing whole roundtrip like ParquetPageIndexRoundTripTest

Are these changes tested?

Yes

Are there any user-facing changes?

User can create Bloom Filter in parquet with C++ api

Closes: [C++][Parquet] Allow writing BloomFilter for specific column #34785
GitHub Issue: [C++][Parquet] Allow writing BloomFilter for specific column #34785

mapleFU · 2023-08-26T14:28:40Z

This is port of #35691 . I'm busy previous days and now I've time on it now.

The previous comment are solved. cc @pitrou @wgtmac @emkornfield

cpp/src/parquet/bloom_filter.h

cpp/src/parquet/bloom_filter_parquet_test.cc

cpp/src/parquet/column_writer.cc

cpp/src/parquet/bloom_filter_parquet_test.cc

cpp/src/parquet/metadata.h

cpp/src/parquet/file_writer.cc

wgtmac

Thanks for adding this! I just did an initial review except the test.

cpp/src/parquet/bloom_filter.h

cpp/src/parquet/properties.h

cpp/src/parquet/CMakeLists.txt

cpp/src/parquet/bloom_filter_builder.cc

cpp/src/parquet/arrow/arrow_reader_writer_test.cc

cpp/src/parquet/bloom_filter.h

cpp/src/parquet/bloom_filter_builder.h

cpp/src/parquet/bloom_filter_builder.cc

wgtmac · 2025-11-26T15:32:13Z

Thanks for the review, @HuaHuaY. I have addressed all your comments.

wgtmac · 2025-11-26T15:35:14Z

As I have made significant changes to this PR, I don't think I have the privilege to approve it. Could you please take a look when you have time? @pitrou

pitrou · 2025-11-26T15:36:11Z

Yes, I should do that. Sorry!

cpp/src/parquet/bloom_filter_writer.cc

pitrou

Some comments, I haven't looked at the tests. Overall the APIs look a bit complicated and clumsy to me.

cpp/src/parquet/CMakeLists.txt

cpp/src/parquet/metadata.h

cpp/src/parquet/page_index.h

cpp/src/parquet/bloom_filter_writer.h

cpp/src/parquet/bloom_filter_writer.cc

wgtmac · 2025-12-06T10:33:07Z

@pitrou Thanks for your review! I've addressed all your comments. Please take a look again.

wgtmac · 2026-01-13T09:22:24Z

Gentle ping @pitrou. Is it possible to include it to the next release?

pitrou · 2026-01-13T09:30:41Z

Hmm, I'm really sorry. I will make some time to review it quickly, but I'm afraid it's probably too late for this release as feature freeze in already in full force and @raulcd has started producing a RC :(

pitrou

This looks much better now, here are some more minor comments. Thanks for tackling this @wgtmac !

cpp/src/parquet/properties.h

cpp/src/parquet/type_fwd.h

cpp/src/parquet/file_writer.cc

cpp/src/parquet/bloom_filter_writer.cc

cpp/src/parquet/column_writer.cc

cpp/src/parquet/arrow/index_test.cc

pitrou · 2026-01-13T14:07:15Z

cpp/src/parquet/arrow/index_test.cc

+      if (value == std::nullopt) {
+        continue;
+      }
+      EXPECT_TRUE(bloom_filter.FindHash(bloom_filter.Hash(value.value())));


Not mandatory, but if we had FindHash(const Scalar&), it could remove the need for templating in these test functions.

This can be a separate improvement because the current bloom filter implementation does not deal with any Arrow type.

cpp/src/parquet/arrow/index_test.cc

pitrou · 2026-01-13T14:14:01Z

This needs rebasing on git main, by the way, also I think you'll need to update the fuzzing harness, for example in

arrow/cpp/src/parquet/arrow/fuzz_internal.cc

Lines 160 to 175 in d54a205

    
           Status FuzzReadPageIndex(RowGroupPageIndexReader* reader, const SchemaDescriptor* schema, 
        
                                    int column) { 
        
             Status st; 
        
             BEGIN_PARQUET_CATCH_EXCEPTIONS 
        
             auto offset_index = reader->GetOffsetIndex(column); 
        
             if (offset_index) { 
        
               offset_index->page_locations(); 
        
               offset_index->unencoded_byte_array_data_bytes(); 
        
             } 
        
             auto col_index = reader->GetColumnIndex(column); 
        
             if (col_index) { 
        
               st &= FuzzReadColumnIndex(col_index.get(), schema->Column(column)); 
        
             } 
        
             END_PARQUET_CATCH_EXCEPTIONS 
        
             return st; 
        
           }

wgtmac · 2026-01-14T08:13:38Z

Thanks for your review @pitrou. I think I've addressed all comments and rebased on top of the main branch. I think fuzz_internal.cc has already dealt with reading bloom filter, am I missing something?

pitrou · 2026-01-14T08:35:47Z

I think fuzz_internal.cc has already dealt with reading bloom filter, am I missing something?

I mean, perhaps the changed APIs mandate some code changes there? Let's see run the CI anyway.

pitrou · 2026-01-14T08:35:56Z

@github-actions crossbow submit -g cpp

github-actions · 2026-01-14T08:38:38Z

Revision: a126e03

Submitted crossbow builds: ursacomputing/crossbow @ actions-b54235b7c8

Task	Status
example-cpp-minimal-build-static
example-cpp-minimal-build-static-system-dependency
example-cpp-tutorial
test-build-cpp-fuzz
test-conda-cpp
test-conda-cpp-valgrind
test-debian-12-cpp-amd64
test-debian-12-cpp-i386
test-debian-experimental-cpp-gcc-15
test-fedora-42-cpp
test-ubuntu-22.04-cpp
test-ubuntu-22.04-cpp-20
test-ubuntu-22.04-cpp-bundled
test-ubuntu-22.04-cpp-emscripten
test-ubuntu-22.04-cpp-no-threading
test-ubuntu-24.04-cpp
test-ubuntu-24.04-cpp-bundled-offline
test-ubuntu-24.04-cpp-gcc-13-bundled
test-ubuntu-24.04-cpp-gcc-14
test-ubuntu-24.04-cpp-minimal-with-formats
test-ubuntu-24.04-cpp-thread-sanitizer

pitrou

Just a couple more minor comments, but LGTM in general.

cpp/src/parquet/properties.h

pitrou · 2026-01-14T09:32:56Z

cpp/src/parquet/properties.h

+  /// | 100,000    | 0.10  | ~6.0     | 75 KB       | **128 KB**   |
+  /// | 100,000    | 0.01  | ~10.5    | 131 KB      | **256 KB**   |


I'm not sure how you computed those numbers? Using the formula from OptimalNumOfBits (-8.0 / log(1 - pow(fpp, 1.0 / 8)) I get 5.77 bits / key for fpp = 0.1 (and 9.68 bits / key for fpp = 0.01).

It is from the table at https://github.com/apache/parquet-format/blob/4b1c72c837bec5b792b2514f0057533030fcedf8/BloomFilter.md?plain=1#L232

Hmm, that is unexpected. I'll open an issue on parquet-format. Regardless, it seems this table should list the actual numbers as obtained through our own OptimalNumOfBits function?

apache/parquet-format#547 opened.

cpp/src/parquet/bloom_filter_writer.cc

cpp/src/parquet/properties.h

pitrou · 2026-01-14T14:05:55Z

cpp/src/parquet/properties.h

+  /// | ndv        | fpp   | bits/key | size      |
+  /// |:-----------|:------|:---------|:----------|
+  /// | 100,000    | 0.10  | 10.5     | 128 KiB   |
+  /// | 100,000    | 0.01  | 10.5     | 128 KiB   |
+  /// | 1,000,000  | 0.10  | 8.4      | 1024 KiB  |
+  /// | 1,000,000  | 0.01  | 16.8     | 2048 KiB  |
+  /// | 10,000,000 | 0.10  | 6.7      | 8192 KiB  |
+  /// | 10,000,000 | 0.01  | 13.4     | 16384 KiB |


Since 0.05 is the default, perhaps show it as well? (sorry, should have noticed before!)

Make sense! Updated.

Parquet: Implement skeleton for BloomFilter

f1c6dc0

mapleFU requested a review from wgtmac as a code owner August 26, 2023 14:26

github-actions bot added Component: Parquet Component: C++ awaiting review Awaiting review labels Aug 26, 2023

tiny fixing

6ebd6da

mapleFU commented Aug 26, 2023

View reviewed changes

github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Aug 26, 2023

mapleFU added 2 commits August 26, 2023 23:32

tiny update test

70c9267

trying to fix ci

48350d8

mapleFU commented Aug 26, 2023

View reviewed changes

cpp/src/parquet/bloom_filter_parquet_test.cc Outdated Show resolved Hide resolved

fix lint

d2a659e

mapleFU commented Aug 26, 2023

View reviewed changes

cpp/src/parquet/metadata.h Show resolved Hide resolved

mapleFU added 4 commits August 27, 2023 00:44

fix some style problem

41236d8

add file roundtrip test

8afba81

add file roundtrip test

96c6691

fix document and ci

c131341

mapleFU commented Aug 26, 2023

View reviewed changes

cpp/src/parquet/file_writer.cc Outdated Show resolved Hide resolved

mapleFU requested review from emkornfield and pitrou August 26, 2023 18:43

Update: tiny style fix

220b58e

mapleFU changed the title ~~GH-34785: [C++][Parquet] Parquet Bloom Filter Implement~~ GH-34785: [C++][Parquet] Parquet Bloom Filter Write Implement Aug 27, 2023

wgtmac reviewed Aug 30, 2023

View reviewed changes

wgtmac changed the title ~~GH-34785: [C++][Parquet] Parquet Bloom Filter Write Implement~~ GH-34785: [C++][Parquet] Parquet Bloom Filter Writer Implementation Aug 30, 2023

pitrou requested changes Aug 31, 2023

View reviewed changes

github-actions bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Sep 1, 2023

Merge branch 'main' into parquet/support-write-bloom-filter

ad96c48

address comments

0f50418

XiangCao1998 reviewed Dec 2, 2025

View reviewed changes

cpp/src/parquet/bloom_filter_writer.cc Outdated Show resolved Hide resolved

pitrou requested changes Dec 3, 2025

View reviewed changes

address comments

789d130

wgtmac force-pushed the parquet/support-write-bloom-filter branch from 6c05857 to 789d130 Compare December 6, 2025 10:32

add template definition back

6dc8d88

pitrou requested changes Jan 13, 2026

View reviewed changes

pitrou mentioned this pull request Jan 13, 2026

[C++][Parquet][CI] Improve Parquet fuzzing seed corpus #43709

Open

wgtmac added 2 commits January 14, 2026 14:27

Merge branch 'main' into parquet/support-write-bloom-filter

0940cd8

polish test and address feedback

a126e03

pitrou reviewed Jan 14, 2026

View reviewed changes

wgtmac changed the title ~~GH-34785: [C++][Parquet] Parquet Bloom Filter Writer Implementation~~ GH-34785: [C++][Parquet] Add bloom filter write support Jan 14, 2026

address more comments

e560a28

pitrou reviewed Jan 14, 2026

View reviewed changes

cpp/src/parquet/properties.h Outdated Show resolved Hide resolved

update table

bf5e859

wgtmac reviewed Jan 14, 2026

View reviewed changes

cpp/src/parquet/properties.h Outdated Show resolved Hide resolved

pitrou reviewed Jan 14, 2026

View reviewed changes

update table

0638b11

wgtmac force-pushed the parquet/support-write-bloom-filter branch from 6e35c3e to 0638b11 Compare January 14, 2026 14:23

		/// \| 100,000 \| 0.10 \| ~6.0 \| 75 KB \| 128 KB \|
		/// \| 100,000 \| 0.01 \| ~10.5 \| 131 KB \| 256 KB \|

GH-34785: [C++][Parquet] Add bloom filter write support #37400

Are you sure you want to change the base?

GH-34785: [C++][Parquet] Add bloom filter write support #37400

Conversation

mapleFU commented Aug 26, 2023 • edited by wgtmac Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

mapleFU commented Aug 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wgtmac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wgtmac commented Nov 26, 2025

Uh oh!

wgtmac commented Nov 26, 2025

Uh oh!

pitrou commented Nov 26, 2025

Uh oh!

Uh oh!

pitrou left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wgtmac commented Dec 6, 2025

Uh oh!

wgtmac commented Jan 13, 2026

Uh oh!

pitrou commented Jan 13, 2026

Uh oh!

pitrou left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mapleFU commented Aug 26, 2023 •

edited by wgtmac

Loading

mapleFU commented Aug 26, 2023 •

edited

Loading