Skip to content

refactor: make BloomFilterProperties fpp/ndv private with accessors#9969

Merged
alamb merged 1 commit into
apache:mainfrom
CuteChuanChuan:raymond/bloomfilterproperties-private-fields
May 14, 2026
Merged

refactor: make BloomFilterProperties fpp/ndv private with accessors#9969
alamb merged 1 commit into
apache:mainfrom
CuteChuanChuan:raymond/bloomfilterproperties-private-fields

Conversation

@CuteChuanChuan
Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

#9877 introduced BloomFilterPropertiesBuilder. With the builder in place, the public fpp / ndv fields constrain future evolution. Locking them down behind accessors makes the public API smaller and more evolvable, matching the original direction outlined in #9667.

What changes are included in this PR?

  • Make BloomFilterProperties::fpp and BloomFilterProperties::ndv private fields
  • Add pub fn fpp(&self) -> f64 and pub fn ndv(&self) -> u64 accessor methods
  • Move the per-field documentation verbatim onto the accessor methods so it remains rendered by rustdoc (private fields are not rendered)

Are these changes tested?

Yes — the existing test suite covers the affected paths

Are there any user-facing changes?

Yes — this is a breaking change to the public API.

  • External struct-literal construction no longer compiles:
// Before — no longer works
BloomFilterProperties { fpp: 0.01, ndv: 10_000 }

// Use the builder added in #9877 instead:
BloomFilterProperties::builder()
  .with_fpp(0.01)
  .with_max_ndv(10_000)
  .build()
  • Direct field reads (props.fpp, props.ndv) no longer compile; use props.fpp() / props.ndv() instead.

- Replace public `fpp` / `ndv` fields on `BloomFilterProperties` with `pub fn fpp()` and `pub fn ndv()` accessors
- Move the original per-field documentation verbatim onto the accessor methods so rustdoc renders it
- BREAKING CHANGE: external struct-literal construction (`BloomFilterProperties { fpp, ndv }`) and direct field reads no longer compile.
@github-actions github-actions Bot added the parquet Changes to the parquet crate label May 13, 2026
Copy link
Copy Markdown
Contributor

@adriangb adriangb left a comment

Choose a reason for hiding this comment

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

Looks good to me! @alamb do we need to wait for some release before we merge this?

Copy link
Copy Markdown
Contributor

@etseidl etseidl left a comment

Choose a reason for hiding this comment

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

Looks good to me. This is a breaking change, but luckily 'tis the season. 😄

@etseidl etseidl added the api-change Changes to the arrow API label May 13, 2026
@alamb
Copy link
Copy Markdown
Contributor

alamb commented May 14, 2026

Looks good to me. This is a breaking change, but luckily 'tis the season. 😄

🎄

@alamb alamb merged commit 28d7537 into apache:main May 14, 2026
16 checks passed
@alamb
Copy link
Copy Markdown
Contributor

alamb commented May 14, 2026

Thank you @CuteChuanChuan @adriangb and @etseidl

@CuteChuanChuan CuteChuanChuan deleted the raymond/bloomfilterproperties-private-fields branch May 16, 2026 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-change Changes to the arrow API parquet Changes to the parquet crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants