diff --git a/qdrant_client/local/json_path_parser.py b/qdrant_client/local/json_path_parser.py index 6e6f39809..b5fdecccd 100644 --- a/qdrant_client/local/json_path_parser.py +++ b/qdrant_client/local/json_path_parser.py @@ -3,6 +3,9 @@ from pydantic import BaseModel +U64_MAX = 2**64 - 1 + + class JsonPathItemType(str, Enum): KEY = "key" INDEX = "index" @@ -140,11 +143,15 @@ def _match_brackets(path: str) -> tuple[JsonPathItem | None, str]: path[right_bracket_pos + 1 :], ) - try: - index = int(path[left_bracket_pos + 1 : right_bracket_pos]) - return ( - JsonPathItem(item_type=JsonPathItemType.INDEX, index=index), - path[right_bracket_pos + 1 :], - ) - except ValueError as e: - raise ValueError("Invalid path") from e + index_str = path[left_bracket_pos + 1 : right_bracket_pos] + if not (index_str.isascii() and index_str.isdigit()): + raise ValueError("Invalid path") + + index = int(index_str) + if index > U64_MAX: + raise ValueError("Invalid path") + + return ( + JsonPathItem(item_type=JsonPathItemType.INDEX, index=index), + path[right_bracket_pos + 1 :], + ) diff --git a/qdrant_client/local/local_collection.py b/qdrant_client/local/local_collection.py index e6a822d6f..1e89b23d4 100644 --- a/qdrant_client/local/local_collection.py +++ b/qdrant_client/local/local_collection.py @@ -65,7 +65,7 @@ validate_filter, ) from qdrant_client.local.payload_value_extractor import value_by_key, parse_uuid -from qdrant_client.local.payload_value_setter import set_value_by_key +from qdrant_client.local.payload_value_setter import delete_value_by_key, set_value_by_key from qdrant_client.local.persistence import CollectionPersistence from qdrant_client.local.utils import last_argmax, swap_remove from qdrant_client.local.sparse import ( @@ -2933,12 +2933,12 @@ def delete_payload( | models.PointIdsList ), ) -> None: + parsed_keys = [parse_json_path(key) for key in keys] ids = self._selector_to_ids(selector) for point_id in ids: idx = self.ids[point_id] - for key in keys: - if key in self.payload[idx]: - self.payload[idx].pop(key) + for parsed_key in parsed_keys: + delete_value_by_key(self.payload[idx], parsed_key) self._persist_by_id(point_id) def clear_payload( diff --git a/qdrant_client/local/payload_value_setter.py b/qdrant_client/local/payload_value_setter.py index c40795756..8bea58d1d 100644 --- a/qdrant_client/local/payload_value_setter.py +++ b/qdrant_client/local/payload_value_setter.py @@ -28,6 +28,60 @@ def set_value_by_key(payload: dict, keys: list[JsonPathItem], value: Any) -> Non Setter.set(payload, keys.copy(), value, None, None) +def delete_value_by_key(payload: dict, keys: list[JsonPathItem]) -> None: + """ + Delete value in payload by key path, matching the server's payload-delete + semantics (nested keys via dot notation, array traversal by index and + wildcards). + + A key path that does not resolve to an existing value is a no-op, and + sibling values are preserved. As on the server, a path ending in an array + index is a no-op (deleting a single element by index is not idempotent); + only a trailing wildcard (``arr[]``) clears an array. This mirrors the + json-path handling that ``set_value_by_key`` and ``value_by_key`` already + use, so ``delete_payload`` honors the same paths as ``set_payload`` and + filters. + + Args: + payload: arbitrary json-like object + keys: list of json path items, e.g. the parse of "address.city", + "location[0].name" or "location[].name" + """ + + def _delete(data: Any, k_list: list[JsonPathItem]) -> None: + if not k_list: + return + + current_key = k_list.pop(0) + + if len(k_list) == 0: + if isinstance(data, dict) and current_key.item_type == JsonPathItemType.KEY: + data.pop(current_key.key, None) + elif ( + isinstance(data, list) and current_key.item_type == JsonPathItemType.WILDCARD_INDEX + ): + # A wildcard clears the whole array. A terminal array index is + # intentionally not handled: the server does not delete a single + # element by index (it is not idempotent), so it is a no-op here + # to keep local and server behavior identical. + data.clear() + return + + if current_key.item_type == JsonPathItemType.KEY: + if isinstance(data, dict) and current_key.key in data: + _delete(data[current_key.key], k_list.copy()) + elif current_key.item_type == JsonPathItemType.INDEX: + assert current_key.index is not None + if isinstance(data, list) and current_key.index < len(data): + _delete(data[current_key.index], k_list.copy()) + elif current_key.item_type == JsonPathItemType.WILDCARD_INDEX: + if isinstance(data, list): + for item in data: + _delete(item, k_list.copy()) + + _delete(payload, keys.copy()) + + class Setter: TYPE: Any SETTERS: dict[JsonPathItemType, Type["Setter"]] = {} diff --git a/qdrant_client/local/tests/test_payload_utils.py b/qdrant_client/local/tests/test_payload_utils.py index bb76621f9..4bbca6d42 100644 --- a/qdrant_client/local/tests/test_payload_utils.py +++ b/qdrant_client/local/tests/test_payload_utils.py @@ -8,7 +8,7 @@ parse_json_path, ) from qdrant_client.local.payload_value_extractor import value_by_key -from qdrant_client.local.payload_value_setter import set_value_by_key +from qdrant_client.local.payload_value_setter import delete_value_by_key, set_value_by_key def test_parse_json_path() -> None: @@ -183,6 +183,25 @@ def test_parse_json_path() -> None: jp_key = "a.c[].[]" parse_json_path(jp_key) + # the server accepts only unsigned decimal digits that fit into u64 + for jp_key in ( + "a[-1]", + "a[-1].b", + "a[+1]", + "a[1_0]", + "a[ 1 ]", + "a[\u0661]", + f"a[{2**64}]", + ): + with pytest.raises(ValueError): + parse_json_path(jp_key) + + assert parse_json_path("a[01]") == [ + JsonPathItem(item_type=JsonPathItemType.KEY, key="a"), + JsonPathItem(item_type=JsonPathItemType.INDEX, index=1), + ] + assert parse_json_path(f"a[{2**64 - 1}]")[1].index == 2**64 - 1 + def test_value_by_key() -> None: payload = { @@ -460,41 +479,32 @@ def test_set_value_by_key() -> None: # region exceptions - try: + # incorrect quotes + with pytest.raises(ValueError): payload = {"a": []} new_value = {"c": 3} key = "a.'b.c'" set_value_by_key(payload, parse_json_path(key), new_value) - assert False, f"Should've raised an exception due to the key with incorrect quotes: {key}" - except Exception: - assert True - try: + # negative indexation is not supported + with pytest.raises(ValueError): payload = {"a": [{"b": 1}, {"b": 2}]} new_value = {"c": 3} key = "a[-1]" set_value_by_key(payload, parse_json_path(key), new_value) - assert False, "Negative indexation is not supported" - except Exception: - assert True - try: + # unbalanced brackets + with pytest.raises(ValueError): payload = {"a": [{"b": 1}, {"b": 2}]} new_value = {"c": 3} key = "a[" set_value_by_key(payload, parse_json_path(key), new_value) - assert False, f"Should've raised an exception due to the incorrect key: {key}" - except Exception: - assert True - try: + with pytest.raises(ValueError): payload = {"a": [{"b": 1}, {"b": 2}]} new_value = {"c": 3} key = "a]" set_value_by_key(payload, parse_json_path(key), new_value) - assert False, f"Should've raise an exception due to the incorrect key: {key}" - except Exception: - assert True # endregion @@ -547,3 +557,42 @@ def test_set_value_by_key() -> None: set_value_by_key(payload, parse_json_path(key), new_value) assert payload == {"a": {"c": [[]]}}, payload # endregion + + +def test_delete_value_by_key() -> None: + def delete(payload: dict, key: str) -> dict: + delete_value_by_key(payload, parse_json_path(key)) + return payload + + # region top-level + assert delete({"a": 1, "b": 2}, "a") == {"b": 2} + assert delete({"a": {"c": 2}}, "nope") == {"a": {"c": 2}} + # endregion + + # region nested keys, siblings preserved + assert delete({"a": {"b": 1, "c": 2}, "top": 9}, "a.b") == {"a": {"c": 2}, "top": 9} + assert delete({"a": {"b": {"c": 1, "d": 2}}}, "a.b.c") == {"a": {"b": {"d": 2}}} + assert delete({"the": {"nested.key": 1, "b": 2}}, 'the."nested.key"') == {"the": {"b": 2}} + # endregion + + # region arrays + assert delete({"loc": [{"x": 1}, {"x": 2}]}, "loc[0].x") == {"loc": [{}, {"x": 2}]} + assert delete({"loc": [{"x": 1, "y": 2}, {"x": 3}]}, "loc[].x") == {"loc": [{"y": 2}, {}]} + assert delete({"loc": [[1, 2], [3, 4]]}, "loc[][]") == {"loc": [[], []]} + # a terminal wildcard clears the array, a terminal index is a no-op + assert delete({"loc": [1, 2, 3], "top": 9}, "loc[]") == {"loc": [], "top": 9} + assert delete({"loc": [{"x": 1}, {"x": 2}]}, "loc[0]") == {"loc": [{"x": 1}, {"x": 2}]} + assert delete({"loc": [1, 2, 3]}, "loc[1]") == {"loc": [1, 2, 3]} + # arrays are not implicitly flattened, unlike in filters + assert delete({"loc": [{"x": 1}]}, "loc.x") == {"loc": [{"x": 1}]} + # endregion + + # region paths that do not resolve + assert delete({"loc": [{"x": 1}]}, "loc[5].x") == {"loc": [{"x": 1}]} + assert delete({"a": {"c": 2}}, "a.b") == {"a": {"c": 2}} + assert delete({"a": {"c": 2}}, "nope.nested") == {"a": {"c": 2}} + assert delete({"a": 5}, "a.b") == {"a": 5} + assert delete({"loc": {"x": 1}}, "loc[]") == {"loc": {"x": 1}} + # a dotted path never matches a literal key containing a dot + assert delete({"a.b": 1, "a": {"b": 2}}, "a.b") == {"a.b": 1, "a": {}} + # endregion diff --git a/tests/congruence_tests/test_payload.py b/tests/congruence_tests/test_payload.py index dd6ebbc1a..bd6df8873 100644 --- a/tests/congruence_tests/test_payload.py +++ b/tests/congruence_tests/test_payload.py @@ -82,6 +82,82 @@ def test_delete_payload(prefer_grpc): # endregion +@pytest.mark.parametrize("prefer_grpc", [True, False]) +def test_delete_payload_with_nested_key(prefer_grpc): + local_client = init_local() + remote_client = init_remote(prefer_grpc=prefer_grpc) + + vector_size = 2 + vectors_config = models.VectorParams(size=vector_size, distance=models.Distance.COSINE) + initialize_fixture_collection(local_client, vectors_config=vectors_config) + initialize_fixture_collection(remote_client, vectors_config=vectors_config) + + vector = np.random.rand(vector_size).tolist() + + def delete_keys(payload, keys): + for client in (local_client, remote_client): + client.upsert( + collection_name=COLLECTION_NAME, + points=[PointStruct(id=9999, payload=payload, vector=vector)], + wait=True, + ) + client.delete_payload( + collection_name=COLLECTION_NAME, keys=keys, points=[9999], wait=True + ) + compare_collections(local_client, remote_client, 1) + + # nested dict key: only the leaf is removed, siblings preserved + delete_keys({"a": {"b": 1, "c": 2}, "top": 9}, ["a.b"]) + + # a non-existent nested path is a no-op + delete_keys({"a": {"c": 2}}, ["a.b"]) + + # field inside every element of a nested array + delete_keys({"loc": [{"x": 1, "y": 2}, {"x": 3, "y": 4}]}, ["loc[].x"]) + + # a terminal array index is a no-op on the server (deleting a single + # element by index is not idempotent), so both sides leave it untouched + delete_keys({"loc": [{"x": 1}, {"x": 2}]}, ["loc[0]"]) + + # a terminal wildcard clears the array instead + delete_keys({"loc": [1, 2, 3], "top": 9}, ["loc[]"]) + + # arrays are not implicitly flattened: "loc.x" does not reach into elements, + # unlike in filters, where "loc.x" and "loc[].x" are equivalent + delete_keys({"loc": [{"x": 1, "y": 2}]}, ["loc.x"]) + + # a dotted path never matches a literal key containing a dot + delete_keys({"a.b": 1, "a": {"b": 2}}, ["a.b"]) + + # nested wildcards clear each inner array + delete_keys({"loc": [[1, 2], [3, 4]]}, ["loc[][]"]) + + # top-level and nested keys together + delete_keys({"a": {"b": 1}, "top": 9}, ["a.b", "top"]) + + # an invalid json path rejects the whole request on both sides: nothing is + # deleted, not even for the keys preceding the invalid one. "loc[-1]" is + # invalid too: the server addresses array elements with an unsigned index. + for invalid_key in ("not a valid path", "loc[-1]", "loc[-1].x"): + for client in (local_client, remote_client): + client.upsert( + collection_name=COLLECTION_NAME, + points=[ + PointStruct(id=9999, payload={"a": {"b": 1}, "loc": [1, 2]}, vector=vector), + PointStruct(id=10000, payload={"a": {"b": 1}, "loc": [1, 2]}, vector=vector), + ], + wait=True, + ) + with pytest.raises((ValueError, UnexpectedResponse, grpc.RpcError)): + client.delete_payload( + collection_name=COLLECTION_NAME, + keys=["a.b", invalid_key], + points=[9999, 10000], + wait=True, + ) + compare_collections(local_client, remote_client, 2) + + @pytest.mark.parametrize("prefer_grpc", [True, False]) def test_clear_payload(prefer_grpc): local_client: QdrantClient = init_local()