Repository navigation
[Redis 8.10] Add SUNIONCARD and SDIFFCARD commands - #4638
Conversation
Add support for the Redis 8.10 set-cardinality commands:
- SUNIONCARD numkeys key [key ...] [APPROX] [LIMIT limit]
- SDIFFCARD numkeys key [key ...] [LIMIT limit]
API shape (options after keys):
- String surface: varargs, List<String>, (key1, key2, params) and
(List, params) overloads
- Binary surface: varargs, (key1, key2, params) and (byte[][], params)
overloads (List<byte[]> is impossible due to erasure clash with
List<String> in UnifiedJedis)
- SUnionCardParams (approx(), limit(long)) emits APPROX before LIMIT;
SDiffCardParams carries only limit(long) since the server rejects
SDIFFCARD APPROX
Covers command interfaces, CommandObjects, UnifiedJedis, PipeliningBase
and legacy Jedis, with params unit tests, mocked delegation tests and
integration tests (standalone, cluster, CommandObjects-level, legacy)
gated on @SinceRedisVersion("8.9.241").
Server PRs: redis/redis#14893, redis/redis#15278
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
uglide
changed the title
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…card # Conflicts: # pom.xml
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20e9698c8b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * Return an approximate cardinality using HyperLogLog (standard error ~0.81%). | ||
| * @return SUnionCardParams | ||
| */ | ||
| public SUnionCardParams approx() { |
There was a problem hiding this comment.
The class-level @since does not annotate the newly introduced public factory and option methods in generated API documentation; sUnionCardParams(), approx(), and limit() are therefore missing the required version metadata, as are the corresponding public methods in SDiffCardParams. Add method-level @since 8.0 tags to these new APIs.
AGENTS.md reference: AGENTS.md:L26-L30
Useful? React with 👍 / 👎.
| RPOPLPUSH, BRPOPLPUSH, BLMOVE, LMOVE, LMPOP, BLMPOP, LPUSHX, RPUSHX, // <-- list | ||
| SADD, SMEMBERS, SREM, SPOP, SMOVE, SCARD, SRANDMEMBER, SINTER, SINTERSTORE, SUNION, SUNIONSTORE, | ||
| SDIFF, SDIFFSTORE, SISMEMBER, SMISMEMBER, SINTERCARD, // <-- set | ||
| SDIFF, SDIFFSTORE, SISMEMBER, SMISMEMBER, SINTERCARD, SUNIONCARD, SDIFFCARD, // <-- set |
There was a problem hiding this comment.
When a client is configured with Jedis's default client-side cache, both new read commands always bypass it because DefaultCacheable.DEFAULT_CACHEABLE_COMMANDS was not extended with SUNIONCARD or SDIFFCARD. CacheConnection.executeCommand() consequently records every invocation as non-cacheable and sends it to Redis, unlike the corresponding SUNION and SDIFF operations; add both protocol commands to the default cacheable set so repeated cardinality reads benefit from caching and key-based invalidation.
Useful? React with 👍 / 👎.
| commandObjects.sunioncard(Arrays.asList(key1, key2), new SUnionCardParams().approx())); | ||
| assertThat(unionCardApprox, equalTo(4L)); | ||
|
|
||
| Long unionCardBinary = exec(commandObjects.sunioncard(key1.getBytes(), key2.getBytes())); |
There was a problem hiding this comment.
When the JVM default charset differs from SafeEncoder.DEFAULT_CHARSET, these binary calls address different Redis keys from the preceding string-key setup: the sadd(String, ...) calls encode the keys through SafeEncoder, while String.getBytes() uses the platform default, so the assertions can receive 0 instead of 4 (and the same problem affects the new SDIFFCARD checks below). Use SafeEncoder.encode(key1) and SafeEncoder.encode(key2) for these binary test keys.
AGENTS.md reference: AGENTS.md:L78-L80
Useful? React with 👍 / 👎.
| import static org.junit.jupiter.api.Assertions.*; | ||
| import static redis.clients.jedis.util.CommandArgumentsMatchers.*; |
There was a problem hiding this comment.
The new test class imports both org.junit.jupiter.api.Assertions.* and CommandArgumentsMatchers.*, and SDiffCardParamsTest repeats the same wildcard imports. Replace them with the specific assertion and matcher imports used by each class so the additions comply with the repository's explicit import convention.
AGENTS.md reference: AGENTS.md:L24-L25
Useful? React with 👍 / 👎.
The aggregate sdiffcard/sunioncard tests mixed all seven public forms per command, so a failure did not identify the API at fault. One test per overload, with missing-key coverage added for the List forms; cluster overrides split to match.
Hardcoded key names on the shared test endpoints can collide across
suites; the registry namespaces them per test and deletes them in
teardown. Cluster overrides hash-tag the per-test prefix so all of a
test's keys share a slot, replacing the literal {.} tags and the
crafted same-slot binary fixture in the new tests.
ggivo
left a comment
ggivo
left a comment
There was a problem hiding this comment.
LGTM
I think we need to address
#4638 (comment)
…card # Conflicts: # src/main/java/redis/clients/jedis/Protocol.java
Adds client support for the Redis 8.10 set-cardinality commands (HLD-based):
SUNIONCARD numkeys key [key ...] [APPROX] [LIMIT limit]— distinct-element count of a union;APPROXestimates via HyperLogLog (~0.81% standard error, fixed server memory),LIMITcaps the result with server-side early termination,LIMIT 0= no limit.SDIFFCARD numkeys key [key ...] [LIMIT limit]— count ofS0 \ (S1 ∪ … ∪ SN).Server PRs: redis/redis#14893 (SUNIONCARD), redis/redis#15278 (SDIFFCARD) — both merged,
since: 8.10.0.Why
Counting the members of a derived union/diff previously required
SUNIONSTORE+SCARD+DEL(a write — not possible on read-only replicas, pollutes the keyspace) or transferring the whole set to the client. These commands are read-only, single-command, and return a plain integer.API
String surface (options after keys):
Binary surface —
List<byte[]>overloads are impossible (type-erasure clash withList<String>sinceUnifiedJedisimplements both interfaces), so the N-keys-with-options form takesbyte[][]:SUnionCardParams:approx(),limit(long); always emitsAPPROXbeforeLIMIT(canonical order), each option at most once.SDiffCardParams:limit(long)only — a separate class makes it impossible to sendSDIFFCARD APPROX, which the server rejects (ERR syntax error) until its semantics are defined.limitis validated non-negative client-side (IllegalArgumentException); all other errors (WRONGTYPE, invalidnumkeys, etc.) propagate from the server unchanged.Layers intentionally not changed
ClusterCommandObjects— no overrides, same assintercard: keys are declared vianumkeys, cluster routing enforces the single-slot constraint and the server returnsCROSSSLOTotherwise.docs// migration guides — purely additive change, no user-facing config or breaking behavior.Behavior against older servers
No client-side version checks: on a pre-8.10 server the command is sent and the server's
unknown commanderror is propagated as-is.Testing
LIMIT 0, negative-limit rejection,equals/hashCodecontract).UnifiedJedisandPipeliningBase.Jedis, gated@SinceRedisVersion("8.9.241")(the 8.10 RC build;8.10-rc2image reportsredis_version:8.9.241).redislabs/client-libs-test:8.10-rc2environment first via redis-cli, including edge cases (missing minuend → 0, missing subtrahend ignored,APPROX+LIMITcombination,WRONGTYPEpropagation).🤖 Generated with Claude Code
Note
Low Risk
Additive API and protocol entries only; behavior mirrors existing
sintercardpatterns with no changes to auth, persistence, or cluster routing beyond new command dispatch.Overview
Adds Redis 8.10 client support for read-only set cardinality commands
SUNIONCARDandSDIFFCARD, so callers can get union/difference sizes without materializing sets or writing temp keys.SUNIONCARDis wired throughCommandObjects,Jedis,UnifiedJedis, and pipeline APIs with string and binary overloads (varargs,List<String>, and params forms).SUnionCardParamsencodesAPPROX(HLL estimate) andLIMITin canonical wire order.SDIFFCARDfollows the same layering withSDiffCardParams(LIMITonly—noAPPROXtype so invalid server syntax cannot be sent).Protocolregisters the new commands and theAPPROXkeyword.Coverage includes param unit tests (wire encoding, validation), mocked delegation for every overload, and integration tests gated to Redis 8.10 RC (standalone, cluster with hash tags, CommandObjects, legacy Jedis).
Reviewed by Cursor Bugbot for commit 2ee876a. Bugbot is set up for automated code reviews on this repo. Configure here.