SET: add IFNE conditional option#3105
Open
arshidkv12 wants to merge 5 commits intovalkey-io:unstablefrom
Open
Conversation
Signed-off-by: arshidkv12 <[email protected]>
Signed-off-by: arshidkv12 <[email protected]>
Signed-off-by: arshidkv12 <[email protected]>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## unstable #3105 +/- ##
============================================
- Coverage 74.27% 74.19% -0.08%
============================================
Files 129 129
Lines 71041 71057 +16
============================================
- Hits 52764 52720 -44
- Misses 18277 18337 +60
🚀 New features to boost your workflow:
|
Signed-off-by: arshidkv12 <[email protected]>
Signed-off-by: arshidkv12 <[email protected]>
Contributor
Author
|
Please check it @zuiderkwast |
PingXie
added a commit
to PingXie/valkey
that referenced
this pull request
Jan 27, 2026
Issue: PR valkey-io#3105 was passing even though it's a subset of Redis #14435. - Valkey: Adds IFNE option (132 lines, 7 files) - Redis: Adds IFEQ/IFNE/IFNEQ + xxhash + digest (9843 lines, 18 files) - Jaccard similarity only 20% due to size difference - But 90-100% of Valkey tokens exist in Redis (subset relationship) Fix: Check subset ratio in addition to Jaccard similarity. - Subset ratio = |Valkey ∩ Redis| / |Valkey| - Detects when smaller PR cherry-picks from larger PR - Final similarity = max(weighted_jaccard, subset_ratio) Per-file analysis of PR valkey-io#3105 vs #14435: - src/commands.def: 92.9% subset ratio - src/commands/set.json: 93.8% subset ratio - src/t_string.c: 95.5% subset ratio - tests/unit/type/string.tcl: 100% subset ratio Result: - PR valkey-io#3105 now correctly detects Redis #14435 match - PR valkey-io#3088 still works (detects #14243) - PR valkey-io#1 still passes (no false positives) Signed-off-by: Ping Xie <[email protected]>
Contributor
|
Hi Arshid. We are aware of this PR. There are many PRs waiting for review. Do you have a real use case for this feature? We discussed compare-and-set a long time ago in #1215 and we decided to add only IFEQ, which we said is probably enough for 99% of the cases. Later we added it also for delete (DELIFEQ) because there was a real use case for it. Do you have a real use case for IFNE? I mean problem that can't be solved using IFEQ. Just because Redis has it doesn't automatically mean that we want it. We want it only if it solves a real problem. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
IFNEoption toSET, allowing a conditional set when the value differs. Includes tests.