Skip to content

feat(ft.aggregate): Add Steps for query building#3782

Merged
ndyakov merged 8 commits intomasterfrom
ndyakov/ftaggregate-steps
Apr 24, 2026
Merged

feat(ft.aggregate): Add Steps for query building#3782
ndyakov merged 8 commits intomasterfrom
ndyakov/ftaggregate-steps

Conversation

@ndyakov
Copy link
Copy Markdown
Member

@ndyakov ndyakov commented Apr 20, 2026

Redis's FT.AGGREGATE command allows SORTBY to appear multiple times at any stage of the aggregation pipeline.

This PR also changes the Builder behaviour which is still experimental and should not be considered a breaking change.


Note

Medium Risk
Touches FT.AGGREGATE argument construction and the experimental AggregateBuilder, so mis-ordered/invalid step validation or serialization bugs could change query behavior at runtime.

Overview
Adds a new step-based FT.AGGREGATE pipeline API via FTAggregateOptions.Steps, allowing LOAD, APPLY, GROUPBY, and SORTBY (with per-step MAX) to be repeated and interleaved in arbitrary order.

Updates query argument generation (FTAggregateQuery/FTAggregateWithArgs) to serialize Steps, validates that each step sets exactly one operation, and rejects mixing Steps with the legacy Load/Apply/GroupBy/SortBy/SortByMax fields (which are now marked deprecated).

Reworks the experimental AggregateBuilder to emit ordered Steps, adds builder-side validation (errors returned from Run), and changes SortBy behavior to merge only consecutive SortBy calls into a single SORTBY step. Docs/tests are updated and a new runnable example (example/search-aggregate-steps) is added to demonstrate multi-stage pipelines and shard-level trimming (SORTBY ... MAX before GROUPBY).

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

@ndyakov ndyakov self-assigned this Apr 20, 2026
@jit-ci
Copy link
Copy Markdown

jit-ci Bot commented Apr 20, 2026

❌ Jit Scanner failed - Our team is investigating

Jit Scanner failed - Our team has been notified and is working to resolve the issue. Please contact support if you have any questions.


💡 Need to bypass this check? Comment @sera bypass to override.

Comment thread search_commands.go
@jit-ci
Copy link
Copy Markdown

jit-ci Bot commented Apr 21, 2026

🛡️ Jit Security Scan Results

CRITICAL HIGH MEDIUM

✅ No security findings were detected in this PR


Security scan by Jit

Copy link
Copy Markdown
Contributor

@elena-kolevska elena-kolevska left a comment

Choose a reason for hiding this comment

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

Looks mostly good, with smaller comments. Maybe we can clean up some general repetition.

Also, we need an example in query_agg_test.go.

Comment thread search_commands.go Outdated
Comment thread search_commands.go Outdated
Comment thread search_commands.go
Comment thread search_builders.go
Comment thread search_builders.go Outdated
Copy link
Copy Markdown

@cursor cursor Bot left a comment

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 and found 1 potential issue.

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 a039f69. Configure here.

Comment thread search_builders.go
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a step-based aggregation pipeline API for RediSearch FT.AGGREGATE, enabling multiple LOAD/APPLY/GROUPBY/SORTBY stages in arbitrary order (including multi-stage SORTBY ... MAX trimming), and updates the experimental fluent AggregateBuilder to generate this new pipeline representation.

Changes:

  • Introduces FTAggregateOptions.Steps plus related step types, and updates FTAggregateQuery/FTAggregateWithArgs to emit arguments from ordered steps with new validation.
  • Reworks AggregateBuilder to build Steps and surface builder misuse errors via Run().
  • Updates tests/doctests and adds a runnable example demonstrating multi-stage pipelines.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
search_commands.go Adds step types, Steps option, validation, and step-based argument emission for FT.AGGREGATE.
search_builders.go Updates AggregateBuilder to construct Steps and return early on builder validation errors.
search_test.go Adds unit tests covering step emission order and validation rules.
doctests/query_agg_test.go Migrates documentation examples to the new Steps API.
example/search-aggregate-steps/main.go Adds runnable example showcasing multi-stage FT.AGGREGATE pipelines and builder usage.
example/search-aggregate-steps/go.mod Adds standalone example module definition and local replace to repo root.
example/search-aggregate-steps/go.sum Adds dependency checksums for the example module.
example/search-aggregate-steps/README.md Documents prerequisites and usage for the new example.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread search_commands.go
@ndyakov
Copy link
Copy Markdown
Member Author

ndyakov commented Apr 24, 2026

@ofekshenawa please review and merge if there are no blockers from your side.

Copy link
Copy Markdown
Collaborator

@ofekshenawa ofekshenawa left a comment

Choose a reason for hiding this comment

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

LGTM

@ndyakov ndyakov merged commit 22b26f4 into master Apr 24, 2026
68 of 69 checks passed
@ndyakov ndyakov deleted the ndyakov/ftaggregate-steps branch April 24, 2026 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants