MOD-11172: Refactor Hybrid Parsing by kei-nan · Pull Request #6857 · RediSearch/RediSearch

kei-nan · 2025-09-18T14:47:10Z

This PR aims to refactor the hybrid parsing by using the newly added ArgsParser.
It also aims to separate the hybrid parsing flow from the aggregation or search flows.

A clear and concise description of what the PR is solving, including:

Current: Parsing of HYBRID is accepting keywords it shouldn't from the Aggregate and Search worlds.
Change: Separate the hybrid parsing and use ArgsParser to make our lives a bit easier.
Outcome: Better and more maintainable parsing code which does not depend on the aggregation/search keywords.

Main objects this PR modified

Hybrid Parsing

Mark if applicable

This PR introduces API changes
This PR introduces serialization changes

src/hybrid/parse_hybrid.c

src/hybrid/parse/hybrid_optional_args.h

nafraf · 2025-09-20T16:49:56Z

src/hybrid/parse/hybrid_optional_args.c

+                         ARG_OPT_END);
+
+    // WITHSCORES flag - sets QEXEC_F_SEND_SCORES
+    ArgParser_AddBitflagV(parser, "WITHSCORES", "Include scores in results",


Check this, it seems WITHSCORES is only supported by FT.SEARCH

Yep, we need the scores to be sent in hybrid.
So we want to support this keyword.
Originally was meant to be added in the next PR:
#6858
But the change wound up here.
If you prefer I can move it to the next PR.

src/hybrid/parse/hybrid_combine.c

src/util/arg_parser.c

src/hybrid/parse/hybrid_callbacks.h

src/hybrid/parse/hybrid_optional_args.c

src/hybrid/parse/hybrid_optional_args.h

src/hybrid/parse/hybrid_optional_args.c

codecov · 2025-09-21T22:22:37Z

Codecov Report

❌ Patch coverage is 85.33654% with 61 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.25%. Comparing base (13846c1) to head (94be2ba).
⚠️ Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
src/hybrid/parse/hybrid_callbacks.c	81.62%	43 Missing ⚠️
src/hybrid/parse/hybrid_combine.c	86.86%	13 Missing ⚠️
src/hybrid/parse/hybrid_optional_args.c	92.45%	4 Missing ⚠️
src/util/arg_parser.c	90.90%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6857      +/-   ##
==========================================
+ Coverage   86.24%   86.25%   +0.01%     
==========================================
  Files         304      307       +3     
  Lines       48983    49289     +306     
  Branches     9543     9543              
==========================================
+ Hits        42243    42516     +273     
- Misses       6590     6623      +33     
  Partials      150      150

Flag	Coverage Δ
flow	`84.25% <70.43%> (+0.47%)`	⬆️
unit	`51.74% <77.40%> (+0.09%)`	⬆️

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/hybrid/parse/hybrid_optional_args.h

src/hybrid/parse/hybrid_optional_args.c

src/hybrid/parse/hybrid_combine.c

src/hybrid/parse/hybrid_callbacks.c

JoanFM

Just small comments, feel free to ignore @kei-nan

src/util/arg_parser.c

The base branch was changed.

(RediSearch/RediSearch#7076, RediSearch/RediSearch#6857) - Introducing `FT.HYBRID`, a new command that enables hybrid queries combining both text and vector search, with support for **RRF** and **LINEAR** result combination. This update enhances performance and reliability through a more efficient networking layer, smoother query execution, and improved overall stability. (RediSearch/RediSearch#7065) - Add `search-default-scorer` configuration to set the default text scorer across queries (by default it is BM25). RediSearch/RediSearch#7022 - Handle Atomic Slot Migration events upon moving slots from one node to another in a cluster mode. (RediSearch/RediSearch#6769, RediSearch/RediSearch#6828, RediSearch/RediSearch#6877, RediSearch/RediSearch#6921) - Introduce query time memory guardrails by adding a new `search-on-oom` configuration which defines the query engine behavior when OOM (Out Of Memory) is reached. OOM checks are applied to `FT.SEARCH`, `FT.AGGREGATE`, and `FT.HYBRID` commands. The behavior on OOM can be configured to one of three modes: `IGNORE`, `FAIL`, or `RETURN`. `IGNORE` - The default behavior, run queries anyway (not recommended for heavy queries returning a large result set). `FAIL` - Fail query execution immediately if any of the nodes are in OOM state when query execution starts. `RETURN` - A best effort appraoch to return partial results when OOM is detected in only some of the nodes in a cluster mode.

kei-nan requested review from Itzikvaknin and nafraf September 18, 2025 14:47

kei-nan self-assigned this Sep 18, 2025

github-actions bot added the size:L label Sep 18, 2025

kei-nan force-pushed the rrf_jk_arg_parser branch from a651354 to 60ef493 Compare September 18, 2025 17:59

kei-nan force-pushed the rrf_jk_refactor_hybrid_parsing branch from 0e62e18 to 767f7fa Compare September 18, 2025 18:00

This comment was marked as outdated.

Sign in to view

nafraf reviewed Sep 20, 2025

View reviewed changes

src/hybrid/parse/hybrid_optional_args.c Outdated Show resolved Hide resolved

github-actions bot added the size:XL label Sep 21, 2025

This comment was marked as outdated.

Sign in to view

kei-nan force-pushed the rrf_jk_refactor_hybrid_parsing branch from 717debb to 884102e Compare September 21, 2025 14:02

This comment was marked as outdated.

Sign in to view

kei-nan force-pushed the rrf_jk_refactor_hybrid_parsing branch from 884102e to 75eb31a Compare September 21, 2025 14:07

This comment was marked as outdated.

Sign in to view

kei-nan force-pushed the rrf_jk_refactor_hybrid_parsing branch from 609d6e1 to a782eb6 Compare September 21, 2025 16:15

This comment was marked as outdated.

Sign in to view

kei-nan force-pushed the rrf_jk_refactor_hybrid_parsing branch from a782eb6 to 8964ee9 Compare September 21, 2025 16:35

kei-nan requested a review from nafraf September 21, 2025 18:31

This comment was marked as outdated.

Sign in to view

JoanFM reviewed Sep 22, 2025

View reviewed changes

src/hybrid/parse/hybrid_optional_args.h Show resolved Hide resolved

JoanFM requested changes Sep 22, 2025

View reviewed changes

src/hybrid/parse/hybrid_optional_args.c Outdated Show resolved Hide resolved

src/hybrid/parse/hybrid_combine.c Outdated Show resolved Hide resolved

src/hybrid/parse/hybrid_combine.c Show resolved Hide resolved

JoanFM reviewed Sep 22, 2025

View reviewed changes

src/hybrid/parse/hybrid_callbacks.c Outdated Show resolved Hide resolved

src/hybrid/parse/hybrid_callbacks.c Outdated Show resolved Hide resolved

JoanFM reviewed Sep 22, 2025

View reviewed changes

src/hybrid/parse/hybrid_callbacks.c Show resolved Hide resolved

JoanFM reviewed Sep 22, 2025

View reviewed changes

src/hybrid/parse/hybrid_callbacks.c Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

kei-nan requested a review from JoanFM September 23, 2025 13:14

JoanFM approved these changes Sep 23, 2025

View reviewed changes

src/util/arg_parser.c Show resolved Hide resolved

src/util/arg_parser.c Show resolved Hide resolved

advance ac after finding the right definition

ac021fb

kei-nan added 12 commits September 25, 2025 09:42

fix tests

75e5e68

fix typo

714311a

changing constant arg definition to be a double

d6ce7ad

fix timeout parsing

77ae2bc

remove temp variable

25e9bfa

fix error messages

6903e24

additional error fixes

ed377cb

free params dict on error

5248ef8

avoid double free

334ad2c

code review comments

0b209aa

remove constant range validation

ba682e4

improve error message, try to improve coverage

84659b5

kei-nan force-pushed the rrf_jk_refactor_hybrid_parsing branch from e7078ff to 84659b5 Compare September 25, 2025 06:48

JoanFM previously approved these changes Sep 25, 2025

View reviewed changes

Base automatically changed from rrf_jk_arg_parser to master September 25, 2025 10:49

Merge branch 'master' into rrf_jk_refactor_hybrid_parsing

94be2ba

kei-nan requested a review from JoanFM September 25, 2025 11:19

JoanFM approved these changes Sep 25, 2025

View reviewed changes

nafraf approved these changes Sep 25, 2025

View reviewed changes

kei-nan added this pull request to the merge queue Sep 25, 2025

kei-nan removed this pull request from the merge queue due to a manual request Sep 25, 2025

kei-nan added this pull request to the merge queue Sep 25, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 25, 2025

kei-nan added this pull request to the merge queue Sep 26, 2025

Merged via the queue into master with commit a990a64 Sep 26, 2025
16 checks passed

kei-nan deleted the rrf_jk_refactor_hybrid_parsing branch September 26, 2025 11:07

alonre24 mentioned this pull request Oct 26, 2025

Update RediSearch to 8.4 RC1 (v8.3.90) redis/redis#14467

Merged

Conversation

kei-nan commented Sep 18, 2025

Main objects this PR modified

Mark if applicable

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nafraf Sep 20, 2025

Choose a reason for hiding this comment

Uh oh!

kei-nan Sep 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

JoanFM left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Sep 21, 2025 •

edited

Loading