fix: keep both filters when FilterPolicy.MERGE combines OR/NOT filters (#12816)

Signed-off-by: Zhang.ZJ <[email protected]>

haystack/document_stores/types/filter_policy.py

@@ -64,11 +64,11 @@ def combine_two_logical_filters(
     init_logical_filter: dict[str, Any], runtime_logical_filter: dict[str, Any]
 ) -> dict[str, Any]:
     """
-    Combine two logical filters, they must have the same operator.
+    Combine two logical filters.
 
-    If `init_logical_filter["operator"]` and `runtime_logical_filter["operator"]` are the same, the conditions
-    of both filters are combined. Otherwise, the `init_logical_filter` is ignored and `
-    runtime_logical_filter` is returned.
+    If both operators are `"AND"`, a single `"AND"` holding the conditions of both is returned. If both use the
+    same other operator, both filters are nested under an `"AND"`. Otherwise, `init_logical_filter` is ignored and
+    `runtime_logical_filter` is returned.
 
         __Example__:
 
@@ -87,9 +87,7 @@ def combine_two_logical_filters(
                 {"field": "meta.publisher", "operator": "==", "value": "nytimes"},
             ]
         }
-        new_filters = combine_two_logical_filters(
-            init_logical_filter, runtime_logical_filter, "AND"
-        )
+        new_filters = combine_two_logical_filters(init_logical_filter, runtime_logical_filter)
         # Output:
         {
             "operator": "AND",
@@ -101,11 +99,35 @@ def combine_two_logical_filters(
             ]
         }
         ```
+
+        With `"OR"` on both sides the filters are nested instead of concatenated:
+
+        ```python
+        init_logical_filter = {
+            "operator": "OR",
+            "conditions": [{"field": "meta.type", "operator": "==", "value": "article"}],
+        }
+        runtime_logical_filter = {
+            "operator": "OR",
+            "conditions": [{"field": "meta.genre", "operator": "==", "value": "economy"}],
+        }
+        new_filters = combine_two_logical_filters(init_logical_filter, runtime_logical_filter)
+        # Output:
+        {"operator": "AND", "conditions": [init_logical_filter, runtime_logical_filter]}
+        ```
     """
     if init_logical_filter["operator"] == runtime_logical_filter["operator"]:
+        if init_logical_filter["operator"] == "AND":
+            return {
+                "operator": "AND",
+                "conditions": init_logical_filter["conditions"] + runtime_logical_filter["conditions"],
+            }
         return {
-            "operator": str(init_logical_filter["operator"]),
-            "conditions": init_logical_filter["conditions"] + runtime_logical_filter["conditions"],
+            "operator": "AND",
+            "conditions": [
+                {**init_logical_filter, "conditions": list(init_logical_filter["conditions"])},
+                {**runtime_logical_filter, "conditions": list(runtime_logical_filter["conditions"])},
+            ],
         }
 
     logger.warning(

releasenotes/notes/filter-policy-merge-or-not-intersection-924563b4bf82d575.yaml

@@ -0,0 +1,12 @@
+---
+upgrade:
+  - |
+    ``FilterPolicy.MERGE`` now intersects initialization and runtime logical filters when both use ``OR`` or
+    ``NOT``. Callers that relied on the previous broader result should construct a single explicit ``OR`` filter
+    containing all alternatives and pass it with ``FilterPolicy.REPLACE`` instead.
+fixes:
+  - |
+    Fixed ``FilterPolicy.MERGE`` returning documents that the initialization filters exclude. When the
+    initialization and runtime filters were both logical filters sharing an ``OR`` or ``NOT`` operator, their
+    conditions were concatenated, which unions the two filters rather than applying both. They are now combined
+    under an ``AND`` so each filter still restricts the result. Merging two ``AND`` filters is unchanged.

test/document_stores/test_filter_policy.py

@@ -6,7 +6,9 @@ from typing import Literal
 
 import pytest
 
+from haystack import Document
 from haystack.document_stores.types import FilterPolicy, apply_filter_policy
+from haystack.utils.filters import document_matches_filter
 
 
 def test_merge_two_comparison_filters():
@@ -227,3 +229,64 @@ def test_merge_with_custom_logical_operator(logical_operator: Literal["AND", "OR
             {"field": "meta.type", "operator": "==", "value": "article"},
         ],
     }
+
+
+@pytest.mark.parametrize("operator", ["OR", "NOT"])
+def test_mutating_merged_non_and_filters_does_not_change_input_filters(operator: Literal["OR", "NOT"]) -> None:
+    init_filters = {"operator": operator, "conditions": [{"field": "meta.type", "operator": "==", "value": "article"}]}
+    runtime_filters = {
+        "operator": operator,
+        "conditions": [{"field": "meta.genre", "operator": "==", "value": "economy"}],
+    }
+
+    merged = apply_filter_policy(FilterPolicy.MERGE, init_filters, runtime_filters)
+    assert merged is not None
+    for logical_filter in merged["conditions"]:
+        logical_filter["operator"] = "AND"
+        logical_filter["conditions"].clear()
+
+    assert init_filters == {
+        "operator": operator,
+        "conditions": [{"field": "meta.type", "operator": "==", "value": "article"}],
+    }
+    assert runtime_filters == {
+        "operator": operator,
+        "conditions": [{"field": "meta.genre", "operator": "==", "value": "economy"}],
+    }
+
+
+@pytest.mark.parametrize("operator", ["OR", "NOT"])
+def test_merge_two_logical_filters_with_non_and_operator_matches_the_intersection(
+    operator: Literal["OR", "NOT"],
+) -> None:
+    """
+    The merged filter selects exactly the documents both input filters select
+    """
+    documents = [
+        Document(id=str(index), meta=meta)
+        for index, meta in enumerate([{"a": 1, "b": 1}, {"a": 1, "b": 9}, {"a": 9, "b": 1}, {"a": 9, "b": 9}])
+    ]
+    init_filters = {
+        "operator": operator,
+        "conditions": [
+            {"field": "meta.a", "operator": "==", "value": 1},
+            {"field": "meta.b", "operator": "==", "value": 1},
+        ],
+    }
+    runtime_filters = {
+        "operator": operator,
+        "conditions": [
+            {"field": "meta.a", "operator": "==", "value": 9},
+            {"field": "meta.b", "operator": "==", "value": 9},
+        ],
+    }
+    merged = apply_filter_policy(FilterPolicy.MERGE, init_filters, runtime_filters)
+    assert merged is not None
+
+    selected = [doc.id for doc in documents if document_matches_filter(merged, doc)]
+    intersection = [
+        doc.id
+        for doc in documents
+        if document_matches_filter(init_filters, doc) and document_matches_filter(runtime_filters, doc)
+    ]
+    assert selected == intersection