Skip to content

Commit 547085a

Browse files
Fix filter expression being overridden with blank string (redis#375)
passing a string as filter expression no longer sets the filter to a blank string
1 parent a4cc954 commit 547085a

File tree

3 files changed

+82
-3
lines changed

3 files changed

+82
-3
lines changed

redisvl/query/aggregate.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -210,10 +210,9 @@ def _tokenize_and_escape_query(self, user_query: str) -> str:
210210

211211
def _build_query_string(self) -> str:
212212
"""Build the full query string for text search with optional filtering."""
213+
filter_expression = self._filter_expression
213214
if isinstance(self._filter_expression, FilterExpression):
214215
filter_expression = str(self._filter_expression)
215-
else:
216-
filter_expression = ""
217216

218217
# base KNN query
219218
knn_query = f"KNN {self._num_results} @{self._vector_field} ${self.VECTOR_PARAM} AS {self.DISTANCE_ID}"
@@ -224,3 +223,7 @@ def _build_query_string(self) -> str:
224223
text += f" AND {filter_expression}"
225224

226225
return f"{text})=>[{knn_query}]"
226+
227+
def __str__(self) -> str:
228+
"""Return the string representation of the query."""
229+
return " ".join([str(x) for x in self.build_args()])

tests/unit/test_aggregation_types.py

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,3 +113,80 @@ def test_aggregate_hybrid_query():
113113
vector_field_name,
114114
stopwords=[1, 2, 3],
115115
)
116+
117+
118+
def test_hybrid_query_with_string_filter():
119+
"""Test that HybridQuery correctly includes string filter expressions in query string.
120+
121+
This test ensures that when a string filter expression is passed to HybridQuery,
122+
it's properly included in the generated query string and not set to empty.
123+
Regression test for bug where string filters were being ignored.
124+
"""
125+
text = "search for document 12345"
126+
text_field_name = "description"
127+
vector_field_name = "embedding"
128+
129+
# Test with string filter expression - should include filter in query string
130+
string_filter = "@category:{tech|science|engineering}"
131+
hybrid_query = HybridQuery(
132+
text=text,
133+
text_field_name=text_field_name,
134+
vector=sample_vector,
135+
vector_field_name=vector_field_name,
136+
filter_expression=string_filter,
137+
)
138+
139+
# Check that filter is stored correctly
140+
print("hybrid_query.filter ===", hybrid_query.filter)
141+
assert hybrid_query._filter_expression == string_filter
142+
143+
# Check that the generated query string includes both text search and filter
144+
query_string = str(hybrid_query)
145+
assert f"@{text_field_name}:(search | document | 12345)" in query_string
146+
assert f"AND {string_filter}" in query_string
147+
148+
# Test with FilterExpression - should also work (existing functionality)
149+
filter_expression = Tag("category") == "tech"
150+
hybrid_query_with_filter_expr = HybridQuery(
151+
text=text,
152+
text_field_name=text_field_name,
153+
vector=sample_vector,
154+
vector_field_name=vector_field_name,
155+
filter_expression=filter_expression,
156+
)
157+
158+
# Check that filter is stored correctly
159+
assert hybrid_query_with_filter_expr._filter_expression == filter_expression
160+
161+
# Check that the generated query string includes both text search and filter
162+
query_string_with_filter_expr = str(hybrid_query_with_filter_expr)
163+
assert (
164+
f"@{text_field_name}:(search | document | 12345)"
165+
in query_string_with_filter_expr
166+
)
167+
assert "AND @category:{tech}" in query_string_with_filter_expr
168+
169+
# Test with no filter - should only have text search
170+
hybrid_query_no_filter = HybridQuery(
171+
text=text,
172+
text_field_name=text_field_name,
173+
vector=sample_vector,
174+
vector_field_name=vector_field_name,
175+
)
176+
177+
query_string_no_filter = str(hybrid_query_no_filter)
178+
assert f"@{text_field_name}:(search | document | 12345)" in query_string_no_filter
179+
assert "AND" not in query_string_no_filter
180+
181+
# Test with wildcard filter - should only have text search (no AND clause)
182+
hybrid_query_wildcard = HybridQuery(
183+
text=text,
184+
text_field_name=text_field_name,
185+
vector=sample_vector,
186+
vector_field_name=vector_field_name,
187+
filter_expression="*",
188+
)
189+
190+
query_string_wildcard = str(hybrid_query_wildcard)
191+
assert f"@{text_field_name}:(search | document | 12345)" in query_string_wildcard
192+
assert "AND" not in query_string_wildcard

tests/unit/test_query_types.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,6 @@ def test_text_query_with_string_filter():
305305
query_string = str(text_query)
306306
assert f"@{text_field_name}:(search | document | 12345)" in query_string
307307
assert f"AND {string_filter}" in query_string
308-
assert string_filter in query_string
309308

310309
# Test with FilterExpression - should also work (existing functionality)
311310
filter_expression = Tag("category") == "tech"

0 commit comments

Comments
 (0)