-
Notifications
You must be signed in to change notification settings - Fork 24.2k
Fix bug in PFMERGE command #13672
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
Fix bug in PFMERGE command #13672
Conversation
Signed-off-by: Xuyang Wang <xuyangwang@link.cuhk.edu.cn>
Signed-off-by: Xuyang Wang <xuyangwang@link.cuhk.edu.cn>
Signed-off-by: Xuyang Wang <xuyangwang@link.cuhk.edu.cn>
Signed-off-by: Xuyang Wang <xuyangwang@link.cuhk.edu.cn>
Co-authored-by: debing.sun <debing.sun@redis.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, @sundb do we need to add release-notes
tag for this, as it introduces PFDEBUG SIMD (ON|OFF)
subcommand
@ShooterIT no, because #13558 doesn't release. |
I don't mean we mention bug fix instead of new sub command |
@ShooterIT you're right, we should add release note label for sub command. |
Hi @YaacovHazan I want to merge this PR. Even this PR introduces a subcommand |
The bug was introduced in #13558 . When merging dense hll structures, `hllDenseCompress` writes to wrong location and the result will be zero. The unit tests didn't cover this case. This PR + fixes the bug + adds `PFDEBUG SIMD (ON|OFF)` for unit tests + adds a new TCL test to cover the cases Synchronized from valkey-io/valkey#1293 --------- Signed-off-by: Xuyang Wang <xuyangwang@link.cuhk.edu.cn> Co-authored-by: debing.sun <debing.sun@redis.com>
The bug was introduced in #13558 .
When merging dense hll structures,
hllDenseCompress
writes to wrong location and the result will be zero. The unit tests didn't cover this case.This PR
PFDEBUG SIMD (ON|OFF)
for unit testsSynchronized from valkey-io/valkey#1293