Repository navigation
[Redis 8.10] Add EXCLUDEEMPTY option to TS.MRANGE / TS.MREVRANGE - #4641
Conversation
There was a problem hiding this comment.
π‘ Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cff0fed44e
βΉοΈ About Codex in GitHub
Codex has been enabled to automatically 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 π.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Adds RedisTimeSeries EXCLUDEEMPTY support to Jedisβ TimeSeries multi-range query parameter builder (TSMRangeParams) so callers can omit matching series that have no samples in the requested range for TS.MRANGE / TS.MREVRANGE. This is implemented as an additive, opt-in flag that only affects command argument encoding and is validated against incompatible options.
Changes:
- Add
TSMRangeParams.excludeEmpty()to emit theEXCLUDEEMPTYkeyword immediately beforeFILTER, and include the flag inequals/hashCode. - Add local validation to reject
EXCLUDEEMPTYcombined withGROUPBYin the typed helper API. - Add unit and integration tests covering encoding/order, compatibility with other options, equality semantics, and server behavior (including raw command error propagation).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/main/java/redis/clients/jedis/timeseries/TSMRangeParams.java | Adds excludeEmpty flag, emits EXCLUDEEMPTY before FILTER, validates against GROUPBY, updates equality/hash. |
| src/main/java/redis/clients/jedis/timeseries/TimeSeriesProtocol.java | Adds EXCLUDEEMPTY to TimeSeriesKeyword enum for raw argument encoding. |
| src/test/java/redis/clients/jedis/timeseries/TSMRangeParamsTest.java | Unit tests for argument emission/ordering, absence when unset, composition, equality, and local rejection with GROUPBY. |
| src/test/java/redis/clients/jedis/commands/unified/timeseries/TimeSeriesCommandsTestBase.java | Integration tests verifying TS.MRANGE/TS.MREVRANGE behavior with EXCLUDEEMPTY, including aggregation and raw-command server error propagation. |
π‘ Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thanks, the implementation and test coverage look good. I've opened atakavci#13 against this PR's branch with test-only improvements: the new integration tests adopt the |
Expose a per-test TestKeyRegistry from UnifiedJedisCommandsTestBase so subclasses can generate unique, auto-cleaned keys on the shared test endpoints. Registered keys are deleted after each test; tests that register nothing are unaffected. The new EXCLUDEEMPTY tests adopt it: series keys and the sensor label value are test-unique, so MRANGE label filters cannot match series leaked by other suites.
8.10.0 RC2 reports redis_version 8.9.241; name the constant after the release (V8_10_0_RC2) so version-gated tests read as intent rather than a magic internal build number.
Replace the mutable s/t/u/sensorFilter fields set by a setup helper with an immutable ExcludeEmptyFixture returned to each test, so the values' origin is visible at the call site and the sample layout is documented once, next to the names assertions use.
Label values, group and consumer names need test-unique values but are not Redis keys, so registering them for cleanup pollutes the registry with phantom DELs. name() applies the same expansion as key() without registering; the EXCLUDEEMPTY sensor label uses it.
Summary
Adds support for the RedisTimeSeries
EXCLUDEEMPTYflag toTSMRangeParamsvia a newexcludeEmpty()builder method. When set, matching series that report no samples for thequeried range are omitted from the
TS.MRANGE/TS.MREVRANGEreply instead of beingreturned with an empty samples array.
Key Decisions & Assumptions
FILTER, so itcomposes with existing options (
WITHLABELS,AGGREGATION, etc.). The response parser isunchanged β an empty reply simply carries fewer (or zero) series.
EXCLUDEEMPTYcannot be combined withGROUPBY. The typed helper rejects thiscombination locally with an
IllegalArgumentExceptionbefore sending a command; a rawrequest that combines them is still sent unchanged and left for the server to reject.
@since 8.0.Behavioral / Conceptual Changes
excludeEmpty()now emits theEXCLUDEEMPTYkeyword; without it, behavior is unchanged.equals/hashCodeonTSMRangeParamsnow account for the new flag, so two paramsdiffering only by
excludeEmptyare no longer considered equal.Testing
Unit tests cover flag emission and ordering (once, before
FILTER), composition with otherrange options, absence when unset, equality/hashCode distinction, and local rejection of the
EXCLUDEEMPTY+GROUPBYcombination. Integration tests verify that empty series are omittedfrom
TS.MRANGEandTS.MREVRANGE(order preserved), composition withAGGREGATION, anall-empty result yielding an empty top-level reply, and that a raw
GROUPBYcombinationpropagates the server error.
Note
Low Risk
Additive API and command encoding for an optional flag; behavior is unchanged unless
excludeEmpty()is used, with a clear client-side guard against invalid GROUPBY pairing.Overview
Adds RedisTimeSeries
EXCLUDEEMPTYsupport forTS.MRANGE/TS.MREVRANGEvia a newexcludeEmpty()builder onTSMRangeParams, which emits theEXCLUDEEMPTYkeyword once (beforeFILTER) so matching series with no samples in the queried range are omitted instead of returned with empty sample arrays.EXCLUDEEMPTY+GROUPBYis rejected inaddParamswithIllegalArgumentException;equals/hashCodenow include the flag.TimeSeriesProtocolgains theEXCLUDEEMPTYkeyword.Tests add wire-format and validation coverage, integration scenarios (including aggregation and raw server errors), a
V8_10_0_RC2version constant,TestKeyRegistry.name()for unique label filters, and per-test key cleanup inUnifiedJedisCommandsTestBase.Reviewed by Cursor Bugbot for commit 9bec2de. Bugbot is set up for automated code reviews on this repo. Configure here.