Skip to content

fix: parenthesise filter clauses in vector query strings - #720

Open
vishal-bala wants to merge 1 commit into
fix/contain-filter-values-in-their-clausefrom
fix/708-parenthesise-filter-clauses
Open

fix: parenthesise filter clauses in vector query strings#720
vishal-bala wants to merge 1 commit into
fix/contain-filter-values-in-their-clausefrom
fix/708-parenthesise-filter-clauses

Conversation

@vishal-bala

@vishal-bala vishal-bala commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Motivation

Redis Search has no AND keyword. Intersection is expressed by whitespace between clauses, and | binds more loosely than whitespace under DIALECT 2, so a filter expression used as one operand of an intersection has to be parenthesised. The match-everything wildcard is the exception in both directions: * is valid only as an entire query and never as an operand of an intersection, yet it is the documented spelling for "no filter" in a KNN pre-filter.

Issue #708 reported that filtered full-text queries joined the filter with a literal " AND ", which Redis parsed as an ordinary fieldless search term. #709 corrected the three sites that issue named. Three further sites still generate query strings that Redis either rejects outright or silently mis-parses, and the normalisation logic all of them share is copy-pasted into eight blocks across four files, which is how the defect came to differ between sites in the first place.

Site Previous output Consequence
VectorQuery pre-filter @a:{x} | @b:{y}=>[KNN ...] Syntax error. A bare pre-filter of more than one clause fails on whitespace as well as on |, so the query cannot run. filter_expression="" produced =>[KNN ...], also a syntax error.
VectorRangeQuery (@v:[VECTOR_RANGE ...]=>{...} @a:{x} | @b:{y}) Silently wrong results. The union binds across the intersection, so documents outside the vector range are returned.
MultiVectorQuery (...) (*) Syntax error on every index. The class had no wildcard guard, so an explicit filter_expression="*" emitted the wildcard as an intersection operand.

Changes

Parenthesise the filter clause at the three remaining sites

Each site now wraps the filter in its own parentheses so that a union inside it cannot bind across the intersection, and drops the clause entirely when the filter selects every document.

# VectorQuery._build_query_string
- return f"{filter_expression}=>[{knn_query}]"
+ return f"{prefilter}=>[{knn_query}]"    # "(@a:{x} | @b:{y})=>[KNN ...]", or bare "*"

# VectorRangeQuery._build_query_string
- return f"({base_query}{attr_section} {filter_expression})"
+ return intersect_with_filter(f"{base_query}{attr_section}", self._filter_expression)

# MultiVectorQuery._build_query_string
- if filter_expression:
-     return f"({range_query}) ({filter_expression})"
+ return intersect_with_filter(range_query, self._filter_expression)

VectorQuery is the one site that does not use intersect_with_filter, because a KNN pre-filter is not an intersection: the bare * has to survive there rather than be dropped. The parenthesised pre-filter is the form the Redis documentation gives as canonical, and parenthesising a single-clause pre-filter is inert — FT.EXPLAIN returns an identical plan for @credit_score:{high}=>[KNN ...] and (@credit_score:{high})=>[KNN ...].

VectorRangeQuery also loses the parentheses it used to wrap around the whole query. Those were redundant: the string is always a complete FT.SEARCH argument, never an embedded operand. The parentheses the filter needed were the inner ones.

Collapse the filter normalisation into two helpers

render_filter and intersect_with_filter in redisvl/query/filter.py are now the single place that decides how a filter joins a query. Six call sites route through them, across query.py, aggregate.py, hybrid.py and full_text_query_helper.py. FilterQuery and CountQuery keep their own blocks deliberately: there the filter is the entire query, so * must survive rather than be dropped.

Two details in render_filter are load-bearing rather than defensive. It coerces with str() before comparing against "*", because FilterField.__eq__ is overloaded to build a filter and mutates the receiver in place — comparing an un-narrowed field against "*" returns a truthy FilterExpression and corrupts the caller's object. And it strips whitespace before that comparison, because ( * ) is a syntax error as an operand of an intersection even though it parses as a query on its own.

filter.py is the right home: it imports only redisvl.utils.token_escaper, so there is no cycle, and every call site already imports FilterExpression from it. Neither helper is exported from redisvl/query/__init__.py; both are implementation detail.

Three stored query strings in docs/user_guide/02_complex_filtering.ipynb are also corrected to match the new VectorQuery output. docs/user_guide/12_sql_to_redis_queries.ipynb needs no change: it goes through the external SQL translator, which already emitted a parenthesised pre-filter.

Notes

Filtered MultiVectorQuery output changes shape from (range) (filter) to range (filter). FT.EXPLAIN confirms the two plan identically, because INTERSECT is flat and associative under DIALECT 2, so the reshaping is a no-op. Anyone asserting on the exact string rather than on results will see a difference.

Callers passing a raw-string filter containing a top-level | to VectorRangeQuery will get different, correct results: the union now binds inside the filter rather than across the whole query, so such queries return fewer and correctly-filtered documents. Filters built through Tag, Text, Num and the other field helpers are unaffected, since format_expression already parenthesises composite nodes.

Three edge-case inputs also change output, each from an invalid or actively wrong string to a valid one. A whitespace-only filter previously emitted ( ) and a whitespace-padded wildcard emitted ( * ). A bare un-narrowed FilterField such as Tag("category") previously emitted a negated filter, because the old code evaluated filter_expression != "*" and __ne__ is overloaded in the same way as __eq__; it now contributes no clause. None of the three is a supported input, and the classes that accept them do no type validation, which is worth addressing separately.

