Skip to content

GH-48877: [C++][Parquet] Fix writer not to throw for bloom filter on disabled bool column - #48878

Merged
wgtmac merged 1 commit into
apache:mainfrom
wgtmac:fix_bf_bool
Jan 19, 2026
Merged

GH-48877: [C++][Parquet] Fix writer not to throw for bloom filter on disabled bool column#48878
wgtmac merged 1 commit into
apache:mainfrom
wgtmac:fix_bf_bool

Conversation

@wgtmac

@wgtmacwgtmac commented Jan 16, 2026

Copy link
Copy Markdown
Member

Rationale for this change

Now that #37400 is merged. After a quick integration, I found that the Parquet writer throws if bloom filter is enabled and there is at least one boolean column (even when it is not enabled).

What changes are included in this PR?

Move the bloom filter type check after bloom filter enablement check.

Are these changes tested?

Yes, added test to verify this does not throw.

Are there any user-facing changes?

No.

@github-actions

Copy link
Copy Markdown

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

@HuaHuaYHuaHuaY left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@github-actionsgithub-actionsBot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Jan 16, 2026
@wgtmac
wgtmac requested review from mapleFU and pitrouJanuary 16, 2026 06:14
@wgtmac
wgtmac merged commit 8b648be into apache:mainJan 19, 2026
52 of 58 checks passed
@wgtmacwgtmac removed the awaiting committer review Awaiting committer review label Jan 19, 2026
@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 8b648be.

There were no benchmark performance regressions. 🎉

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

Mottl pushed a commit to Mottl/arrow that referenced this pull request May 26, 2026
…er on disabled bool column (apache#48878)
### Rationale for this change
Now that apache#37400 is merged. After a quick integration, I found that the Parquet writer throws if bloom filter is enabled and there is at least one boolean column (even when it is not enabled).
### What changes are included in this PR?
Move the bloom filter type check after bloom filter enablement check.
### Are these changes tested?
Yes, added test to verify this does not throw.
### Are there any user-facing changes?
No.
* GitHub Issue: apache#48877
Authored-by: Gang Wu <ustcwg@gmail.com>
Signed-off-by: Gang Wu <ustcwg@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.

3 participants

@wgtmac@mapleFU@HuaHuaY