mirror of
https://github.com/infiniflow/ragflow.git
synced 2026-07-17 05:07:23 +08:00
fix(metadata): meta_filter loses case-insensitive matches and leaks state across dict entries (#16956)
## What
`meta_filter()`'s in-memory `filter_out()` helper has two related bugs
in how it coerces `input`/`value` for comparison operators (`=`, `≠`,
`>`, `<`, `≥`, `≤`):
**1. Asymmetric commit on partial `literal_eval` failure.** The original
code:
```python
input = ast.literal_eval(input)
value = ast.literal_eval(value)
```
runs as two separate statements inside one `try`. If the first succeeds
and the second raises, the first assignment already committed — `input`
and `value` end up as different types, and the subsequent `.lower()`
case-folding silently no-ops for whichever side didn't get lowered as a
string. Concretely: metadata cell `"None"` is a valid Python literal
(`ast.literal_eval("None")` → `None`), but a query value `"none"`
(lowercase) is not — so `status = "none"` never matches a cell whose
value is `"None"`, even though the intended semantics are
case-insensitive.
**2. `value` mutated in place, reused across dict entries.**
`filter_out(v2docs, operator, value)` loops over every `(input, docids)`
pair in `v2docs` and coerces `value` inside the loop body without
resetting it — so once one entry's `literal_eval(value)` succeeds and
rebinds `value` to a non-string, every later entry in the same call
compares against that already-coerced leftover instead of the original
filter value.
## Fix
- Commit both `literal_eval` results together via tuple assignment
(`input, value = ast.literal_eval(input), ast.literal_eval(value)`), so
a failure on either side leaves both operands in their pre-coercion form
instead of a mismatched mix.
- Save the original `value` before the loop and reset it at the top of
each iteration, so per-entry coercion never leaks into the next entry.
## Testing
Added 3 tests to
`test/unit_test/common/test_metadata_filter_operators.py` covering both
symptoms (case-insensitive match against a metadata cell that's a Python
keyword literal, both `=` and `≠`; a numeric `>` comparison unaffected
by an earlier dict entry having coerced the query value). Confirmed red
on `common/metadata_utils.py` at HEAD (`git stash` the fix, tests fail
with the exact symptom described above), green after. Full existing
`test_metadata_filter_operators.py` suite (22/22, including the 3 new
tests) passes. `ruff check` and `ruff format --check` clean on both
touched files.
Sandbox note: this environment has no network access to install
`pytest`/`pytest-asyncio`, so tests were run by importing the test
module and invoking each `test_*` function directly (same approach as
prior PRs from this account against this repo, e.g. #16949).
`test_apply_semi_auto_meta_data_filter.py` (the other file exercising
`meta_filter` indirectly through `apply_meta_data_filter`) needs
`pytest-asyncio` + heavier mocking and wasn't run, but it exercises
`apply_meta_data_filter`'s async/LLM-filter-generation path, not
`filter_out`'s coercion logic touched here.
Cross-referenced open PR #16833 (also touches
`common/metadata_utils.py`) — confirmed via `gh pr diff` it only touches
`convert_conditions`/operator-alias normalization and
`apply_meta_data_filter`'s `None`-vs-`["-999"]` sentinel logic, not
`filter_out`'s comparison-coercion code path. No overlap.
---
This PR was drafted with AI assistance (Claude); I reviewed the change,
independently reproduced both symptoms, and take responsibility for it.
Signed-off-by: chuenchen309 <48723787+chuenchen309@users.noreply.github.com>
This commit is contained in:
@@ -39,7 +39,13 @@ def meta_filter(metas: dict, filters: list[dict], logic: str = "and"):
|
||||
|
||||
def filter_out(v2docs, operator, value):
|
||||
ids = []
|
||||
original_value = value
|
||||
for input, docids in v2docs.items():
|
||||
# Reset to the pristine filter value each iteration -- the comparison branch
|
||||
# below reassigns `value` in place (date normalization, literal_eval coercion),
|
||||
# and reusing that mutated value on the next dict entry compares it against
|
||||
# the wrong (already-coerced) type/content instead of the original filter value.
|
||||
value = original_value
|
||||
if operator in ["=", "≠", ">", "<", "≥", "≤"]:
|
||||
# Check if input is in YYYY-MM-DD date format
|
||||
input_str = str(input).strip()
|
||||
@@ -64,8 +70,14 @@ def meta_filter(metas: dict, filters: list[dict], logic: str = "and"):
|
||||
try:
|
||||
if isinstance(input, list):
|
||||
input = input[0]
|
||||
input = ast.literal_eval(input)
|
||||
value = ast.literal_eval(value)
|
||||
except Exception:
|
||||
pass
|
||||
# Commit both literal_eval results together, or neither -- assigning
|
||||
# just one side (e.g. "None" parses but "none" doesn't) would compare
|
||||
# mismatched types after lowercasing and silently break the
|
||||
# case-insensitive match below.
|
||||
try:
|
||||
input, value = ast.literal_eval(input), ast.literal_eval(value)
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
|
||||
@@ -164,3 +164,31 @@ def test_or_logic_still_unions_after_empty_first_condition():
|
||||
]
|
||||
|
||||
assert set(meta_filter(metas, filters, logic="or")) == {"doc2"}
|
||||
|
||||
|
||||
def test_equal_is_case_insensitive_for_python_keyword_literals():
|
||||
# "None" is a metadata cell that happens to be a Python literal keyword; the
|
||||
# query value "none" (lowercase) is not a valid literal on its own. Coercing
|
||||
# one side (input -> None) while leaving the other as the string "none" would
|
||||
# compare mismatched types and silently fail the case-insensitive match.
|
||||
metas = {"status": {"None": ["doc1"], "Active": ["doc2"]}}
|
||||
filters = [{"key": "status", "op": "=", "value": "none"}]
|
||||
|
||||
assert meta_filter(metas, filters) == ["doc1"]
|
||||
|
||||
|
||||
def test_not_equal_is_case_insensitive_for_python_keyword_literals():
|
||||
metas = {"status": {"None": ["doc1"], "Active": ["doc2"]}}
|
||||
filters = [{"key": "status", "op": "≠", "value": "none"}]
|
||||
|
||||
assert meta_filter(metas, filters) == ["doc2"]
|
||||
|
||||
|
||||
def test_greater_than_unaffected_by_prior_dict_entry_coercing_the_query_value():
|
||||
# The query value must be re-read fresh for every metadata entry -- if an
|
||||
# earlier entry coerces it in place (e.g. "5" -> 5), a later entry must not
|
||||
# compare against that already-coerced leftover instead of the original "5".
|
||||
metas = {"score": {"5": ["doc1"], "10": ["doc2"]}}
|
||||
filters = [{"key": "score", "op": ">", "value": "5"}]
|
||||
|
||||
assert meta_filter(metas, filters) == ["doc2"]
|
||||
|
||||
Reference in New Issue
Block a user