fix: accept any Sequence (not only list) as filter values - #2169
Open
joaquinhuigomez wants to merge 1 commit into
Open
joaquinhuigomez wants to merge 1 commit into
joaquinhuigomez wants to merge 1 commit into
Conversation
`FilterValuesList` is typed as a `Sequence`, so `contains_any(("a", "b"))`
and any other non-list sequence is valid input. Both serialisers gated on
`isinstance(value, list)` instead, so a tuple was dropped from the gRPC
`Filters` message without any error, and the REST path raised a bare
`ValueError: Unknown filter value type: <class 'tuple'>`.
Normalise the value once with `_to_value_list()` and feed that to the
array helpers and to the REST parser. `str`/`bytes` are sequences too but
are single filter values, so they are excluded and keep their current
handling, including the empty-string case.
Fixing the serialisers rather than the builders covers every entry point
at once: `_FilterByProperty.equal()` and friends also accept
`FilterValuesList`, and `_FilterByTime.contains_any()` had the same gap.
`_FilterById.contains_any()` already normalised its `Sequence[UUID]` input.
![orca-security-eu[bot]](https://avatars.githubusercontent.com/in/276244?s=60&v=4){kind=small}
There was a problem hiding this comment.
Orca Security Scan Summary
| Status | Check | Issues by priority | |
|---|---|---|---|
 Passed |
Infrastructure as Code |  0  0  0  0 |
View in Orca |
| Passed |
SAST | 0 0 0 0 |
View in Orca |
| Passed |
Secrets | 0 0 0 0 |
View in Orca |
| Passed |
Vulnerabilities | 0 0 0 0 |
View in Orca |
|
To avoid any confusion in the future about your contribution to Weaviate, we work with a Contributor License Agreement. If you agree, you can simply add a comment to this PR that you agree with the CLA so that we can merge. |
Author
|
I have read and agree to the Weaviate Contributor License Agreement and approve the next steps. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Passing a tuple where a filter expects a list of values produces a filter with no value at all.
Filter.by_property("category").contains_any(("news", "sports"))serializes to a gRPCFiltersmessage carrying the operator and target but novalue_text_array, and the query runs with it — no error, no warning, even withvalidate_arguments=True. The REST path raises a bareValueError: Unknown filter value type: <class 'tuple'>instead of aWeaviateInvalidInputError.FilterValuesListis declared asSequence[...], so a tuple type-checks, and_FilterById.contains_anyalready accepts anySequence— the property and time filters just never normalized. Rather than patch each of the nine_FilterByPropertymethods that takeFilterValues(which would still leave_FilterByTimeexposed), the serializer now converts any non-str/bytesSequenceto a list once, in a small_to_value_listhelper used by both the gRPC and REST paths. The str/bytes exclusion is load-bearing:""is a zero-lengthSequenceandequal("")has to keep working — there is a regression test for it.Tests parametrize the existing filter-to-gRPC cases over list and tuple and assert identical protos, plus the REST path; 38 of them fail on main.
pytest test533 passed; ruff, flake8 and pyright clean.