fix: parenthesise filter clauses in vector query strings - #720
Open
vishal-bala wants to merge 1 commit into
Open
fix: parenthesise filter clauses in vector query strings#720vishal-bala wants to merge 1 commit into
vishal-bala wants to merge 1 commit into
Conversation
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
changed the base branch from
main
to
fix/contain-filter-values-in-their-clause
September 3, 2026 14:58
vishal-bala
marked this pull request as ready for review
September 3, 2026 15:32
limjoobin
approved these changes
Sep 3, 2026
limjoobin
left a comment
Contributor
There was a problem hiding this comment.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Redis Search has no
ANDkeyword. 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.VectorQuerypre-filter@a:{x} | @b:{y}=>[KNN ...]|, so the query cannot run.filter_expression=""produced=>[KNN ...], also a syntax error.VectorRangeQuery(@v:[VECTOR_RANGE ...]=>{...} @a:{x} | @b:{y})MultiVectorQuery(...) (*)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.
VectorQueryis the one site that does not useintersect_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.EXPLAINreturns an identical plan for@credit_score:{high}=>[KNN ...]and(@credit_score:{high})=>[KNN ...].VectorRangeQueryalso loses the parentheses it used to wrap around the whole query. Those were redundant: the string is always a completeFT.SEARCHargument, never an embedded operand. The parentheses the filter needed were the inner ones.Collapse the filter normalisation into two helpers
render_filterandintersect_with_filterinredisvl/query/filter.pyare now the single place that decides how a filter joins a query. Six call sites route through them, acrossquery.py,aggregate.py,hybrid.pyandfull_text_query_helper.py.FilterQueryandCountQuerykeep their own blocks deliberately: there the filter is the entire query, so*must survive rather than be dropped.Two details in
render_filterare load-bearing rather than defensive. It coerces withstr()before comparing against"*", becauseFilterField.__eq__is overloaded to build a filter and mutates the receiver in place — comparing an un-narrowed field against"*"returns a truthyFilterExpressionand 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.pyis the right home: it imports onlyredisvl.utils.token_escaper, so there is no cycle, and every call site already importsFilterExpressionfrom it. Neither helper is exported fromredisvl/query/__init__.py; both are implementation detail.Three stored query strings in
docs/user_guide/02_complex_filtering.ipynbare also corrected to match the newVectorQueryoutput.docs/user_guide/12_sql_to_redis_queries.ipynbneeds no change: it goes through the external SQL translator, which already emitted a parenthesised pre-filter.Notes
Filtered
MultiVectorQueryoutput changes shape from(range) (filter)torange (filter).FT.EXPLAINconfirms the two plan identically, becauseINTERSECTis 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
|toVectorRangeQuerywill 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 throughTag,Text,Numand the other field helpers are unaffected, sinceformat_expressionalready 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-narrowedFilterFieldsuch asTag("category")previously emitted a negated filter, because the old code evaluatedfilter_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
"(*)"asfilter_expressionproduces((*)), 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_resultsinredisvl/index/index.pymasks theVectorRangeQuerydefect when the distance is requested. Redis yields no distance for a document matched only through a stray union branch, and any vector-query result missingvector_distanceis dropped as a suspected Redis 8.8 expiry race, with a warning that misattributes the cause. The extra documents are therefore invisible with the defaultreturn_score=Trueand visible withreturn_score=False. The integration test covering this usesreturn_score=Falsefor 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 — inOPTIONALrather 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 passingdialect=1to 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
VectorQuerywhosefilter_expressioncontains more than one clause previously failed with a syntax error and now runs. AMultiVectorQuerygiven an explicit wildcard filter previously failed with a syntax error and now runs. AVectorRangeQuerygiven 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,GeoandTimestamp, which were already parenthesised. Two generated query strings change shape without changing meaning: a filteredMultiVectorQueryno longer parenthesises its vector-range clauses as a group, and a filteredVectorRangeQueryno 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, orNone) are omitted instead of emitted as invalid intersection operands.Adds internal helpers
render_filterandintersect_with_filterinfilter.pyand routesVectorQuery,VectorRangeQuery,MultiVectorQuery,TextQuery, hybrid VSIM filters, andFullTextQueryHelperthrough them. KNN pre-filters now use(@filter)=>[KNN ...](or bare*=>[KNN ...]when unfiltered); range and text queries intersect asbase_clause (filter)so top-level|in raw string filters cannot bind across the vector clause.VectorRangeQuerywith union filters returns correct (fewer) results;MultiVectorQuerywithfilter_expression="*"no longer produces syntax-error(*). User guide notebook outputs are updated for the newVectorQuerystring 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.