[Redis 8.10] Add SUNIONCARD and SDIFFCARD set cardinality commands - #3874
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a16f0a9d53
ℹ️ 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".
| * @see SUnionCardArgs#limit(long) | ||
| * @since 7.7 | ||
| */ | ||
| public static SUnionCardArgs limit(long limit) { |
There was a problem hiding this comment.
Document validation on both limit factories
When callers pass a negative value to this factory, it delegates to limit(long) and throws IllegalArgumentException, but the factory Javadoc omits that contractual exception; SDiffCardArgs.Builder.limit(long) has the same omission. Add the matching @throws IllegalArgumentException if {@code limit} is negative. tag to both factory methods so callers see the validation at the entry point they invoke.
AGENTS.md reference: AGENTS.md:L153-L154
Useful? React with 👍 / 👎.
a16f0a9 to
86f7096
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 86f709608d
ℹ️ 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".
ggivo
left a comment
There was a problem hiding this comment.
This introduces List for multi-key commands, while the rest of the API uniformly uses K... keys as the last parameter (sdiff, sintercard, mget, …). Is this an intentional new convention?
86f7096 to
e856324
Compare
ae63108 to
20c3c66
Compare
e856324 to
deb8e80
Compare
Summary
Adds support for the two set-cardinality commands shipping in Redis 8.10:
SUNIONCARD numkeys key [key ...] [APPROX] [LIMIT limit](server PR redis/redis#14893) andSDIFFCARD numkeys key [key ...] [LIMIT limit](redis/redis#15278). They return the cardinality of a set union / difference without materializing or returning the derived set;LIMITcaps the result and allows server-side early termination (LIMIT 0= no limit), andSUNIONCARD APPROXreturns a HyperLogLog-based estimate with fixed, small server-memory overhead for large-set use cases.All behavior was verified against a live 8.10 pre-release server before implementation:
Key Decisions & Assumptions
CompositeArgumentclasses for both commands, passed as the last parameter:SUnionCardArgs(APPROX,LIMIT) andSDiffCardArgs(LIMIT).SDiffCardArgskeeps the diff API future-proof for when the server definesSDIFFCARD APPROXsemantics (today it rejects it with a syntax error, so noapprox()is exposed there).List<K>overloads instead of varargs —sunioncard(K, K),sunioncard(List<K>)and the same pair with a trailing args parameter, mirrored forsdiffcard— 8 methods per interface flavor. The two-key shape covers the dominant call pattern; theListoverload replaces varargs so the options object can sit last without ambiguity and callers avoid generic-varargs array creation. No single-key overload is exposed — a one-key union/difference is justSCARD.SUnionCardArgsalways emitsAPPROXbeforeLIMIT(the canonical order per the design doc), regardless of setter order; each option is emitted at most once.IntegerOutputis reused and no RESP2-specific test overload is needed.READONLYand registered inReadOnlyCommandsfor replica-read routing. In cluster mode they follow standard multi-key single-slot routing (likeSINTER/SDIFF); no fan-out overrides — the command tips forbid splitting/aggregating across shards.Behavioral / Conceptual Changes
@since 7.7, plus the newSUnionCardArgs/SDiffCardArgstypes and theAPPROXCommandKeyword.LIMIT 0is passed through as-is (server semantics: no limit); negative limits are rejected client-side withIllegalArgumentException.ERR unknown commandreply — no client-side version gating, consistent with other command additions.Testing
SUnionCardArgsUnitTests/SDiffCardArgsUnitTests(new): encoded token order incl.APPROX-before-LIMITnormalization,LIMIT 0passthrough, negative-limit rejection.RedisCommandBuilderUnitTests: full wire-level assertions for the builder overloads (numkeysplacement, key encoding, option order) plus empty-keys/null-args validation.SetCommandIntegrationTests: new tests gated with@EnabledOnCommand("SUNIONCARD")/@EnabledOnCommand("SDIFFCARD")covering exact/APPROX/LIMIT/LIMIT 0and missing first/subtrahend keys; they run unchanged through the existing reactive and transactional overload classes (all green against the 8.10 test env, none skipped).ClusterReadOnlyCommandsUnitTestscount bumped for the two new read-only registrations.Notes
.envbump needed.SINTERCARDis (pre-existing) missing fromReadOnlyCommands; left untouched as out of scope.🤖 Generated with Claude Code
Note
Low Risk
Additive API and command encoding only; no changes to existing command behavior or security-sensitive paths.
Overview
Adds client support for Redis 8.10
SUNIONCARDandSDIFFCARD, which return union/difference cardinality without materializing the set.SUnionCardArgssupportsAPPROXandLIMIT;SDiffCardArgssupportsLIMITonly (noapprox()until the server allows it).Each command is exposed as two-key and
List<K>overloads, with optional trailing args objects (eight methods per API surface). Wiring follows existing set commands:RedisCommandBuilder, async/reactive dispatch, sync/async/reactive set interfaces, cluster node-selection APIs, and Kotlin coroutines. Protocol updates registerSDIFFCARD/SUNIONCARD, addAPPROXtoCommandKeyword, and mark both as read-only for replica routing.Client-side validation rejects empty key lists and negative
LIMITvalues;LIMIT 0is sent through as “no limit.” Tests cover args encoding, RESP wire format, integration behind@EnabledOnCommand, and the cluster read-only command count bump.Reviewed by Cursor Bugbot for commit deb8e80. Bugbot is set up for automated code reviews on this repo. Configure here.