Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: make bloom filter optional on column level #615

Merged
merged 5 commits into from
Feb 7, 2023

Conversation

ShiKaiWi
Copy link
Member

@ShiKaiWi ShiKaiWi commented Feb 6, 2023

Which issue does this PR close?

Closes #

Rationale for this change

Currently, the bloom filter is only optional on file level, and the bloom filters on column level are still required. However, in some cases, a column bloom filter may be useless, which can be avoided after this PR.

And for compatibility, the version of bloom filter should be introduced.

What changes are included in this PR?

  • Introduce the version of bloom filter. [Breaking Change]
  • Make bloom filter optional on column level.

Are there any user-facing changes?

None.

How does this change test

Existing tests.

@ShiKaiWi ShiKaiWi changed the title feat: make bloom filter optional in both row-group level and column level feat: make bloom filter optional on column level Feb 6, 2023
@ShiKaiWi ShiKaiWi added feature New feature or request A-analytic-engine Area: Analytic Engine breaking-change Contains user-facing changes labels Feb 6, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #615 (fcb2a9d) into main (fe32f85) will increase coverage by 0.00%.
The diff coverage is 88.70%.

@@           Coverage Diff           @@
##             main     #615   +/-   ##
=======================================
  Coverage   66.02%   66.02%           
=======================================
  Files         285      285           
  Lines       44358    44395   +37     
=======================================
+ Hits        29288    29314   +26     
- Misses      15070    15081   +11     
Impacted Files Coverage Δ
analytic_engine/src/sst/parquet/meta_data.rs 79.57% <86.79%> (+0.33%) ⬆️
analytic_engine/src/sst/parquet/async_reader.rs 89.49% <100.00%> (ø)
...nalytic_engine/src/sst/parquet/row_group_filter.rs 82.22% <100.00%> (+0.19%) ⬆️
analytic_engine/src/sst/parquet/writer.rs 98.18% <100.00%> (ø)
analytic_engine/src/sst/file.rs 76.17% <0.00%> (-1.09%) ⬇️
interpreters/src/tests.rs 96.07% <0.00%> (-0.36%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@jiacai2050 jiacai2050 left a comment

Choose a reason for hiding this comment

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

LGTM

@ShiKaiWi ShiKaiWi merged commit 1cab6ba into apache:main Feb 7, 2023
chunshao90 pushed a commit to chunshao90/ceresdb that referenced this pull request May 15, 2023
* feat: make bloom filter optional in both row-group level and column level

* make bloom-filter row_group level required

* fix relative importing

* add UT for conversion of BloomFilter

* fix ut of conversion for bloom filter
@ShiKaiWi ShiKaiWi deleted the feat-bloom-filter-compatible branch May 29, 2023 02:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-analytic-engine Area: Analytic Engine breaking-change Contains user-facing changes feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants