-
Notifications
You must be signed in to change notification settings - Fork 24.3k
Add variable key-spec flags to SET IF* and DELEX #14529
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
Conversation
These commands behave as DEL and SET (Remove and Overwrite) when they don't get IF* flags, and require the data when they do run with these flags. Move lookupKey call in DELEX to avoid double lookup
|
Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset. In case there are security findings, they will be communicated to you as a comment inside the PR. Hope you’ll enjoy using Jit. Questions? Comments? Want to learn more? Get in touch with us. |
this seems wrong. why not syntax error? [edit] it seems very explicit and on purpose, but it looks very wrong to me. i took the liberty to fix it, unless you'll instruct me to revert it. |
sundb
left a comment
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
|
i can't find any discussion in #14435 on the order of lookup vs arity check and which error should be returned in that case. but still the test was very explicit. |
|
@oranagra yes, it wasn't discussed and was overlooked. |
There was no specific requirement about this but certainly seems more logical to return arity error. |
|
@minchopaskal Theoretically, syntax errors are the top priority, and a syntax wrong command should not sometimes return 0 but sometimes return a syntax error, which can cause confusion. |
minchopaskal
left a comment
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
|
no, in that case we're good |
This is the General Availability release of Redis 8.4 in Redis Open Source. ### Major changes compared to 8.2 - `DIGEST`, `DELEX`; `SET` extensions - atomic compare-and-set and compare-and-delete for string keys - `MSETEX` - atomically set multiple string keys and update their expiration - `XREADGROUP` - new `CLAIM` option for reading both idle pending and incoming stream entries - `CLUSTER MIGRATION` - atomic slot migration - `CLUSTER SLOT-STATS` - per-slot usage metrics: key count, CPU time, and network I/O - Redis query engine: `FT.HYBRID` - hybrid search and fused scoring - Redis query engine: I/O threading with performance boost for search and query commands (FT.*) - I/O threading: substantial throughput increase (e.g. >30% for caching use cases (10% `SET`, 90% `GET`), 4 cores) - JSON: substantial memory reduction for homogenous arrays (up to 91%) ### Binary distributions - Alpine and Debian Docker images - https://hub.docker.com/_/redis - Install using snap - see https://github.com/redis/redis-snap - Install using brew - see https://github.com/redis/homebrew-redis - Install using RPM - see https://github.com/redis/redis-rpm - Install using Debian APT - see https://github.com/redis/redis-debian ### Operating systems we test Redis 8.4 on - Ubuntu 22.04 (Jammy Jellyfish), 24.04 (Noble Numbat) - Rocky Linux 8.10, 9.5 - AlmaLinux 8.10, 9.5 - Debian 12 (Bookworm), Debian 13 (Trixie) - macOS 13 (Ventura), 14 (Sonoma), 15 (Sequoia) ### Bug fixes (compared to 8.4-RC1) - #14524 `XREADGROUP CLAIM` returns strings instead of integers - #14529 Add variable key-spec flags to SET IF* and DELEX - #P928 Potential memory leak (MOD-11484) - #T1801, #T1805 macOS build failures (MOD-12293) - #J1438 `JSON.NUMINCRBY` - wrong result on integer array with non-integer increment (MOD-12282) - #J1437 Thread safety issue related to ASM and shared strings (MOD-12013) ### Performance and resource utilization improvements (compared to 8.4-RC1) - #14480, #14516 Optimize `XREADGROUP` ### known bugs and limitations - When executing `FT.SEARCH`, `FT.AGGREGATE`, `FT.CURSOR`, `FT.HYBRID`, `TS.MGET`, `TS.MRANGE`, `TS.MREVRANGE` and `TS.QUERYINDEX` while an atomic slot migration process is in progress, the results may be partial or contain duplicates - `FT.PROFILE`, `FT.EXPLAIN` and `FT.EXPLACINCLI` doesn’t contain the `FT.HYBRID` option - Metrics from `FT.HYBRID` command aren’t displayed on `FT.INFO` and `INFO` - Option `EXPLAINSCORE`, `SHARD_K_RATIO`, `YIELD_DISTANCE_AS` and `WITHCURSOR` with `FT.HYBRID` are not available - Post-filtering (after `COMBINE` step) using FILTER is not available - Currently the default response format considers only `key_id` and `score`, this may change for delivering entire document content
These commands behave as DEL and SET (blindly Remove or Overwrite) when they don't get IF* flags, and require the value of the key when they do run with these flags.
Making sure they have the VARIABLE_FLAGS flag, and getKeysProc that can provide the right flags depending on the arguments used. (the plain flags when arguments are unknown are the common denominator ones)
Move lookupKey call in DELEX to avoid double lookup, which also means (some, namely arity) syntax errors are checked (and reported) before checking the existence of the key.