Repository navigation
Python: Fix Qdrant null/presence filters on Qdrant server 1.19 - #8982
Zakaria Khchiche (Zakariakhchiche) wants to merge 6 commits into
Conversation
GitHub Actions
Inactive
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
Review effort: Lite
Findings: 2
Open (2)
What changed in this PR
This PR updates Qdrant portable filter translation to preserve “null vs missing vs empty” semantics across Qdrant server versions, specifically addressing behavior changes introduced in Qdrant 1.19.x.
Changes:
- Adjusts null and presence/non-null filter generation to avoid relying on
values_countfor declared scalar fields. - Updates
/ne/not_in/contains_allfilter composition to use the revised presence/non-null behavior. - Adds unit + server tests to pin the generated native conditions and validate behavior around missing keys and
Noneelements in arrays.
| File | Description |
|---|---|
| python/packages/qdrant/tests/qdrant/test_server_filters.py | Adds a server-side regression test covering scalar presence filters and list null handling under Qdrant 1.19.x behaviors. |
| python/packages/qdrant/tests/qdrant/test_filters.py | Adds a unit test that pins the generated native Qdrant conditions for scalar presence/null and list is_null. |
| python/packages/qdrant/agent_framework_qdrant/_vector_store.py | Changes filter compilation: introduces is_empty + composite null condition and revises presence/non-null logic for scalar fields; documents remaining 1.19+ limitation for collection fields. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| (Filter("text", "exists"), [1, 2, 3]), | ||
| (Filter("text", "is_not_null"), [2, 3]), | ||
| (Filter("text", "ne", "a"), [1, 3]), | ||
| (Filter("number", "exists"), [1, 2, 3]), | ||
| (Filter("number", "ne", 1.0), [1, 3]), | ||
| (Filter("integer", "not_in", [1]), [3]), | ||
| (Filter("flag", "is_not_null"), [2, 3]), | ||
| (Filter("tags", "is_null"), [1]), | ||
| (Filter("tags", "contains_all", ["a"]), [2]), |
| for index, payload in enumerate([ | ||
| {}, | ||
| {"body": None, "price": None, "integer": None, "flag": None, "tags": None}, | ||
| {"body": "a", "price": 1.0, "integer": 1, "flag": True, "tags": ["a", None]}, | ||
| {"body": "b", "price": 2.0, "integer": 2, "flag": False, "tags": [None]}, | ||
| ]) |
agent-framework-automation
Bot
added
the
python
GitHub Actions
Inactive
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: No findings
Scope: full PR (3 commit(s)): c6970a647890, 316a92702609, 9eaaeacae1a6
Model: gpt-5.6-sol
Overview
The PR replaces version-sensitive scalar presence checks with explicit is_empty/is_null compositions while retaining the established collection-field behavior. Declared scalar payload validation, fail-closed handling for unsupported operations, native-shape unit coverage, and server integration cases support the changed behavior. No Critical, High, or Medium defect introduced by this PR remained after reconciliation.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
No publishable findings remained after source verification for this scope.
| return models.IsNullCondition(is_null=models.PayloadField(key=name)) | ||
| def _prepare_presence_condition(field: VectorStoreField) -> models.Condition: | ||
| name = field.storage_name or field.name | ||
| if field.type_ in _SCALAR_FIELD_TYPES: |
There was a problem hiding this comment.
Zakaria Khchiche (@Zakariakhchiche) This scalar-only branch leaves supported dict data fields on the old values_count(gte=0) presence path. _prepare_field_payload accepts type_="dict", and exists/is_not_null/ne None apply to all data fields, so on Qdrant 1.19 a point missing a required dict payload can still match and later fail deserialization. Please include dict in the non-array presence path, or otherwise avoid values_count for dict fields, with a regression.
There was a problem hiding this comment.
Good catch, thanks. Fixed in 7489ed6: dict fields now take the same non-array presence path as scalars (is_empty / is_null), so exists, is_null and is_not_null no longer use values_count for them. On Qdrant {} is not empty, so an empty dict still counts as present.
Regressions: a unit test on the generated filter (57a4744) and a server test (f02d652) with payloads {}, {attributes: null}, {attributes: {}} and {attributes: {a: 1}}. Checked against a local Qdrant 1.19.2: the new tests fail without the change (the point with no key matches exists / is_not_null) and pass with it.
GitHub Actions
Inactive
Add test for dict field presence and null conditions in Qdrant.
GitHub Actions
Inactive
Add asynchronous test for server dict presence filtering in Qdrant.
GitHub Actions
Inactive
|
The only failing job is |
|
/review |
There was a problem hiding this comment.
MAF Automated Review — Iteration 2
Result: No findings
Scope: 3 net-new commit(s): 7489ed630cd5, 57a47443b374, f02d65287e9d
Model: gpt-5.6-sol
Overview
This incremental revision extends the Qdrant 1.19-compatible non-array presence path from scalar fields to declared dict fields. Declared-type validation, exact generated-filter unit coverage, and server coverage that distinguishes missing, null, empty-object, and populated-object payloads support the change. No Critical, High, or Medium issue introduced by this review range remained after reconciliation.
Reviewed the supplied incremental change set across correctness, security/reliability, architecture, and failure behavior.
No publishable findings remained after source verification for this scope.
Motivation & Context
Qdrant 1.19 changed two filter behaviors that
QdrantCollectiondepends on:values_counttreats a missing key as 0 values (fix: treat missing field as count 0 for values_count filter qdrant/qdrant#9607).is_nullalso matches arrays that containnull(fix(segment): make IsNullCondition consistent for arrays qdrant/qdrant#10101).On these servers the portable
is_null,is_not_nullandcontains_allfilters return wrong records for list fields that containNone. That data was written by the connector itself, and no error is raised.exists,is_not_null,neandnot_inalso stop excluding points that lack the key, so filteredget/searchcalls that work on 1.18 fail withIntegrationInvalidResponseException. CI pinsqdrant/qdrant:v1.16.2, so the existing server tests do not catch this. Against 1.19.1, 10 of the 39test_server_native_portable_semanticscases fail.Description & Review Guide
_prepare_null_conditionis nowis_null AND is_empty, which matches only an explicitnullon every server version.str/int/float/bool), "present" is nowNOT is_empty OR nulland "non-null" isNOT is_empty. Neither depends onvalues_count.values_countfor presence. From 1.19.0 Qdrant cannot tell[]apart from a missing key, so this is documented onQdrantCollectionas a remaining limitation.None.ne,not_inand null checks match the portable semantics again. Local mode is unchanged.test_server_native_portable_semantics(tags exists,tags is_not_null,tags contains_all []) still fail against 1.19.x, so CI stays on v1.16.2 here. A follow-up could bump the image once that behavior is decided.Related Issue
Fixes #8981
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.