feat(bitmap): support DIFF, DIFF1, ANDOR, ONE for BITOP command - #3471
feat(bitmap): support DIFF, DIFF1, ANDOR, ONE for BITOP command#3471nkroker wants to merge 11 commits into
Conversation
Implements Redis 8.2+ bitwise operations: - DIFF: bits set in X but not in any Y - DIFF1: bits set in any Y but not in X - ANDOR: bits set in X and at least one Y - ONE: bits set in exactly one key Closes apache#3132 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
3603de9 to
9526046
Compare
|
Hi @nkroker. Thanks for your contribution. Please read our guidelines for AI-assisted code contributions: https://kvrocks.apache.org/community/contributing#guidelines-for-ai-assisted-contributions If you used AI-assisted coding, please let me know which software and model you used. This will help us review the code more effectively. |
|
Hi @jihuayu, I have used the Claude Opus 4.6 to make changes to the codebase, but I have also reviewed and checked the code by performing a build locally. I'll mention this in the description as well. |
jihuayu
left a comment
There was a problem hiding this comment.
- DIFF, DIFF1, and ANDOR produce incorrect results when the first source key does not exist, which is inconsistent with Redis semantics.
BITFIELD y SET u8 #0 15
BITOP DIFF dest nosuch y
BITFIELD dest GET u8 #0
In this PR
BITOP return length 1
BITFIELD dest GET u8 #0 return 15
But in Redis:
BITOP return length 1
BITFIELD dest GET u8 #0 return 0
It is also in DIFF、DIFF1、ANDOR
- DIFF, DIFF1, and ANDOR lack minimum source key validation and do not align with Redis semantics.
BITFIELD x SET u8 #0 170
BITOP DIFF dest x
BITFIELD dest GET u8 #0
it will throw error in redis.
PS: You can explicitly require your AI tools to follow Redis semantics when coding to avoid these types of issues.
|
Also, you need to format the code (use clang-format) to ensure the GitHub Actions pass. |
|
Hi @nkroker , just checking in on this PR since it's been quiet for a bit. Are you still planning to work on this? Let me know if you run into any issues and feel free to ping me. |
Yes I'll update it actually I'm travelling that's why it's been open from few days |
Wow, thanks for your reply. Hope you have a great trip! |
…d key count validation - DIFF/DIFF1/ANDOR now correctly treat missing X as zero (Redis semantics) - DIFF/DIFF1/ANDOR now require at least two source keys - Fix clang-format violations - Add edge case tests documenting Redis semantics for missing keys Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
7171f79 to
d6a53d2
Compare
|
It looks like there may have been an issue while updating the code. Do you need any help? |
d6a53d2 to
ceaaf7f
Compare
|
Hey @jihuayu is this PR still required or should I close it? |
|
Hi @nkroker. Welcome back—we need you. I’ll find some time to take a look at the code. |
There was a problem hiding this comment.
Pull request overview
This PR extends Kvrocks’ bitmap implementation to support additional Redis 8.2+ BITOP operations (DIFF, DIFF1, ANDOR, ONE). It updates command parsing, implements the new byte-wise operation semantics in the bitmap type layer (excluding them from the existing 64-bit fast path), and adds Go unit tests (including fuzzing) to validate correctness and Redis-compatible missing-key behavior.
Changes:
- Add new bitmap operation flags (
DIFF,DIFF1,ANDOR,ONE) and wire them intoBITOPparsing. - Implement the new ops in
Bitmap::BitOpwith correct handling of missing keys (including Redis semantics for missing first key). - Add targeted unit tests plus fuzzing to validate multi-key and missing-key cases for all new operations.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/types/redis_bitmap.h |
Adds new BitOpFlags values for the four new operations. |
src/types/redis_bitmap.cc |
Implements new BITOP semantics in the bitmap engine and gates the 64-bit fast path for these ops. |
src/commands/cmd_bit.cc |
Extends BITOP op-name parsing and validates minimum arity for DIFF/DIFF1/ANDOR. |
tests/gocase/unit/type/bitmap/bitmap_test.go |
Adds correctness tests and fuzz coverage for the new operations and missing-key semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
BITOPoperations:DIFF,DIFF1,ANDOR,ONEDIFF: bits set in X but not in any Y key (X & ~Y1 & ~Y2 & ...)DIFF1: bits set in any Y key but not in X ((Y1 | Y2 | ...) & ~X)ANDOR: bits set in X and in at least one Y key (X & (Y1 | Y2 | ...))ONE: bits set in exactly one key across all inputsChanges
src/types/redis_bitmap.h— 4 newBitOpFlagsenum valuessrc/types/redis_bitmap.cc— byte-loop logic for new ops + excluded from 64-bit fast pathsrc/commands/cmd_bit.cc— parsediff/diff1/andor/oneop name stringstests/gocase/unit/type/bitmap/bitmap_test.go— 9 targeted tests + fuzzing for all 4 opsTest plan
Ai Tools Used
Closes #3132