feat(time-series): support multiple aggregators per key in TS.NRANGE/TS.NREVRANGE - #3360
Open
nkaradzhov wants to merge 1 commit into
Open
feat(time-series): support multiple aggregators per key in TS.NRANGE/TS.NREVRANGE#3360nkaradzhov wants to merge 1 commit into
nkaradzhov wants to merge 1 commit into
Conversation
…TS.NREVRANGE RedisTimeSeries#2079 finalized the semantics: one AGGREGATION token per key, each holding one or more comma-separated aggregators (e.g. `avg,max sum`). The merged implementation (redis#3337) only exposed a single aggregator per key. Change `AGGREGATION.types` from a flat `TimeSeriesAggregationTypeList` to `TimeSeriesAggregationTypeGroups` (one group per key); emit each group as a single comma-joined token. The shared `TimeSeriesAggregationTypeList` used by single-key TS.RANGE is left untouched. BREAKING (vs unreleased redis#3337): flat `types: [MAX, MIN]` becomes grouped `types: [[MAX], [MIN]]`. Wire output for single-aggregator usage is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
This pull request updates
TS.NRANGEandTS.NREVRANGEto support multiple aggregators per key, matching the semantics finalized in RedisTimeSeries#2079 (merged Jul 2) and the updated HLD.Background
The originally merged implementation (#3337) exposed a typed API that accepted only one aggregator per key (
AGGREGATION.typeswas a flatTimeSeriesAggregationTypeList, one entry per key, emitted as separate tokens). The server and HLD now define one aggregation token per key, where each token may hold multiple comma-separated aggregators — e.g.AGGREGATION avg,max sum 1000gives key 1 bothavgandmax, key 2sum. The merged wire format was still valid, but the typed API could not express the multi-aggregator case.Changes
NRANGE.ts:AGGREGATION.typeschanges fromTimeSeriesAggregationTypeListto a newTimeSeriesAggregationTypeGroups— an array of per-key groups ([TimeSeriesAggregationTypeList, ...TimeSeriesAggregationTypeList[]]). Each group is emitted as a single comma-joined token (group.join(',')).NREVRANGE.ts: unchanged — reuses NRANGE's parser.TimeSeriesAggregationTypeList(used by single-keyTS.RANGE) is deliberately left untouched.Compatibility
This is a breaking change to the typed API introduced in #3337. Existing callers using the flat form (
types: [MAX, MIN]) must migrate to the grouped form (types: [[MAX], [MIN]]). The generated wire output for single-aggregator-per-key usage is unchanged, so no server-side behavior regresses. Since #3337 is unreleased, this only affects branch/edge consumers.🤖 Generated with Claude Code
Note
Medium Risk
Breaking change to an unreleased typed API; wire format for one aggregator per key is preserved, but misuse of the new nested shape could send invalid AGGREGATION arguments.
Overview
AGGREGATION.typesforTS.NRANGE/TS.NREVRANGEis now an array of per-key aggregator groups (e.g.[[MAX], [MIN]]or[[MIN, MAX], [SUM]]) instead of a flat one-aggregator-per-key list. Each group is serialized as a single comma-joined token on the wire (MIN,MAX,SUM), aligning with RedisTimeSeries#2079.Callers who used the previous flat shape must wrap each aggregator in its own inner array; single-aggregator-per-key wire output is unchanged.
NREVRANGEpicks up the behavior via the sharedNRANGEargument parser. Specs add transform and integration coverage for multi-aggregator-per-key cases.Reviewed by Cursor Bugbot for commit e959ff0. Bugbot is set up for automated code reviews on this repo. Configure here.