Skip to content

feat(CuckooFilter): add cf.exists and cf.mexists command#3467

Open
nagisa-kunhah wants to merge 12 commits into
apache:unstablefrom
nagisa-kunhah:feature/3359-cf-exists
Open

feat(CuckooFilter): add cf.exists and cf.mexists command#3467
nagisa-kunhah wants to merge 12 commits into
apache:unstablefrom
nagisa-kunhah:feature/3359-cf-exists

Conversation

@nagisa-kunhah

Copy link
Copy Markdown
Contributor

task id: #3359

@nagisa-kunhah

Copy link
Copy Markdown
Contributor Author

@jihuayu Sorry for the noise. The initial version of this PR branch accidentally contained replayed upstream commits because it was not cleanly rebased onto the latest upstream/unstable. This caused GitHub to show unrelated files in the PR diff and also created automatic cross-references to other PRs from the replayed commit messages.
Can we clear these messages?

@jihuayu

jihuayu commented Apr 29, 2026

Copy link
Copy Markdown
Member

@nagisa-kunhah Thank you for your contribution. It doesn't matter. I only care about the code diff.
However, please take note: you need to understand all the code you submit, as you will be held ultimately responsible for it.

Please @ me if you have any other questions.

Tips:

@nagisa-kunhah
nagisa-kunhah force-pushed the feature/3359-cf-exists branch from 3471314 to 76c01ae Compare July 9, 2026 15:44

Copilot AI 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.

Pull request overview

This PR adds RedisBloom-compatible existence queries for Kvrocks’ CuckooFilter implementation by introducing CF.EXISTS and CF.MEXISTS commands, backed by new read-path APIs in the Cuckoo filter type layer and validated through both Go integration/unit tests and C++ unit tests.

Changes:

  • Add CF.EXISTS and CF.MEXISTS command implementations and register them as read-only commands.
  • Implement redis::CuckooChain::Exists / MExists using per-sub-filter bucket lookups via a new CuckooSubFilter::Contains helper.
  • Add test coverage for missing keys, wrong types, argument validation, ordering, and post-expansion behavior (Go + C++).

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/gocase/unit/type/bloom/cuckoo_filter_test.go Adds Go tests for cf.exists/cf.mexists, including wrongtype/arity and expansion scenarios.
tests/cppunit/types/cuckoo_filter_test.cc Adds extensive C++ unit tests for Exists/MExists behavior (including expansion and edge-case items).
src/types/redis_cuckoo_chain.h Exposes new Exists/MExists APIs on the Cuckoo filter type.
src/types/redis_cuckoo_chain.cc Implements Exists/MExists by scanning sub-filters and checking bucket slots.
src/types/cuckoo_filter_sub_filter.h Adds a read-only Contains method for sub-filter membership checks.
src/types/cuckoo_filter_sub_filter.cc Implements Contains using cached page prefetch + per-slot scans of candidate buckets.
src/storage/redis_metadata.h Clarifies base_capacity meaning for auto-created vs reserved filters (comment update).
src/commands/cmd_cuckoo_filter.cc Adds cf.exists and cf.mexists Commander implementations and registers them.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jihuayu jihuayu 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.

Hi @nagisa-kunhah. Thank you for your contribution. Overall, this looks good.

Comment thread src/commands/cmd_cuckoo_filter.cc Outdated
Comment thread src/commands/cmd_cuckoo_filter.cc Outdated
Comment thread src/types/redis_cuckoo_chain.cc Outdated
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
42.0% Coverage on New Code (required ≥ 50%)

See analysis details on SonarQube Cloud

@nagisa-kunhah

Copy link
Copy Markdown
Contributor Author

Hi @jihuayu , thank you for your careful review. I have addressed all your comments and added some unit tests. Feel free to @ me if you have any questions.

Comment thread tests/cppunit/types/cuckoo_filter_test.cc Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants