support multi-aggregates in TS.[M][REV]RANGE#469
Conversation
approach: 1. avoid hard breaks at either runtime or build; use overloads with de-prioritization and forwarding plus hiding 2. for inbound: introduce new TsAggregates type that behaves as a "zero, one or many" wrapper, without requiring all callers to create arrays 3. for outbound: create new subclass of TimeSeriesTuple that overrides a new indexer to expose multiple aggregate values Impact: - all code is directly compatible, zero change upgrade - existing callers use same path as now - no new allocs - folks wanting multi-aggregates will alloc some arrays, but these are constrained En-passant: - tidy up the alloc story in TimeStamp
🛡️ Jit Security Scan Results✅ No security findings were detected in this PR
Security scan by Jit
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 677a773. Configure here.
| : TimeSeriesTuple(time, values.Span[0]) | ||
| { | ||
| // this is the main point of this class: to provide access to the other values by overriding the indexer | ||
| public override double this[int index] => values.Span[index]; |
There was a problem hiding this comment.
MultiAggregate tuple equality ignores additional aggregate values
Low Severity
MultiAggregateTimeSeriesTuple inherits Equals and GetHashCode from the base class, which only compare Time and Val (the first aggregate value). Two multi-aggregate tuples with the same timestamp and first value but different subsequent aggregate values will be considered equal. This affects any use of these tuples in hash-based collections, Distinct(), or equality assertions like Assert.Equal on collections.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 677a773. Configure here.


approach:
Impact:
En-passant:
Note
Medium Risk
Touches core TimeSeries command construction and response parsing, so regressions could affect existing range queries; changes are mitigated by keeping legacy overloads and marking new multi-aggregate APIs as experimental.
Overview
Adds experimental Redis 8.8 multi-aggregate support for time-series
RANGE/REVRANGE/MRANGE/MREVRANGE(sync + async) by introducingTsAggregations(zero/one/many wrapper) and new overloads that take it; the previousTsAggregation?overloads are retained but hidden/obsolete and forward into the new APIs.Updates command arg building to serialize multiple aggregations, extends response parsing and
TimeSeriesTuplewith an experimental indexer plusTimeSeriesTuple.Create(...)to expose multiple per-bucket aggregate values, and refactorsTimeStampto reduce allocations for well-known string timestamps. AddsNRS002experimental documentation/suppression wiring and expands tests (plus Docker image pin) to cover multi-aggregate behavior on Redis >= 8.8.Reviewed by Cursor Bugbot for commit 64dddc6. Bugbot is set up for automated code reviews on this repo. Configure here.