Skip to content

Add support for warnings in FT.SEARCH, AGGREGATE and HYBRID#514

Merged
uglide merged 6 commits into
masterfrom
im/search-timeout
Jul 10, 2026
Merged

Add support for warnings in FT.SEARCH, AGGREGATE and HYBRID#514
uglide merged 6 commits into
masterfrom
im/search-timeout

Conversation

@uglide

@uglide uglide commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Note

Medium Risk
Changes public search result shapes and RESP3 reply parsing; incorrect parsing could hide or misreport timeout/partial-result warnings, though behavior is additive (empty warnings when absent).

Overview
Adds public Warnings on SearchResult, AggregationResult, and HybridSearchResult so callers can see RediSearch server messages (e.g. query timeout under the return / return-strict on-timeout policy). RESP3 FT.SEARCH and FT.AGGREGATE replies are parsed via a new ResponseParser.ParseWarnings helper that reads the top-level "warning" map field; FT.HYBRID warning parsing is enabled and exposed (works on RESP2 and RESP3).

Adds AggregationRequest.AddScores() to send the ADDSCORES option and surface @__score in the aggregation pipeline.

Integration tests cover search-on-timeout (fail throws RedisServerException; return yields partial results with timeout text in Warnings) for FT.SEARCH, FT.AGGREGATE, and FT.HYBRID (Redis ≥ 8.9). Shared test helpers: AssertIndexSize and SetSearchOnTimeout for cluster-wide config.

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

@uglide uglide force-pushed the im/search-timeout branch from 8667682 to 536e82e Compare July 10, 2026 13:43
@uglide uglide requested a review from mgravell July 10, 2026 13:43
Comment thread src/NRedisStack/PublicAPI/PublicAPI.Unshipped.txt Outdated
Comment thread src/NRedisStack/Search/AggregationResult.cs Outdated

@mgravell mgravell left a comment

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.

LGTM, but I'm wary of the list allocs that happen on the happy (no warnings to report) path

@uglide uglide merged commit 1447b23 into master Jul 10, 2026
22 checks passed
@uglide uglide deleted the im/search-timeout branch July 10, 2026 15:07
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