Skip to content

GH-49376: [Python][Parquet] Add ability to write Bloom filters from pyarrow - #49377

Merged
mapleFU merged 7 commits into
apache:mainfrom
etseidl:pyarrow_bloom
Apr 6, 2026
Merged

GH-49376: [Python][Parquet] Add ability to write Bloom filters from pyarrow#49377
mapleFU merged 7 commits into
apache:mainfrom
etseidl:pyarrow_bloom

Conversation

@etseidl

@etseidletseidl commented Feb 23, 2026

Copy link
Copy Markdown
Contributor

Fixes#49376

Rationale for this change

Adds ability to enable the writing of Parquet Bloom filters via pyarrow.

What changes are included in this PR?

Adds bloom_filter_options to parquet.write_table.

Are these changes tested?

Yes, new tests are added.

Are there any user-facing changes?

Adds an option (defaults to None) to parquet.write_table.

@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #49376has been automatically assigned in GitHub to PR creator.

Comment threadpython/pyarrow/_parquet.pyx Outdated
elif isinstance(_bloom_opts, bool):
# if false do nothing, if true then just pass defaults
if not _bloom_opts:
continue

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that bloom filter is disabled by default but should we be explicit here calling props.disable_bloom_filter(tobytes(column))?
Otherwise exposing Builder* disable_bloom_filter(const c_string& path) on libparquet.pxd seems slightly unnecessary as is never being used.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree it would be better to be explicit here. Thanks!

@github-actionsgithub-actionsBot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Feb 24, 2026
@github-actionsgithub-actionsBot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Feb 24, 2026
@etseidletseidl changed the title GH-49376: [Python] Add ability to write Bloom filters from pyarrowGH-49376: [Python][Parquet] Add ability to write Bloom filters from pyarrowFeb 24, 2026
Comment threadpython/pyarrow/parquet/core.py Outdated
The keys of the `dict` are column paths. For each path, the value can be either:

- A boolean, with ``True`` indicating that a Bloom filter should be produced with
the default values of `NDV=1048576` and `FPP=0.05`.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lower case?

Comment threadpython/pyarrow/parquet/core.py Outdated
- A boolean, with ``True`` indicating that a Bloom filter should be produced with
the default values of `NDV=1048576` and `FPP=0.05`.
- A dictionary, with keys `ndv` and `fpp`. `ndv` must be a positive integer, and
`fpp` must be a float between 0.0 and 1.0. Default values will be used for any

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexists ndv and fpp is regarded as 1048576/0.05 ?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I've reworked the docs to hopefully make this clearer.

@github-actionsgithub-actionsBot added awaiting changes Awaiting changes and removed Component: Parquet awaiting change review Awaiting change review labels Feb 25, 2026
@etseidl
etseidl requested a review from mapleFUMarch 3, 2026 16:10

@mapleFUmapleFU left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General LGTM. Sorry for late review!

Comment threadpython/pyarrow/_parquet.pyx Outdated
if isinstance(fpp, float):
if fpp <= 0.0 or fpp >= 1.0:
raise ValueError(
f"'fpp' for column '{column}' must be in (0.0, 1,0), got {fpp}")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1,0 typo here?

Comment threadpython/pyarrow/_parquet.pyx Outdated
if "ndv" in _bloom_opts:
ndv = _bloom_opts["ndv"]
if isinstance(ndv, int):
if ndv < 0:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ndv <= 0?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if NDV can be 0 or not...I can't seem to find validation in either the C++ nor rust implementations. This test just ensures it's a positive value. Honestly, I don't see why you'd need a bloom filter for a column with no values, so I'm fine following your suggestion.

Comment threadpython/pyarrow/_parquet.pyx Outdated
Comment threadpython/pyarrow/_parquet.pyx Outdated
bloom_opts.ndv = ndv
else:
raise TypeError(
f"'ndv' for column '{column}' must be an int")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we notice they're in bloom filter options?

@github-actionsgithub-actionsBot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Mar 24, 2026

@mapleFUmapleFU left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @pitrou@raulcd would you mind take a look? Otherwise I'll merge this next week

Comment threadpython/pyarrow/_parquet.pyx Outdated
f"'bloom_filter_options:ndv' for column '{column}' must be an int")
if "fpp" in _bloom_opts:
fpp = _bloom_opts["fpp"]
if isinstance(fpp, float):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know whether casting to float is a better way

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. We don't want to accept strings, for example.

@raulcdraulcd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a minor question about defining the defaults vs trying to reuse the ones defined on the C++ struct, so we don't diverge if at any point we change the defaults.
But overall all looks good!

Comment threadpython/pyarrow/_parquet.pyx Outdated
@github-actionsgithub-actionsBot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Mar 25, 2026
Comment threadpython/pyarrow/_parquet.pyx Outdated

# bloom filters
if bloom_filter_options is not None:
if isinstance(bloom_filter_options, dict):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we factor this out into a helper function? This function is becoming much too large IMHO.

@github-actionsgithub-actionsBot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Mar 25, 2026

@raulcdraulcd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @etseidl

@github-actionsgithub-actionsBot added awaiting merge Awaiting merge and removed awaiting change review Awaiting change review labels Mar 27, 2026
yharby added a commit to walkthru-earth/walkthru-overture-index that referenced this pull request Mar 28, 2026
PyArrow 23.0.1 has no API for writing Parquet bloom filters.
The C++ layer supports it (apache/arrow#37400) but Python
bindings are still in draft (apache/arrow#49377).
Row group size (2000) and sorting metadata still work and
provide the main pushdown benefit. Bloom filters are kept
as commented-out TODOs for when PyArrow 24+ ships.
@mapleFU

Copy link
Copy Markdown
Member

I would merge if no objection next monday

@raulcd

Copy link
Copy Markdown
Member

I would merge if no objection next monday

Sounds good to me. I've added the 24.0.0 milestone to the original issue so I don't forget to pick it unless there are other concerns being worked on.

@mapleFU
mapleFU merged commit 5fd30d3 into apache:mainApr 6, 2026
17 checks passed
@mapleFUmapleFU removed the awaiting merge Awaiting merge label Apr 6, 2026
@mapleFU

Copy link
Copy Markdown
Member

Thanks all, pr is merged

@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 5fd30d3.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 4 possible false positives for unstable benchmarks that are known to sometimes produce them.

thisisnic pushed a commit to thisisnic/arrow that referenced this pull request Apr 6, 2026
…from pyarrow (apache#49377)
Fixesapache#49376
### Rationale for this change
Adds ability to enable the writing of Parquet Bloom filters via pyarrow.
### What changes are included in this PR?
Adds `bloom_filter_options` to `parquet.write_table`.
### Are these changes tested?
Yes, new tests are added.
### Are there any user-facing changes?
Adds an option (defaults to `None`) to `parquet.write_table`.
* GitHub Issue: apache#49376
Authored-by: seidl <seidl2@llnl.gov>
Signed-off-by: mwish <maplewish117@gmail.com>
Mottl pushed a commit to Mottl/arrow that referenced this pull request May 26, 2026
…from pyarrow (apache#49377)
Fixesapache#49376
### Rationale for this change
Adds ability to enable the writing of Parquet Bloom filters via pyarrow.
### What changes are included in this PR?
Adds `bloom_filter_options` to `parquet.write_table`.
### Are these changes tested?
Yes, new tests are added.
### Are there any user-facing changes?
Adds an option (defaults to `None`) to `parquet.write_table`.
* GitHub Issue: apache#49376
Authored-by: seidl <seidl2@llnl.gov>
Signed-off-by: mwish <maplewish117@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Python][Parquet] Add options to control writing of Bloom filters to parquet.write_table

4 participants

@etseidl@mapleFU@raulcd@pitrou