fix(solr): match ints and floats alike in numeric filters

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.

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)

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"):