diff --git a/test/collection/test_filter.py b/test/collection/test_filter.py index f2adff72b..6173c50a0 100644 --- a/test/collection/test_filter.py +++ b/test/collection/test_filter.py @@ -12,7 +12,7 @@ _FilterValue, _Operator, ) -from weaviate.collections.filters import _FilterToGRPC +from weaviate.collections.filters import _FilterToGRPC, _FilterToREST from weaviate.proto.v1 import base_pb2 @@ -103,14 +103,46 @@ def test_filter_lists_one_entry() -> None: assert or_list == f1 -def test_filter_lists_empty() -> None: - with pytest.raises(weaviate.exceptions.WeaviateInvalidInputError): - wvc.query.Filter.all_of([]) +@pytest.mark.parametrize("filters", [[], [None], [None, None]]) +def test_filter_lists_empty(filters: list[None]) -> None: + and_list = wvc.query.Filter.all_of(filters) + + assert _FilterToGRPC.convert(and_list) is None + assert _FilterToREST.convert(and_list) is None with pytest.raises(weaviate.exceptions.WeaviateInvalidInputError): wvc.query.Filter.any_of([]) +def test_filter_lists_ignore_none() -> None: + filter_ = wvc.query.Filter.by_property("test").equal("test") + + assert wvc.query.Filter.all_of([None, filter_, None]) is filter_ + + +@pytest.mark.parametrize("operator", ["and", "or"]) +def test_empty_filter_composition_is_ignored(operator: str) -> None: + empty = wvc.query.Filter.all_of([]) + filter_ = wvc.query.Filter.by_property("test").equal("test") + expected_grpc = _FilterToGRPC.convert(filter_) + expected_rest = _FilterToREST.convert(filter_) + + combinations = [empty & filter_, filter_ & empty] + if operator == "or": + combinations = [empty | filter_, filter_ | empty] + + for combination in combinations: + assert _FilterToGRPC.convert(combination) == expected_grpc + assert _FilterToREST.convert(combination) == expected_rest + + +def test_inverted_empty_filter_is_ignored() -> None: + inverted = ~wvc.query.Filter.all_of([]) + + assert _FilterToGRPC.convert(inverted) is None + assert _FilterToREST.convert(inverted) is None + + def test_filter_bitwise_and_assignment() -> None: f1 = wvc.query.Filter.by_property("test").equal("test") f2 = wvc.query.Filter.by_creation_time().greater_or_equal(datetime.datetime.now()) diff --git a/weaviate/collections/aggregations/base_executor.py b/weaviate/collections/aggregations/base_executor.py index 90cc72089..f6fd9a794 100644 --- a/weaviate/collections/aggregations/base_executor.py +++ b/weaviate/collections/aggregations/base_executor.py @@ -351,8 +351,8 @@ def _base( builder = self._query() if return_metrics is not None: builder = builder.with_fields(" ".join([metric.to_gql() for metric in return_metrics])) - if filters is not None: - builder = builder.with_where(_FilterToREST.convert(filters)) + if filters is not None and (where := _FilterToREST.convert(filters)) is not None: + builder = builder.with_where(where) if total_count: builder = builder.with_meta_count() if self._tenant is not None: diff --git a/weaviate/collections/classes/filters.py b/weaviate/collections/classes/filters.py index 215329d72..874e9416c 100644 --- a/weaviate/collections/classes/filters.py +++ b/weaviate/collections/classes/filters.py @@ -77,6 +77,10 @@ def __invert__(self) -> "_Filters": return _FilterNot(self) +class _FilterNone(_Filters): + """A filter that is omitted when a request is serialized.""" + + class _FilterAnd(_Filters): def __init__(self, filters: List[_Filters]): self.filters: List[_Filters] = filters @@ -650,13 +654,14 @@ def by_property(name: str, length: bool = False) -> _FilterByProperty: return _FilterByProperty(prop=name, length=length, target=None) @staticmethod - def all_of(filters: List[_Filters]) -> _Filters: - """Combine all filters in the input list with an AND operator.""" - if len(filters) == 1: - return filters[0] - elif len(filters) == 0: - raise WeaviateInvalidInputError("Filter.all_of must have at least one filter") - return _FilterAnd(filters) + def all_of(filters: Sequence[Optional[_Filters]]) -> _Filters: + """Combine all non-None filters in the input list with an AND operator.""" + filtered = [filter_ for filter_ in filters if filter_ is not None] + if len(filtered) == 1: + return filtered[0] + elif len(filtered) == 0: + return _FilterNone() + return _FilterAnd(filtered) @staticmethod def any_of(filters: List[_Filters]) -> _Filters: diff --git a/weaviate/collections/filters.py b/weaviate/collections/filters.py index a489c7d55..497ff7790 100644 --- a/weaviate/collections/filters.py +++ b/weaviate/collections/filters.py @@ -6,6 +6,7 @@ FilterValues, _CountRef, _FilterAnd, + _FilterNone, _FilterNot, _FilterOr, _FilterTargets, @@ -27,12 +28,14 @@ def convert(weav_filter: Literal[None]) -> None: ... @overload @staticmethod - def convert(weav_filter: FilterReturn) -> base_pb2.Filters: ... + def convert(weav_filter: FilterReturn) -> Optional[base_pb2.Filters]: ... @staticmethod def convert(weav_filter: Optional[FilterReturn]) -> Optional[base_pb2.Filters]: if weav_filter is None: return None + elif isinstance(weav_filter, _FilterNone): + return None elif isinstance(weav_filter, _FilterValue): return _FilterToGRPC.__value_filter(weav_filter) else: @@ -175,20 +178,24 @@ def __and_or_not_filter(weav_filter: FilterReturn) -> Optional[base_pb2.Filters] or isinstance(weav_filter, _FilterOr) or isinstance(weav_filter, _FilterNot) ) - return base_pb2.Filters( - operator=weav_filter.operator._to_grpc(), - filters=[ - filter_ - for single_filter in weav_filter.filters - if (filter_ := _FilterToGRPC.convert(single_filter)) is not None - ], - ) + filters = [ + filter_ + for single_filter in weav_filter.filters + if (filter_ := _FilterToGRPC.convert(single_filter)) is not None + ] + if len(filters) == 0: + return None + if len(filters) == 1 and not isinstance(weav_filter, _FilterNot): + return filters[0] + return base_pb2.Filters(operator=weav_filter.operator._to_grpc(), filters=filters) class _FilterToREST: @staticmethod - def convert(weav_filter: FilterReturn) -> Dict[str, Any]: - if isinstance(weav_filter, _FilterValue): + def convert(weav_filter: FilterReturn) -> Optional[Dict[str, Any]]: + if isinstance(weav_filter, _FilterNone): + return None + elif isinstance(weav_filter, _FilterValue): return _FilterToREST.__value_filter(weav_filter) else: return _FilterToREST.__and_or_not_filter(weav_filter) @@ -254,17 +261,19 @@ def __parse_filter(value: FilterValues) -> Dict[str, Any]: raise ValueError(f"Unknown filter value type: {type(value)}") @staticmethod - def __and_or_not_filter(weav_filter: FilterReturn) -> Dict[str, Any]: + def __and_or_not_filter(weav_filter: FilterReturn) -> Optional[Dict[str, Any]]: assert ( isinstance(weav_filter, _FilterAnd) or isinstance(weav_filter, _FilterOr) or isinstance(weav_filter, _FilterNot) ) - return { - "operator": weav_filter.operator.value, - "operands": [ - filter_ - for single_filter in weav_filter.filters - if (filter_ := _FilterToREST.convert(single_filter)) is not None - ], - } + filters = [ + filter_ + for single_filter in weav_filter.filters + if (filter_ := _FilterToREST.convert(single_filter)) is not None + ] + if len(filters) == 0: + return None + if len(filters) == 1 and not isinstance(weav_filter, _FilterNot): + return filters[0] + return {"operator": weav_filter.operator.value, "operands": filters} diff --git a/weaviate/collections/grpc/query.py b/weaviate/collections/grpc/query.py index 281da6e2d..55389803e 100644 --- a/weaviate/collections/grpc/query.py +++ b/weaviate/collections/grpc/query.py @@ -575,7 +575,9 @@ def __boost_to_grpc(self, boost: Optional[_Boost]) -> Optional[search_get_pb2.Bo for cond in boost.conditions: grpc_cond = _B.Condition(weight=cond.weight) if cond.filter is not None: - grpc_cond.filter.CopyFrom(_FilterToGRPC.convert(cond.filter)) + grpc_filter = _FilterToGRPC.convert(cond.filter) + if grpc_filter is not None: + grpc_cond.filter.CopyFrom(grpc_filter) elif cond.time_decay is not None: grpc_cond.time_decay.CopyFrom( _B.TimeDecayFunction(