Skip to content

Add COLLECT reducer to FT.AGGREGATE - #524

Merged
mgravell merged 3 commits into
masterfrom
feat/ft-aggregate-collect-reducer
Jul 28, 2026
Merged

Add COLLECT reducer to FT.AGGREGATE#524
mgravell merged 3 commits into
masterfrom
feat/ft-aggregate-collect-reducer

Conversation

@uglide

@uglide uglide commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Introduce the COLLECT reducer for FT.AGGREGATE GROUPBY, which gathers per-document projections within each group and returns them as an array of per-entry maps under the reducer alias, optionally sorted and bounded.

  • Add CollectReducer with Fields(...)/FieldsAll(), SortBy(...)/SortByAsc/ SortByDesc, Limit(...) and As(...), plus the Reducers.Collect() factory. Field and sort names are normalized to a single '@' prefix on the wire, and covers the FIELDS/SORTBY/LIMIT tokens (excluding AS ), matching the RediSearch grammar. Nested COLLECT columns are read through the existing AggregationResult parsing across RESP2 and RESP3.
  • Register the new public API symbols in PublicAPI.Unshipped.txt.

COLLECT is experimental and gated behind search-enable-unstable-features on the server.


Note

Medium Risk
New public aggregation API must emit correct COLLECT wire tokens; behavior depends on an experimental, server-gated RediSearch feature.

Overview
Adds client support for the experimental REDUCE COLLECT reducer on FT.AGGREGATE GROUPBY, so each group can expose an array of per-document field maps under a reducer alias, with optional in-group sort and limit.

A new fluent CollectReducer (via Reducers.Collect()) builds the RediSearch token layout: FIELDS (explicit names or *), optional SORTBY, and LIMIT, with @ normalization and narg counting that matches the COLLECT grammar. The builder rejects mixing FIELDS * with named fields, missing field configuration, negative limits, and any mutation after GroupBy eagerly serializes the reducer. Public surface is registered in PublicAPI.Unshipped.txt.

Coverage includes unit tests that assert exact aggregation args and an integration test (Redis ≥ 8.7) that enables search-enable-unstable-features (all cluster nodes when needed) and checks grouped COLLECT output shape (e.g. limit + sort).

Reviewed by Cursor Bugbot for commit 92e8220. Bugbot is set up for automated code reviews on this repo. Configure here.

uglide added 2 commits July 16, 2026 19:15
Introduce the COLLECT reducer for FT.AGGREGATE GROUPBY, which gathers
per-document projections within each group and returns them as an array
of per-entry maps under the reducer alias, optionally sorted and bounded.

- Add CollectReducer with Fields(...)/FieldsAll(), SortBy(...)/SortByAsc/
  SortByDesc, Limit(...) and As(...), plus the Reducers.Collect() factory.
  Field and sort names are normalized to a single '@' prefix on the wire,
  and <narg> covers the FIELDS/SORTBY/LIMIT tokens (excluding AS <alias>),
  matching the RediSearch grammar. Nested COLLECT columns are read through
  the existing AggregationResult parsing across RESP2 and RESP3.
- Register the new public API symbols in PublicAPI.Unshipped.txt.

COLLECT is experimental and gated behind search-enable-unstable-features
on the server.
/// Configure the reducer fully before attaching it to a <c>GROUPBY</c>: <see cref="AggregationRequest.GroupBy(string, Reducer[])"/>
/// serializes the reducer immediately, so builder calls made after that point cannot reach the wire and throw
/// <see cref="InvalidOperationException"/>.
/// </para>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As after GroupBy silently ignored

Medium Severity

CollectReducer documentation states that builder calls after GroupBy throw, but inherited As does not use EnsureMutable. Calling As after the reducer is serialized updates Alias on the object while the aggregation args list stays unchanged, so results are read under the wrong key without an error.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b630fd1. Configure here.

{
var server = redis.GetServer(endPoint);
if (!server.IsConnected) continue;
server.Execute("CONFIG", "SET", "search-enable-unstable-features", "yes");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if these are "unstable" features, should the new APIs be marked [Experimental] ?

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ 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 92e8220. Configure here.

/// </para>
/// </summary>
/// <seealso cref="Reducers.Collect()"/>
public sealed class CollectReducer : Reducer

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Experimental COLLECT API missing [Experimental] attribute

Medium Severity

The CollectReducer class and Reducers.Collect() factory are described as experimental in both the PR description and the XML doc comments, yet they are not marked with the [Experimental] attribute. The codebase has established infrastructure for this in Experiments.cs (with retired IDs NRS001 and NRS002 and corresponding docs/exp/ pages). Without the attribute, consumers get no compiler warning when depending on an unstable API that may change or break.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 92e8220. Configure here.

@mgravell
mgravell merged commit 62e2d19 into master Jul 28, 2026
40 of 41 checks passed
@mgravell
mgravell deleted the feat/ft-aggregate-collect-reducer branch July 28, 2026 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants