From a2d1d4529ea6f4ba3003a968521e944149fdb23b Mon Sep 17 00:00:00 2001 From: Akshit Khandelwal Date: Thu, 29 Oct 2026 21:16:00 +0530 Subject: [PATCH] fix(solr): match ints and floats alike in numeric filters An int and a float of the same metadata key are stored in different fields (meta_l_ and meta_d_), but a filter only queried the field matching the type of its own value. With ratings [3, 4, 4.5, 5.0], `rating >= 4` returned only 4. Numeric equality, range and `in` filters now query both fields. plong rejects a fractional term, so a fractional bound is compared against the int field as the nearest whole number the range admits, inclusively (`> 4.5` becomes `>= 5`). A fractional equality value only targets the float field, since no int can equal it. Fixes #4083 Co-Authored-By: Claude Opus 5.5 --- .../document_stores/solr/filters.py | 49 ++++++++++++- .../solr/tests/test_document_store.py | 34 ++++++++- integrations/solr/tests/test_filters.py | 69 +++++++++++++++---- 3 files changed, 135 insertions(+), 17 deletions(-) diff --git a/integrations/solr/src/haystack_integrations/document_stores/solr/filters.py b/integrations/solr/src/haystack_integrations/document_stores/solr/filters.py index 978c4119e0..cbe2a4dd38 100644 --- a/integrations/solr/src/haystack_integrations/document_stores/solr/filters.py +++ b/integrations/solr/src/haystack_integrations/document_stores/solr/filters.py @@ -4,6 +4,7 @@ """Translation of Haystack filters into Solr filter query (`fq`) clauses.""" +import math from datetime import datetime from typing import Any @@ -72,6 +73,31 @@ def _resolve_field(field: str, value: Any) -> str: return meta_field_name(normalized, type_code_for_value(value)) +def _is_numeric_meta(field: str, value: Any) -> bool: + is_number = isinstance(value, (int, float)) and not isinstance(value, bool) + return is_number and _normalize_field_name(field) not in _SPECIAL_FIELDS + + +def _numeric_terms(field: str, value: float) -> list[tuple[str, float]]: + """ + Pair a number with every Solr field a number under `field` can be stored in. + + An int is written to a `plong` field and a float to a `pdouble` one, so a key whose values mix the two, + as metadata parsed from JSON routinely does, has values in both. `plong` rejects a fractional term, + so only a whole number is also compared there. The field of the value's own type comes first. + """ + normalized = _normalize_field_name(field) + double_term = (meta_field_name(normalized, "d"), value) + if isinstance(value, float) and not value.is_integer(): + return [double_term] + long_term = (meta_field_name(normalized, "l"), int(value)) + return [long_term, double_term] if isinstance(value, int) else [double_term, long_term] + + +def _any_of(clauses: list[str]) -> str: + return clauses[0] if len(clauses) == 1 else "(" + " OR ".join(clauses) + ")" + + def _negate(clause: str) -> str: """ Wrap `clause` in a negation that is valid in every position. @@ -107,6 +133,8 @@ def _equality_clause(field: str, value: Any) -> str: return _negate(_existence_clause(field)) solr_field = _resolve_field(field, value) return "(" + " AND ".join(f"{solr_field}:{_format_value(element)}" for element in value) + ")" + if _is_numeric_meta(field, value): + return _any_of([f"{solr_field}:{_format_value(term)}" for solr_field, term in _numeric_terms(field, value)]) return f"{_resolve_field(field, value)}:{_format_value(value)}" @@ -129,7 +157,22 @@ def _range_clause(field: str, operator: str, value: Any) -> str: msg = f"Filter value can't be of type bool using operator {operator!r}" raise FilterError(msg) - solr_field = _resolve_field(field, value) + if _is_numeric_meta(field, value): + ranges = [(solr_field, operator, term) for solr_field, term in _numeric_terms(field, value)] + if isinstance(value, float) and not value.is_integer() and math.isfinite(value): + # `plong` rejects a fractional bound, so the int field is compared with the nearest whole number the + # range still admits, inclusively: `> 4.5` becomes `>= 5` and `< 4.5` becomes `<= 4`. + long_field = meta_field_name(_normalize_field_name(field), "l") + if operator in (">", ">="): + ranges.append((long_field, ">=", math.ceil(value))) + else: + ranges.append((long_field, "<=", math.floor(value))) + return _any_of([_range_term(*solr_range) for solr_range in ranges]) + + return _range_term(_resolve_field(field, value), operator, value) + + +def _range_term(solr_field: str, operator: str, value: Any) -> str: return f"{solr_field}:{_RANGE_TEMPLATES[operator].format(value=_format_value(value))}" @@ -145,6 +188,10 @@ def _in_clause(field: str, operator: str, value: Any) -> str: for element in value: if element is None: continue + if _is_numeric_meta(field, element): + for solr_field, term in _numeric_terms(field, element): + grouped.setdefault(solr_field, []).append(term) + continue grouped.setdefault(_resolve_field(field, element), []).append(element) if not grouped: # A list of nothing but nulls can only match documents that have no value for the field. diff --git a/integrations/solr/tests/test_document_store.py b/integrations/solr/tests/test_document_store.py index c240924c59..ce844eb8f1 100644 --- a/integrations/solr/tests/test_document_store.py +++ b/integrations/solr/tests/test_document_store.py @@ -8,6 +8,7 @@ import pytest from haystack.dataclasses import Document, SparseEmbedding from haystack.document_stores.errors import DuplicateDocumentError +from haystack.document_stores.in_memory import InMemoryDocumentStore from haystack.document_stores.types import DuplicatePolicy from haystack.testing.document_store import ( CountDocumentsByFilterTest, @@ -519,7 +520,7 @@ def test_unique_value_params(self): params = SolrDocumentStore._unique_values_params("meta_s_a", _FILTERS, "needle") assert params["facet.contains"] == "needle" assert params["facet.contains.ignoreCase"] is True - assert params["fq"] == "meta_l_page:1" + assert params["fq"] == "(meta_l_page:1 OR meta_d_page:1)" assert "facet.contains" not in SolrDocumentStore._unique_values_params("meta_s_a", None, None) def test_no_facets_means_every_field_counts_zero(self): @@ -870,6 +871,37 @@ def test_empty_content_is_not_dropped(self, document_store): document_store.write_documents([Document(id="1", content="")], DuplicatePolicy.OVERWRITE) assert document_store.filter_documents()[0].content == "" + @pytest.mark.parametrize( + ("operator", "value"), + [ + (">=", 4), + (">=", 4.0), + (">", 4.5), + ("<", 4.2), + ("<=", 4.5), + (">=", -1.5), + ("<", -0.5), + ("==", 5), + ("==", 4.0), + ("!=", 4), + ("!=", 4.0), + ("in", [4, 5]), + ("not in", [4.5, 5]), + ], + ) + def test_numeric_filters_match_ints_and_floats_alike(self, document_store, operator, value): + """An int and a float of the same key live in different fields, and a filter must look in both.""" + # 5 and 5.0 are both present so that a fractional bound moved onto the int field is checked at its edge. + ratings = [-1.5, -1, 3, 4, 4.5, 5, 5.0] + documents = [Document(id=str(rating), content="x", meta={"rating": rating}) for rating in ratings] + document_store.write_documents(documents, DuplicatePolicy.OVERWRITE) + reference = InMemoryDocumentStore() + reference.write_documents(documents) + + filters = {"field": "meta.rating", "operator": operator, "value": value} + expected = sorted(document.id for document in reference.filter_documents(filters)) + assert sorted(document.id for document in document_store.filter_documents(filters)) == expected + def test_numeric_looking_strings_stay_strings(self, document_store): """Solr's `parse-long` processor would turn "100" into 100 if it were still active.""" document_store.write_documents([Document(id="1", content="x", meta={"page": "100"})], DuplicatePolicy.OVERWRITE) diff --git a/integrations/solr/tests/test_filters.py b/integrations/solr/tests/test_filters.py index 06acf51066..641031852f 100644 --- a/integrations/solr/tests/test_filters.py +++ b/integrations/solr/tests/test_filters.py @@ -27,15 +27,28 @@ def test_escape_query_chars(self, raw, expected): class TestComparisons: def test_equal_picks_the_field_matching_the_value_type(self): - """The type code comes from the filter value, so "100" and 100 address different fields.""" + """ + The type code comes from the filter value, so "100" and 100 address different fields. + + A number is the exception within its kind: ints and floats of one key are stored in separate fields, so a + whole number is looked up in both, while a fractional one can only be in the float field. + """ assert normalize_filters({"field": "meta.page", "operator": "==", "value": "100"}) == 'meta_s_page:"100"' - assert normalize_filters({"field": "meta.page", "operator": "==", "value": 100}) == "meta_l_page:100" + assert ( + normalize_filters({"field": "meta.page", "operator": "==", "value": 100}) + == "(meta_l_page:100 OR meta_d_page:100)" + ) assert normalize_filters({"field": "meta.page", "operator": "==", "value": 1.5}) == "meta_d_page:1.5" assert normalize_filters({"field": "meta.page", "operator": "==", "value": True}) == "meta_b_page:true" @pytest.mark.parametrize( ("value", "expected"), - [(10, "meta_l_n:10"), (-10, "meta_l_n:\\-10"), (2.5, "meta_d_n:2.5"), (-2.5, "meta_d_n:\\-2.5")], + [ + (10, "(meta_l_n:10 OR meta_d_n:10)"), + (-10, "(meta_l_n:\\-10 OR meta_d_n:\\-10)"), + (2.5, "meta_d_n:2.5"), + (-2.5, "meta_d_n:\\-2.5"), + ], ) def test_negative_numbers_are_escaped(self, value, expected): """ @@ -47,11 +60,11 @@ def test_negative_numbers_are_escaped(self, value, expected): def test_negative_numbers_are_escaped_in_groups(self): clause = normalize_filters({"field": "meta.n", "operator": "in", "value": [-10, 100]}) - assert clause == "(meta_l_n:(\\-10 OR 100))" + assert clause == "(meta_l_n:(\\-10 OR 100) OR meta_d_n:(\\-10 OR 100))" def test_negative_numbers_are_escaped_in_ranges(self): clause = normalize_filters({"field": "meta.n", "operator": ">=", "value": -10}) - assert clause == "meta_l_n:[\\-10 TO *]" + assert clause == "(meta_l_n:[\\-10 TO *] OR meta_d_n:[\\-10 TO *])" def test_meta_prefix_is_optional(self): with_prefix = normalize_filters({"field": "meta.page", "operator": "==", "value": "x"}) @@ -75,22 +88,45 @@ def test_not_equal_none_means_the_key_is_present(self): def test_not_equal_also_matches_documents_missing_the_field(self): """`None != 100` is true in Python, so a Solr negation is the right translation.""" - assert ( - normalize_filters({"field": "meta.number", "operator": "!=", "value": 100}) == "(*:* -(meta_l_number:100))" - ) + clause = normalize_filters({"field": "meta.number", "operator": "!=", "value": 100}) + assert clause == "(*:* -((meta_l_number:100 OR meta_d_number:100)))" @pytest.mark.parametrize( ("operator", "expected"), [ - (">", "meta_l_number:{100 TO *}"), - (">=", "meta_l_number:[100 TO *]"), - ("<", "meta_l_number:[* TO 100}"), - ("<=", "meta_l_number:[* TO 100]"), + (">", "(meta_l_number:{100 TO *} OR meta_d_number:{100 TO *})"), + (">=", "(meta_l_number:[100 TO *] OR meta_d_number:[100 TO *])"), + ("<", "(meta_l_number:[* TO 100} OR meta_d_number:[* TO 100})"), + ("<=", "(meta_l_number:[* TO 100] OR meta_d_number:[* TO 100])"), ], ) def test_ranges(self, operator, expected): assert normalize_filters({"field": "meta.number", "operator": operator, "value": 100}) == expected + @pytest.mark.parametrize( + ("operator", "value", "expected"), + [ + (">", 4.5, "(meta_d_n:{4.5 TO *} OR meta_l_n:[5 TO *])"), + (">=", 4.5, "(meta_d_n:[4.5 TO *] OR meta_l_n:[5 TO *])"), + ("<", 4.5, "(meta_d_n:[* TO 4.5} OR meta_l_n:[* TO 4])"), + ("<=", -0.5, "(meta_d_n:[* TO \\-0.5] OR meta_l_n:[* TO \\-1])"), + ], + ) + def test_fractional_bounds_are_rounded_inclusively_on_the_int_field(self, operator, value, expected): + """`plong` rejects a fractional term, so the int field gets the nearest whole number the range admits.""" + assert normalize_filters({"field": "meta.n", "operator": operator, "value": value}) == expected + + def test_whole_floats_are_compared_on_the_int_field_too(self): + assert normalize_filters({"field": "meta.n", "operator": "==", "value": 5.0}) == "(meta_d_n:5.0 OR meta_l_n:5)" + + def test_infinite_bounds_are_not_rounded(self): + """Rounding infinity raises, so a non-finite bound is left to the float field as before.""" + assert "meta_l_n" not in normalize_filters({"field": "meta.n", "operator": ">=", "value": float("inf")}) + + def test_in_compares_whole_numbers_on_both_fields(self): + clause = normalize_filters({"field": "meta.n", "operator": "in", "value": [4, 4.5]}) + assert clause == "(meta_l_n:(4) OR meta_d_n:(4 OR 4.5))" + def test_iso_dates_compare_lexicographically_on_a_string_field(self): clause = normalize_filters({"field": "meta.date", "operator": ">", "value": "1972-12-11T19:54:58"}) assert clause == 'meta_s_date:{"1972\\-12\\-11T19\\:54\\:58" TO *}' @@ -121,7 +157,7 @@ def test_in_groups_values_by_type(self): def test_not_in_is_a_negated_in(self): clause = normalize_filters({"field": "meta.number", "operator": "not in", "value": [1, 2]}) - assert clause == "(*:* -((meta_l_number:(1 OR 2))))" + assert clause == "(*:* -((meta_l_number:(1 OR 2) OR meta_d_number:(1 OR 2))))" @pytest.mark.parametrize("operator", ["in", "not in"]) def test_non_list_values_are_rejected(self, operator): @@ -169,7 +205,7 @@ def test_and(self): ], } ) - assert clause == '(meta_s_page:"100" AND meta_l_number:{1 TO *})' + assert clause == '(meta_s_page:"100" AND (meta_l_number:{1 TO *} OR meta_d_number:{1 TO *}))' def test_or(self): clause = normalize_filters( @@ -205,7 +241,10 @@ def test_nesting(self): ], } ) - assert clause == '(meta_s_chapter:"intro" AND (meta_l_number:2 OR meta_l_number:\\-10))' + assert clause == ( + '(meta_s_chapter:"intro" AND ' + "((meta_l_number:2 OR meta_d_number:2) OR (meta_l_number:\\-10 OR meta_d_number:\\-10)))" + ) def test_unknown_logical_operator(self): with pytest.raises(FilterError, match="Unknown logical operator"):