mirror of
				https://github.com/deepset-ai/haystack.git
				synced 2025-10-31 09:49:48 +00:00 
			
		
		
		
	Fix corner cases and error handling with filters conversion (#6376)
This commit is contained in:
		
							parent
							
								
									456902235a
								
							
						
					
					
						commit
						76165d024f
					
				| @ -330,6 +330,10 @@ def convert(filters: Dict[str, Any]) -> Dict[str, Any]: | ||||
|     } | ||||
|     ``` | ||||
|     """ | ||||
|     if not isinstance(filters, dict): | ||||
|         msg = f"Can't convert filters from type '{type(filters)}'" | ||||
|         raise ValueError(msg) | ||||
| 
 | ||||
|     converted = _internal_convert(filters) | ||||
|     if "conditions" not in converted: | ||||
|         # This is done to handle a corner case when filter is really simple like so: | ||||
| @ -354,23 +358,35 @@ def _internal_convert(filters: Union[List[Any], Dict[str, Any]], previous_key=No | ||||
|         return _handle_non_dict(filters, previous_key) | ||||
| 
 | ||||
|     for key, value in filters.items(): | ||||
|         if key not in ALL_OPERATORS: | ||||
|         if ( | ||||
|             previous_key is not None | ||||
|             and previous_key not in ALL_LEGACY_OPERATORS_MAPPING | ||||
|             and key not in ALL_LEGACY_OPERATORS_MAPPING | ||||
|         ): | ||||
|             msg = f"This filter ({filters}) seems to be malformed." | ||||
|             raise FilterError(msg) | ||||
|         if key not in ALL_LEGACY_OPERATORS_MAPPING: | ||||
|             converted = _internal_convert(value, previous_key=key) | ||||
|             if isinstance(converted, list): | ||||
|                 conditions.extend(converted) | ||||
|             else: | ||||
|                 conditions.append(converted) | ||||
|         elif key in LOGIC_OPERATORS: | ||||
|             if previous_key not in ALL_OPERATORS and isinstance(value, list): | ||||
|         elif key in LEGACY_LOGICAL_OPERATORS_MAPPING: | ||||
|             if previous_key not in ALL_LEGACY_OPERATORS_MAPPING and isinstance(value, list): | ||||
|                 converted = [_internal_convert({previous_key: v}) for v in value] | ||||
|                 conditions.append({"operator": ALL_OPERATORS[key], "conditions": converted}) | ||||
|                 conditions.append({"operator": ALL_LEGACY_OPERATORS_MAPPING[key], "conditions": converted}) | ||||
|             else: | ||||
|                 converted = _internal_convert(value, previous_key=key) | ||||
|                 if key == "$not" and type(converted) not in [dict, list]: | ||||
|                     # This handles a corner when '$not' is used like this: | ||||
|                     # '{"page": {"$not": 102}}' | ||||
|                     # Without this check we would miss the implicit '$eq' | ||||
|                     converted = {"field": previous_key, "operator": "==", "value": value} | ||||
|                 if not isinstance(converted, list): | ||||
|                     converted = [converted] | ||||
|                 conditions.append({"operator": ALL_OPERATORS[key], "conditions": converted}) | ||||
|         elif key in COMPARISON_OPERATORS: | ||||
|             conditions.append({"field": previous_key, "operator": ALL_OPERATORS[key], "value": value}) | ||||
|                 conditions.append({"operator": ALL_LEGACY_OPERATORS_MAPPING[key], "conditions": converted}) | ||||
|         elif key in LEGACY_COMPARISON_OPERATORS_MAPPING: | ||||
|             conditions.append({"field": previous_key, "operator": ALL_LEGACY_OPERATORS_MAPPING[key], "value": value}) | ||||
| 
 | ||||
|     if len(conditions) == 1: | ||||
|         return conditions[0] | ||||
| @ -382,23 +398,23 @@ def _internal_convert(filters: Union[List[Any], Dict[str, Any]], previous_key=No | ||||
| 
 | ||||
| 
 | ||||
| def _handle_list(filters, previous_key): | ||||
|     if previous_key in LOGIC_OPERATORS: | ||||
|     if previous_key in LEGACY_LOGICAL_OPERATORS_MAPPING: | ||||
|         return [_internal_convert(f) for f in filters] | ||||
|     elif previous_key not in COMPARISON_OPERATORS: | ||||
|     elif previous_key not in LEGACY_COMPARISON_OPERATORS_MAPPING: | ||||
|         return {"field": previous_key, "operator": "in", "value": filters} | ||||
|     return None | ||||
| 
 | ||||
| 
 | ||||
| def _handle_non_dict(filters, previous_key): | ||||
|     if previous_key not in ALL_OPERATORS: | ||||
|     if previous_key not in ALL_LEGACY_OPERATORS_MAPPING: | ||||
|         return {"field": previous_key, "operator": "==", "value": filters} | ||||
|     return filters | ||||
| 
 | ||||
| 
 | ||||
| # Operator mappings from legacy style to new one | ||||
| LOGIC_OPERATORS = {"$and": "AND", "$or": "OR", "$not": "NOT"} | ||||
| LEGACY_LOGICAL_OPERATORS_MAPPING = {"$and": "AND", "$or": "OR", "$not": "NOT"} | ||||
| 
 | ||||
| COMPARISON_OPERATORS = { | ||||
| LEGACY_COMPARISON_OPERATORS_MAPPING = { | ||||
|     "$eq": "==", | ||||
|     "$ne": "!=", | ||||
|     "$gt": ">", | ||||
| @ -409,4 +425,4 @@ COMPARISON_OPERATORS = { | ||||
|     "$nin": "not in", | ||||
| } | ||||
| 
 | ||||
| ALL_OPERATORS = {**LOGIC_OPERATORS, **COMPARISON_OPERATORS} | ||||
| ALL_LEGACY_OPERATORS_MAPPING = {**LEGACY_LOGICAL_OPERATORS_MAPPING, **LEGACY_COMPARISON_OPERATORS_MAPPING} | ||||
|  | ||||
| @ -653,9 +653,27 @@ filters_data = [ | ||||
|         }, | ||||
|         id="Root explicit $not", | ||||
|     ), | ||||
|     pytest.param( | ||||
|         {"page": {"$not": 102}}, | ||||
|         {"operator": "NOT", "conditions": [{"field": "page", "operator": "==", "value": 102}]}, | ||||
|         id="Explicit $not with implicit $eq", | ||||
|     ), | ||||
| ] | ||||
| 
 | ||||
| 
 | ||||
| @pytest.mark.parametrize("old_style, new_style", filters_data) | ||||
| def test_convert(old_style, new_style): | ||||
|     assert convert(old_style) == new_style | ||||
| 
 | ||||
| 
 | ||||
| def test_convert_with_incorrect_input_type(): | ||||
|     with pytest.raises(ValueError): | ||||
|         convert("some string") | ||||
| 
 | ||||
| 
 | ||||
| def test_convert_with_incorrect_filter_nesting(): | ||||
|     with pytest.raises(FilterError): | ||||
|         convert({"number": {"page": "100"}}) | ||||
| 
 | ||||
|     with pytest.raises(FilterError): | ||||
|         convert({"number": {"page": {"chapter": "intro"}}}) | ||||
|  | ||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user
	 Silvano Cerza
						Silvano Cerza