mirror of
https://github.com/infiniflow/ragflow.git
synced 2026-07-19 22:21:04 +08:00
fix(metadata): Infinity push-down must reject multi-valued-unsafe negative ops (≠, not in) (#16943)
### What problem does this PR solve?
The Elasticsearch and Infinity metadata push-down translators are meant
to be
interchangeable pre-filters that both mirror the in-memory `meta_filter`
fallback — the shared test section is even titled
*"is_pushdown_supported
pre-check (same logic for both backends)"*. But the two
`is_pushdown_supported`
implementations diverge:
- `common/metadata_es_filter.py` defines
`MULTIVALUE_UNSAFE_NEGATIVE_OPS = frozenset({"≠", "not in"})` and
refuses
push-down for those operators.
- `common/metadata_infinity_filter.py` has **no such guard** and pushes
them
down.
**Why the guard exists:** `meta_fields.<key>` can hold a JSON array, and
the
in-memory `meta_filter` matches a document when **any** of its values
satisfies
the predicate (per-value-bucket semantics). A document whose `tag` is
`[a, b]`
therefore still matches `tag ≠ a` — bucket `b` satisfies it. The
Infinity
push-down emits `NOT JSON_CONTAINS(meta_fields, '$.tag', '"a"')`, which
means
*"the array contains no `a` at all"*, so it **silently drops** that
document.
Same divergence for `not in`. The result: `tag ≠ a` / `tag not in (...)`
under-counts results for any document that has the excluded value
alongside
others, but only on the Infinity backend.
This is on a live production path:
`DocMetadataService._filter_doc_ids_by_metadata_infinity`
(`api/db/services/doc_metadata_service.py:948`) calls this exact
`is_pushdown_supported` as the sole gate before building the Infinity
SQL,
mirroring the ES branch at line 880 which uses the guarded ES version.
Reproduction (both real modules, no services needed):
```python
>>> from common import metadata_es_filter as es, metadata_infinity_filter as inf
>>> f = [{"op": "≠", "key": "tag", "value": "a"}]
>>> es.is_pushdown_supported(f), inf.is_pushdown_supported(f)
(False, True) # ES falls back to in-memory (correct); Infinity pushes down (wrong)
```
### Type of change
- [x] Bug Fix (non-breaking change which fixes an issue)
### Fix
Add the same `MULTIVALUE_UNSAFE_NEGATIVE_OPS` set to
`metadata_infinity_filter` and reject those operators in
`is_pushdown_supported`, so a single such filter forces the whole
request to
the in-memory path — the only place the per-bucket semantics are
reproduced.
`not contains` is intentionally still allowed, matching the ES backend
(`all(not contains)` == `not any(contains)`, which the push-down
expresses
correctly on multi-valued fields). The `≠` / `not in` translators
themselves
are unchanged — they remain correct for the in-memory-fallback path;
only the
push-down eligibility gate is fixed.
### Testing
- Confirmed `is_pushdown_supported([{op}])` now returns `False` for `≠`
and
`not in` on **both** backends (previously ES=False, Infinity=True).
- Added `test_pushdown_check_rejects_multivalue_unsafe_negative_ops`,
which
asserts both backends reject these ops. Confirmed red→green: it fails
against
the pre-fix Infinity module, passes after.
- `ruff check` / `ruff format --check` clean.
### Note on overlap
Open PR #16833 also edits
`test/unit_test/common/test_metadata_filter.py`, but
only **appends** at the end of the file (line 640+) and changes
`metadata_es_filter.py` / `metadata_utils.py`, not the Infinity module —
no
overlap with this change.
### Disclosure
AI-assisted (Claude Code): the divergence was surfaced by an AI-assisted
review
pass, but I independently reproduced it against the real modules,
confirmed the
production call path, and verified the fix and test before submitting.
Signed-off-by: Andrew Chen <48723787+chuenchen309@users.noreply.github.com>
Signed-off-by: chuenchen309 <48723787+chuenchen309@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -55,6 +55,20 @@ _RANGE_OPS: Dict[str, str] = {
|
||||
"≤": "<=",
|
||||
}
|
||||
|
||||
# Negative operators whose legacy per-value-bucket semantics cannot be
|
||||
# reproduced by a single push-down predicate over a multi-valued metadata
|
||||
# field. ``meta_fields.<key>`` may hold a JSON array, and the in-memory
|
||||
# ``meta_filter`` matches a doc when ANY of its values satisfies the predicate;
|
||||
# e.g. a doc whose ``tag`` is ``[a, b]`` still matches ``tag ≠ a`` (via bucket
|
||||
# ``b``). ``NOT JSON_CONTAINS(...)`` instead means "the array contains no ``a``
|
||||
# at all", which would silently drop that doc. Without a cheap way to prove a
|
||||
# field is single-valued at query time we refuse push-down for these operators
|
||||
# and let the in-memory fallback handle them. Mirrors the identically named set
|
||||
# in ``metadata_es_filter``. ``not contains`` is intentionally excluded:
|
||||
# ``all(not contains)`` equals ``not any(contains)``, which the push-down
|
||||
# expresses correctly on both single- and multi-valued fields.
|
||||
MULTIVALUE_UNSAFE_NEGATIVE_OPS: frozenset[str] = frozenset({"≠", "not in"})
|
||||
|
||||
|
||||
class MetaFilterTranslator:
|
||||
"""Translate one user filter clause at a time into Infinity SQL filter strings."""
|
||||
@@ -209,6 +223,8 @@ def is_pushdown_supported(filters: Sequence[Dict[str, Any]]) -> bool:
|
||||
op = flt.get("op")
|
||||
if op not in SUPPORTED_OPERATORS:
|
||||
return False
|
||||
if op in MULTIVALUE_UNSAFE_NEGATIVE_OPS:
|
||||
return False
|
||||
if not isinstance(flt.get("key"), str) or not flt.get("key"):
|
||||
return False
|
||||
return True
|
||||
|
||||
@@ -61,6 +61,20 @@ def test_pushdown_check_accepts_not_contains():
|
||||
assert is_pushdown_supported([{"key": "tag", "op": "not contains", "value": "x"}])
|
||||
|
||||
|
||||
@pytest.mark.parametrize("op", ["≠", "not in"])
|
||||
def test_pushdown_check_rejects_multivalue_unsafe_negative_ops(op):
|
||||
# These negative operators cannot be expressed as a single push-down
|
||||
# predicate over a multi-valued metadata field without diverging from the
|
||||
# in-memory meta_filter's per-value-bucket semantics, so both backends must
|
||||
# refuse push-down and fall back to in-memory evaluation. The ES backend
|
||||
# already does this; the Infinity backend must match it.
|
||||
from common.metadata_es_filter import is_pushdown_supported as es_is_pushdown_supported
|
||||
|
||||
flt = [{"key": "tag", "op": op, "value": "a"}]
|
||||
assert not is_pushdown_supported(flt)
|
||||
assert not es_is_pushdown_supported(flt)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Shared: plan_pushdown (same logic for both backends)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user