Skip to content

fix: use whitespace intersection for filtered text queries - #709

Open
limjoobin wants to merge 1 commit into
mainfrom
fix/filtered-text-queries-intersection
Open

fix: use whitespace intersection for filtered text queries#709
limjoobin wants to merge 1 commit into
mainfrom
fix/filtered-text-queries-intersection

Conversation

@limjoobin

@limjoobin limjoobin commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Fixes #708.

Summary

When an index is created without including "and" as a stopword, the literal "AND" is included in the full-text query generated by TextQuery, AggregateHybridQuery and HybridQuery. This causes Redis Search to parse AND as an ordinary, fieldless full-text search term, returning no results even when documents satisfy both the text query and the supplied filter.

Fixes

This PR modifies TextQuery, AggregateHybridQuery and HybridQuery to use whitespace instead of "AND" to join full-text queries and filter expressions.

Tests

Added regression tests covering filtered text and aggregate hybrid queries on an index created with stopwords disabled.


Note

Low Risk
Narrow query-string formatting change with broad test coverage; behavior aligns with Redis intersection semantics and fixes incorrect empty results for STOPWORDS 0 indexes.

Overview
Fixes filtered full-text query generation when an index does not treat and as a stopword (e.g. STOPWORDS 0). Previously, TextQuery, hybrid text helpers, and related paths joined the text clause and filter with a literal AND, which Redis Search could index and match as a normal term—often yielding no hits even when text and tag/numeric filters both matched.

TextQuery._build_query_string and FullTextQueryHelper.build_query_string now append the filter with a single space only, relying on Redis’s implicit intersection between clauses. Unit expectations for TextQuery, AggregateHybridQuery, and HybridQuery were updated accordingly, and integration tests cover filtered TextQuery and AggregateHybridQuery on a stopwords-disabled index.

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

@limjoobin
limjoobin requested a review from vishal-bala August 31, 2026 11:19

@vishal-bala vishal-bala 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.

Thanks for this — the diagnosis in #708 is exactly right, and the FT.EXPLAIN output showing and as a required term is what made this easy to act on. The two sites you picked are the two that matter most, and the new STOPWORDS 0 integration tests are the right shape: two documents differing only in the filtered field, so they actually distinguish "filter applied" from "filter ignored".

Two things I'd like to see before this merges. Both are on lines you're already touching, and both leave a real bug live otherwise.

The first is parenthesising the filter, which I've commented on inline. Worth noting your own issue proposed f" ({filter_expression})" — I think the issue had it right and the PR is the weaker version of it.

The second can't be an inline comment because the file isn't in this diff. There's a third AND join site: MultiVectorQuery._build_query_string in redisvl/query/aggregate.py has two of them.

383:  range_query = " AND ".join(range_queries)
390:  return f"({range_query}) AND ({filter_expression})"

Line 383 is the same failure mode you diagnosed, on a STOPWORDS 0 index:

INTERSECT {
  VECTOR { ... @v1 }
  UNION { and, +and(expanded) }
  VECTOR { ... @v2 }
  ...
}

Line 390 is easy to miss — grep '" AND "' doesn't match it, because the literal there is ) AND (. It needs the AND deleted separately; the groups are already parenthesised, so the parenthesisation change won't cover it. Both are one-line edits, and after them grep -rn ' AND ' redisvl/query/ redisvl/utils/ should come back empty, which seems like the right bar for a PR closing #708.

For scope: we have follow-up work queued for the remaining call sites — VectorQuery's KNN pre-filter (where a bare multi-clause pre-filter is a hard syntax error rather than silently wrong), VectorRangeQuery, and a shared helper so the join logic lives in one place instead of four. We'll open that as a downstream PR crediting this one, so please don't take any of it on here.

Comment thread redisvl/query/query.py

if filter_expression and filter_expression != "*":
text += f" AND {filter_expression}"
text += f" {filter_expression}"

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.

Could you wrap this in parentheses — text += f" ({filter_expression})", as your issue proposed?

Whitespace alone fixes the AND term but leaves a second precedence bug live. Redis binds | more loosely than whitespace intersection, so an unparenthesised multi-clause filter escapes the intersection. Unlike the AND bug, this one is not masked by the default stopword list, so it affects every user rather than only STOPWORDS 0 indexes:

FT.CREATE demo ON HASH PREFIX 1 e: SCHEMA text TEXT a TAG b TAG
HSET e:1 text "the quick brown fox" a x b n
HSET e:2 text "nothing here"        a n b y

FT.SEARCH demo '@text:(fox) @a:{x} | @b:{y}'   DIALECT 2   ->  e:1, e:2
FT.SEARCH demo '@text:(fox) (@a:{x} | @b:{y})' DIALECT 2   ->  e:1

e:2 contains neither fox nor a:{x}. FT.EXPLAIN on the first form shows why:

UNION {
  INTERSECT { @text:fox, TAG:@a { x } }
  TAG:@b { y }
}

To be clear, this is pre-existing rather than something you're introducing — on a default-stopwords index the old string parsed to the same plan once AND was dropped. But since the join is being rewritten anyway it's worth closing both at once, and the failure mode is nastier than the one you fixed: documents that match neither clause come back, which is harder to spot than zero results.

Only raw-string filters are affected. FilterExpression.format_expression already parenthesises composites, so (Tag("a") == "x") | (Tag("b") == "y") was never at risk.


if filter_expression and filter_expression != "*":
query += f" AND {filter_expression}"
query += f" {filter_expression}"

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.

Same parenthesisation here — query += f" ({filter_expression})".

The impact is lower than in TextQuery, because the text clause is ~-optional so INTERSECT{~T, X} returns the same document set as X whichever way the union binds. But the query plan is still wrong, and this helper is shared by both AggregateHybridQuery and the native HybridQuery, so I'd rather the two sites stay consistent than have one carry a latent precedence bug.

query = TextQuery(
text="handbook",
text_field_name="text",
filter_expression=Tag("team") == "docs",

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.

This filter is a single clause, so the test passes with or without the parenthesisation fix. Could you add one case with a union filter, so the precedence behaviour is pinned rather than incidental?

Something like filter_expression=(Tag("team") == "docs") | (Tag("team") == "legal") — though note that a FilterExpression union is self-parenthesising, so to exercise the actual bug the filter needs to be a raw string: filter_expression="@team:{docs} | @team:{legal}", with a third document that matches neither the text nor either tag. Without the fix that document comes back; with it, it doesn't.

@vishal-bala vishal-bala added the auto:patch Increment the patch version when merged label Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto:patch Increment the patch version when merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Filtered TextQuery and AggregateHybridQuery treat literal AND as a required search term when stopwords are disabled

2 participants