MOD-13252 Support multiple warnings in reply by meiravgri · Pull Request #7892 · RediSearch/RediSearch

meiravgri · 2025-12-25T09:53:37Z

Problem

Currently, if a query triggers more than one warning (e.g., both timeout and max prefix expansions reached), only one warning is exposed to the client while the rest are silently dropped.

Solution

Modified warning handling to collect and return all relevant warnings from query execution.

Format Changes

FT.PROFILE (⚠️ breaking change)

Per-shard warning field changes from string to array of strings:

Location: res['Profile']['Shards'][i]['Warning']

Before	After
`"Timeout limit was reached"`	`["Timeout limit was reached", "Max prefix expansions limit was reached"]`

FT.SEARCH / FT.AGGREGATE (RESP3)

res['Results']['warning'] was already an array but only ever contained one warning. Now it can contain multiple:

Before	After
`["Timeout limit was reached"]`	`["Timeout limit was reached", "Max prefix expansions limit was reached"]`

Coordinator warnings (cluster mode, RESP3)

The warnings field remains an array, but we now aggregate warnings from all shards instead of just one.

Note

Introduces multi-warning support and unifies warning formatting.

Refactors RESP3 warning emission into _replyWarnings and ensures Results warning accumulates multiple entries
Coordinator (rpnet.c) iterates all warning strings from shards, setting timeout/max-prefix/OOM/indexing-failure/ASM flags; handles empty replies and returns timed out when present
Changes FT.PROFILE Warning from string to array for shards and coordinator; prints multiple warnings or ['None']
Updates tests to expect arrays and multiple warnings; adds get_shards_profile helper in common.py and adjusts RESP2 expectations where profile packs warnings in arrays

^{Written by Cursor Bugbot for commit c996a3c. This will update automatically on new commits. Configure here.}

codecov · 2025-12-25T11:58:02Z

Codecov Report

❌ Patch coverage is 97.14286% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 84.19%. Comparing base (bd8228f) to head (c996a3c).
⚠️ Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
src/aggregate/aggregate_exec.c	95.23%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7892      +/-   ##
==========================================
- Coverage   84.25%   84.19%   -0.06%     
==========================================
  Files         361      361              
  Lines       56295    56301       +6     
  Branches    15720    15720              
==========================================
- Hits        47429    47403      -26     
- Misses       8650     8682      +32     
  Partials      216      216