One caller-supplied string still defeats the wildcard guard. Passing the literal "(*)" as filter_expression produces ((*)), a syntax error. The guard is not widened to cover it: the input is a programming error, the behaviour predates this change, and it fails loudly rather than silently.

process_results in redisvl/index/index.py masks the VectorRangeQuery defect when the distance is requested. Redis yields no distance for a document matched only through a stray union branch, and any vector-query result missing vector_distance is dropped as a suspected Redis 8.8 expiry race, with a warning that misattributes the cause. The extra documents are therefore invisible with the default return_score=True and visible with return_score=False. The integration test covering this uses return_score=False for that reason. The masking itself is a separate concern and is left alone here.

DIALECT 1 inverts the scope of the ~ optional operator, wrapping the whole intersection — filter included — in OPTIONAL rather than just the text clause. This is unreachable from the affected code paths, because a KNN clause is itself a syntax error under DIALECT 1, so any caller passing dialect=1 to a vector query already fails loudly. Recorded because the behaviour is not documented.

Release Notes

Filter expressions are now parenthesised wherever they are combined with another clause, which fixes three classes of broken query. A VectorQuery whose filter_expression contains more than one clause previously failed with a syntax error and now runs. A MultiVectorQuery given an explicit wildcard filter previously failed with a syntax error and now runs. A VectorRangeQuery given a raw-string filter containing a top-level | previously returned documents outside the vector range, because the union bound across the intersection rather than inside the filter; such queries now return fewer, correctly-filtered results.

The change is backwards-compatible for filters built through Tag, Text, Num, Geo and Timestamp, which were already parenthesised. Two generated query strings change shape without changing meaning: a filtered MultiVectorQuery no longer parenthesises its vector-range clauses as a group, and a filtered VectorRangeQuery no longer wraps the whole query in parentheses. Code asserting on exact query strings rather than on results may need updating.


Note

Medium Risk
Changes core FT.SEARCH query strings for filtered vector/range/multi-vector queries; raw-string filters with | get different (correct) results, and exact-string assertions may need updates, but helper-built filters stay compatible.

Overview
Fixes Redis Search query string generation (#708) so filters are parenthesised when combined with vector, text, or multi-vector clauses, and match-all filters (*, empty, or None) are omitted instead of emitted as invalid intersection operands.

Adds internal helpers render_filter and intersect_with_filter in filter.py and routes VectorQuery, VectorRangeQuery, MultiVectorQuery, TextQuery, hybrid VSIM filters, and FullTextQueryHelper through them. KNN pre-filters now use (@filter)=>[KNN ...] (or bare *=>[KNN ...] when unfiltered); range and text queries intersect as base_clause (filter) so top-level | in raw string filters cannot bind across the vector clause.

VectorRangeQuery with union filters returns correct (fewer) results; MultiVectorQuery with filter_expression="*" no longer produces syntax-error (*). User guide notebook outputs are updated for the new VectorQuery string shape. Unit and integration tests cover pre-filter shapes, wildcard handling, and union-filter regressions.

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

PR #709 fixed the three sites issue #708 named, but three more still
build query strings Redis rejects or mis-parses.

A VectorQuery pre-filter was interpolated bare, so any multi-clause
filter was a hard syntax error -- on whitespace as well as on "|" --
and an empty filter produced "=>[KNN ...]". A VectorRangeQuery filter
carried no parentheses of its own, so a raw-string union bound across
the intersection and returned documents outside the vector range.
MultiVectorQuery had no wildcard guard, so an explicit "*" filter
emitted "(...) (*)", which Redis rejects on every index.

Underneath all three, the "stringify the filter, skip it when falsy or
a wildcard" logic was copy-pasted into eight blocks across four files,
which is why the defect drifted between sites. Collapse it into
render_filter and intersect_with_filter in redisvl/query/filter.py and
route the six intersection call sites through them. FilterQuery and
CountQuery keep their own blocks: there the filter is the whole query,
so "*" must survive rather than be dropped.

render_filter coerces with str() before comparing against "*" because
FilterField.__eq__ is overloaded to build a filter and mutates the
receiver in place, and it strips whitespace first because "( * )" is a
syntax error as an operand of an intersection, even though it is a
valid query on its own.

Filtered MultiVectorQuery output changes shape from "(range) (filter)"
to "range (filter)". FT.EXPLAIN confirms both plan identically.

Three edge-case inputs also change output, each from an invalid or
actively wrong string to a valid one: a whitespace-only filter and a
whitespace-padded wildcard previously emitted "(  )" and "( * )", and
a bare un-narrowed FilterField previously emitted a negated filter
because the old code evaluated `filter_expression != "*"`. The helper
tests pin the mechanism behind all three.
@vishal-bala vishal-bala added the auto:patch Increment the patch version when merged label Sep 3, 2026
@vishal-bala
vishal-bala changed the base branch from main to fix/contain-filter-values-in-their-clause September 3, 2026 14:58
@vishal-bala
vishal-bala marked this pull request as ready for review September 3, 2026 15:32

@limjoobin limjoobin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. This PR parenthesises the filter clause wherever it is combined with another clause, so a union inside the filter cannot bind across the intersection, and omits the clause entirely when the filter matches everything.

seems like there are a few merge conflicts though, a rebase will be needed here once #721 is merged.

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.

2 participants