Flag	Coverage Δ
flow	`85.11% <97.14%> (-0.31%)`	⬇️
unit	`51.99% <0.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

src/aggregate/aggregate_exec.c

lerman25 · 2025-12-25T13:36:35Z

src/coord/rpnet.c

      if (!strcmp(warning_str, QueryError_Strerror(QUERY_ERROR_CODE_TIMED_OUT))) {
        timed_out = true;
-      } else if (!strcmp(warning_str, QUERY_WMAXPREFIXEXPANSIONS)) {
+      }


The else - if from before is still correct though
Maybe it's even better.
But I think a refactor for this code might be the better approach (smarter string compare)

lerman25 · 2025-12-25T13:38:25Z

tests/pytests/test_resp3.py

         n_warnings += 1
  env.assertEqual(n_warnings, 1)

+def test_multiple_warnings():


Can't we test the profile also here?

redisearch-backport-pull-request · 2025-12-25T17:18:56Z

Backport failed for 8.2, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 8.2
git worktree add -d .worktree/backport-7892-to-8.2 origin/8.2
cd .worktree/backport-7892-to-8.2
git switch --create backport-7892-to-8.2
git cherry-pick -x 0b7aee51b64c6ebe760c69a81ecd5e7ab08245c6

redisearch-backport-pull-request · 2025-12-25T17:18:57Z

Backport failed for 8.4, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 8.4
git worktree add -d .worktree/backport-7892-to-8.4 origin/8.4
cd .worktree/backport-7892-to-8.4
git switch --create backport-7892-to-8.4
git cherry-pick -x 0b7aee51b64c6ebe760c69a81ecd5e7ab08245c6

* take code from meiravg_persist_warnings_in_areq * take tests * fix test * review fixes * move test (cherry picked from commit 0b7aee5)

* MOD-13252 Support multiple warnings in reply (#7892) * take code from meiravg_persist_warnings_in_areq * take tests * fix test * review fixes * move test (cherry picked from commit 0b7aee5) * fix if -else

MOD-13252 Support multiple warnings in reply (#7892) * take code from meiravg_persist_warnings_in_areq * take tests * fix test * review fixes * move test (cherry picked from commit 0b7aee5)

* bit map place holder * save profilectx in AREQ introduce ProfileWarnings * return all warnings in coord reply return all warnings in sendchunk resp3 return all warnings as an array in profil;e * align tests to the new format add testProfileMultipleWarnings and testPersistProfileWarning_MaxPrefixExpansions and test_multiple_warnings * more fixes * test fixes * revert multiple warnings code relted changes * fixes * remove tests relarted to #7892 * imp testPersistProfileWarning_MaxPrefixExpansions * remove temporary test * fix mising BG indexing OOM * fix test * move to header * add oom test * asserts * detect timeout when there are multiple warnings in netCursorCallback * fix assert * enhance test_multiple_warnings to test we have exactly 1 cursor read * fix assert * test with oom (remove in the next commit) * fix test - only test with timeout * comment on ProfileWarningType * fix test * remove unused

* bit map place holder * save profilectx in AREQ introduce ProfileWarnings * return all warnings in coord reply return all warnings in sendchunk resp3 return all warnings as an array in profil;e * align tests to the new format add testProfileMultipleWarnings and testPersistProfileWarning_MaxPrefixExpansions and test_multiple_warnings * more fixes * test fixes * revert multiple warnings code relted changes * fixes * remove tests relarted to #7892 * imp testPersistProfileWarning_MaxPrefixExpansions * remove temporary test * fix mising BG indexing OOM * fix test * move to header * add oom test * asserts * detect timeout when there are multiple warnings in netCursorCallback * fix assert * enhance test_multiple_warnings to test we have exactly 1 cursor read * fix assert * test with oom (remove in the next commit) * fix test - only test with timeout * comment on ProfileWarningType * fix test * remove unused (cherry picked from commit 2f47432)

* MOD-12984 Persist query warnings across cursor reads (#7865) * bit map place holder * save profilectx in AREQ introduce ProfileWarnings * return all warnings in coord reply return all warnings in sendchunk resp3 return all warnings as an array in profil;e * align tests to the new format add testProfileMultipleWarnings and testPersistProfileWarning_MaxPrefixExpansions and test_multiple_warnings * more fixes * test fixes * revert multiple warnings code relted changes * fixes * remove tests relarted to #7892 * imp testPersistProfileWarning_MaxPrefixExpansions * remove temporary test * fix mising BG indexing OOM * fix test * move to header * add oom test * asserts * detect timeout when there are multiple warnings in netCursorCallback * fix assert * enhance test_multiple_warnings to test we have exactly 1 cursor read * fix assert * test with oom (remove in the next commit) * fix test - only test with timeout * comment on ProfileWarningType * fix test * remove unused (cherry picked from commit 2f47432) * fix includes and

* MOD-12984 Persist query warnings across cursor reads (#7865) * bit map place holder * save profilectx in AREQ introduce ProfileWarnings * return all warnings in coord reply return all warnings in sendchunk resp3 return all warnings as an array in profil;e * align tests to the new format add testProfileMultipleWarnings and testPersistProfileWarning_MaxPrefixExpansions and test_multiple_warnings * more fixes * test fixes * revert multiple warnings code relted changes * fixes * remove tests relarted to #7892 * imp testPersistProfileWarning_MaxPrefixExpansions * remove temporary test * fix mising BG indexing OOM * fix test * move to header * add oom test * asserts * detect timeout when there are multiple warnings in netCursorCallback * fix assert * enhance test_multiple_warnings to test we have exactly 1 cursor read * fix assert * test with oom (remove in the next commit) * fix test - only test with timeout * comment on ProfileWarningType * fix test * remove unused (cherry picked from commit 2f47432) * push last

* bit map place holder * save profilectx in AREQ introduce ProfileWarnings * return all warnings in coord reply return all warnings in sendchunk resp3 return all warnings as an array in profil;e * align tests to the new format add testProfileMultipleWarnings and testPersistProfileWarning_MaxPrefixExpansions and test_multiple_warnings * more fixes * test fixes * revert multiple warnings code relted changes * fixes * remove tests relarted to #7892 * imp testPersistProfileWarning_MaxPrefixExpansions * remove temporary test * fix mising BG indexing OOM * fix test * move to header * add asm warning in Profile * add oom test * asserts * detect timeout when there are multiple warnings in netCursorCallback * fix assert * enhance test_multiple_warnings to test we have exactly 1 cursor read * fix assert * test with oom (remove in the next commit) * fix test - only test with timeout * comment on ProfileWarningType * fix test * remove unused * add test * fix crash * change test show crash * add test * final fix * fix as per comments * fix adding profile cursor * fix comments from cursor * revert changes related to crash * handle comment from cursor * small refactor --------- Co-authored-by: meiravgri <[email protected]>

take code from meiravg_persist_warnings_in_areq

a90715d

github-actions bot added the size:S label Dec 25, 2025

take tests

e8f4fa6

github-actions bot added the size:M label Dec 25, 2025

meiravgri added a commit that referenced this pull request Dec 25, 2025

remove tests relarted to #7892

28b5864

meiravgri mentioned this pull request Dec 25, 2025

MOD-12984 Persist query warnings across cursor reads #7865

Merged

meiravgri added backport 8.2 backport 8.4 labels Dec 25, 2025

fix test

dbddafa

meiravgri requested a review from lerman25 December 25, 2025 11:10

lerman25 reviewed Dec 25, 2025

View reviewed changes

src/aggregate/aggregate_exec.c Outdated Show resolved Hide resolved

lerman25 reviewed Dec 25, 2025

View reviewed changes

meiravgri added 2 commits December 25, 2025 14:37

review fixes

aeabad9

move test

c996a3c

lerman25 approved these changes Dec 25, 2025

View reviewed changes

meiravgri enabled auto-merge December 25, 2025 14:52

meiravgri added this pull request to the merge queue Dec 25, 2025

Merged via the queue into master with commit 0b7aee5 Dec 25, 2025
32 checks passed

meiravgri deleted the meiravg_support_multiple_warnings branch December 25, 2025 17:18

meiravgri added a commit that referenced this pull request Dec 27, 2025

MOD-13252 Support multiple warnings in reply (#7892)

5ca61e8

* take code from meiravg_persist_warnings_in_areq * take tests * fix test * review fixes * move test (cherry picked from commit 0b7aee5)

meiravgri mentioned this pull request Dec 27, 2025

[8.4] MOD-13252 Support multiple warnings in reply #7900

Merged

meiravgri added a commit that referenced this pull request Dec 28, 2025

MOD-13252 Support multiple warnings in reply (#7892)

341355b

* take code from meiravg_persist_warnings_in_areq * take tests * fix test * review fixes * move test (cherry picked from commit 0b7aee5)

This was referenced Dec 28, 2025

[8.2] MOD-13252 Support multiple warnings in reply #7901

Merged

MOD-13252 Support multiple warnings in reply (small fix) #7902

Merged

meiravgri mentioned this pull request Dec 31, 2025

[8.2] MOD-12984 Persist query warnings across cursor reads #7957

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MOD-13252 Support multiple warnings in reply#7892

MOD-13252 Support multiple warnings in reply#7892
meiravgri merged 5 commits intomasterfrom
meiravg_support_multiple_warnings

meiravgri commented Dec 25, 2025 •

edited by cursor bot

Loading

Uh oh!

codecov bot commented Dec 25, 2025 •

edited

Loading

Uh oh!

Uh oh!

lerman25 Dec 25, 2025

Uh oh!

meiravgri Dec 25, 2025

Uh oh!

lerman25 Dec 25, 2025

Uh oh!

Uh oh!

redisearch-backport-pull-request bot commented Dec 25, 2025

Uh oh!

redisearch-backport-pull-request bot commented Dec 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

meiravgri commented Dec 25, 2025 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Format Changes

FT.PROFILE (⚠️ breaking change)

FT.SEARCH / FT.AGGREGATE (RESP3)

Coordinator warnings (cluster mode, RESP3)

Uh oh!

codecov bot commented Dec 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

lerman25 Dec 25, 2025

Choose a reason for hiding this comment

Uh oh!

meiravgri Dec 25, 2025

Choose a reason for hiding this comment

Uh oh!

lerman25 Dec 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

redisearch-backport-pull-request bot commented Dec 25, 2025

Uh oh!

redisearch-backport-pull-request bot commented Dec 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

meiravgri commented Dec 25, 2025 •

edited by cursor bot

Loading

codecov bot commented Dec 25, 2025 •

edited

Loading