diff --git a/pymilvus/client/entity_helper.py b/pymilvus/client/entity_helper.py index d477a4ab0..8171851a3 100644 --- a/pymilvus/client/entity_helper.py +++ b/pymilvus/client/entity_helper.py @@ -1,7 +1,9 @@ import itertools import json import math +import os import struct +import uuid from collections.abc import Sized from typing import Any, Dict, Iterable, List, Optional @@ -197,7 +199,31 @@ def get_max_len_of_var_char(field_info: Dict) -> int: return field_info.get("params", {}).get(k, v) +def _coerce_str_like(value: Any) -> Any: + """Coerce a uuid.UUID or os.PathLike value to its string form; pass through otherwise. + + Uses os.fsdecode rather than os.fspath: __fspath__() is allowed by the + os.PathLike protocol to return bytes, and fsdecode() normalizes that to + str (via the filesystem encoding) instead of leaving raw bytes behind. + """ + if isinstance(value, os.PathLike): + return os.fsdecode(value) + if isinstance(value, uuid.UUID): + return str(value) + return value + + def convert_to_str_array(orig_str_arr: Any, field_info: Dict, check: bool = True): + # convert_to_str_array is called with either a single scalar value (row-based + # insert, via _ROW_SCALAR_NORMALIZERS) or a list/tuple of values (column-based + # insert). uuid.UUID and os.PathLike values are unambiguous as their string + # form, so coerce them before the rest of this function treats its input as + # an iterable of strings. See GH-2917. + if isinstance(orig_str_arr, (uuid.UUID, os.PathLike)): + orig_str_arr = _coerce_str_like(orig_str_arr) + elif isinstance(orig_str_arr, (list, tuple)): + orig_str_arr = [_coerce_str_like(s) for s in orig_str_arr] + arr = [] if Config.EncodeProtocol.lower() != "utf-8": for s in orig_str_arr: @@ -254,6 +280,26 @@ def assign_to_parent(value: Any) -> None: assign_to_parent(float(current)) elif isinstance(current, np.bool_): assign_to_parent(bool(current)) + elif isinstance(current, os.PathLike): + # pathlib.Path (and subclasses like PosixPath/WindowsPath, + # plus the pure variants PurePosixPath/PureWindowsPath) are + # not natively JSON-serializable by orjson, which raises a + # raw, unhelpful "Type is not JSON serializable" TypeError. + # Path values are unambiguous as their string form, so + # convert them here alongside the other leaf-type + # normalizations. os.fsdecode (not os.fspath) since + # __fspath__() is allowed to return bytes, which still + # isn't natively JSON-serializable. See GH-2917. + assign_to_parent(os.fsdecode(current)) + elif isinstance(current, uuid.UUID): + # orjson serializes UUID natively, so this branch is a no-op + # there. But convert_to_json falls back to stdlib json past + # orjson's recursion limit (~500 levels), and stdlib json has + # no native UUID support, so without this the fallback path + # would raise the same raw TypeError this function exists to + # avoid. Normalize here so both serializers are covered. See + # GH-2917. + assign_to_parent(str(current)) elif isinstance(current, dict): # Process dict: create new dict first processed = {} @@ -261,8 +307,14 @@ def assign_to_parent(value: Any) -> None: # Add items to stack for processing (reverse order to maintain original order) for k, v in reversed(tuple(current.items())): stack.append((v, processed, k)) - elif isinstance(current, list): - # Process list: create new list with placeholders first + elif isinstance(current, (list, tuple)): + # Process list/tuple: create new list with placeholders first. + # Tuples are staged as a list (mutable, needed while children + # are resolved below) rather than kept as a tuple; JSON has no + # tuple/list distinction, so orjson serializes either the same + # way. Without this, a tuple's elements were previously never + # descended into, so e.g. a PathLike inside a tuple stayed + # unconverted and still failed to serialize. See GH-2917. processed = [None] * len(current) assign_to_parent(processed) # Add items to stack for processing (reverse order to maintain original order) diff --git a/pymilvus/orm/types.py b/pymilvus/orm/types.py index c3cd50949..8c4699d70 100644 --- a/pymilvus/orm/types.py +++ b/pymilvus/orm/types.py @@ -10,6 +10,8 @@ # or implied. See the License for the specific language governing permissions and limitations under # the License. +import os +import uuid from typing import Any import numpy as np @@ -92,12 +94,23 @@ def infer_dtype_by_scalar_data(data: Any, dtype: DataType = None): return DataType.VARCHAR if isinstance(data, bytes): return DataType.BINARY_VECTOR + # uuid.UUID and pathlib.Path (and its variants) are unambiguous as their + # string form, so treat them as VARCHAR rather than falling through to + # UNKNOWN. See GH-2917. + if isinstance(data, (uuid.UUID, os.PathLike)): + return DataType.VARCHAR return DataType.UNKNOWN def infer_dtype_bydata(data: Any): d_type = DataType.UNKNOWN + # pandas' is_scalar() doesn't consider os.PathLike scalar, so it would + # otherwise fall through to the data[0] subscript probe below and raise + # TypeError (Path objects aren't subscriptable). Handle both PathLike and + # UUID here explicitly before that probe. See GH-2917. + if isinstance(data, (uuid.UUID, os.PathLike)): + return infer_dtype_by_scalar_data(data) if is_scalar(data): return infer_dtype_by_scalar_data(data) @@ -121,7 +134,7 @@ def infer_dtype_bydata(data: Any): if d_type == DataType.UNKNOWN: try: elem = data[0] - except IndexError: + except (IndexError, TypeError): elem = None if elem is not None and is_scalar(elem): diff --git a/tests/unit/entity_helper/test_edge_cases.py b/tests/unit/entity_helper/test_edge_cases.py index c07a6e3b8..1e7a60a30 100644 --- a/tests/unit/entity_helper/test_edge_cases.py +++ b/tests/unit/entity_helper/test_edge_cases.py @@ -1,8 +1,13 @@ +import json +import os +import pathlib import struct +import uuid from unittest import mock from unittest.mock import MagicMock, patch import numpy as np +import orjson import pytest from pymilvus.client import entity_helper from pymilvus.client.entity_helper import ( @@ -193,6 +198,96 @@ def test_json_arr_with_none(self): with pytest.raises(ParamError): entity_to_json_arr([None], {"name": "json_field"}) + # Regression tests for GH-2917: pathlib.Path values (e.g. WindowsPath, + # PosixPath, or the pure variants) in a JSON/dynamic field used to raise + # a raw, unhelpful orjson TypeError ("Type is not JSON serializable: ...") + # instead of being handled like the other leaf types (numpy scalars, + # etc.) that convert_to_json already normalizes before serialization. + def test_json_with_windows_path(self): + """A PureWindowsPath value is serialized as its string form""" + + result = convert_to_json({"path": pathlib.PureWindowsPath("C:\\Users\\a")}) + assert result == b'{"path":"C:\\\\Users\\\\a"}' + + def test_json_with_native_path(self): + """A concrete, OS-native pathlib.Path value is serialized as its string form. + + Uses pathlib.Path rather than PosixPath/WindowsPath directly, since a + concrete cross-flavor path class can only be instantiated on its + matching OS on Python < 3.13, and this repo's CI runs both Windows + and Linux; the expected JSON is computed from the same OS-native + string form rather than a hardcoded separator. + """ + p = pathlib.Path("tmp") / "foo" / "bar.txt" + result = convert_to_json({"path": p}) + assert result == orjson.dumps({"path": str(p)}) + + def test_json_with_nested_path_values(self): + """Path values nested in lists/dicts are converted, not just top-level""" + + data = { + "paths": [pathlib.PurePosixPath("/a"), pathlib.PurePosixPath("/b")], + "meta": {"p": pathlib.PurePosixPath("/x/y")}, + } + result = convert_to_json(data) + assert result == b'{"paths":["/a","/b"],"meta":{"p":"/x/y"}}' + + def test_json_with_path_nested_in_tuple(self): + """PR review (yhmo): preprocess_numpy_types previously only descended + into dict/list, so a PathLike inside a tuple (which orjson also + accepts as a JSON array) was left unconverted and still raised + TypeError. Tuples must be traversed too.""" + + result = convert_to_json({"paths": (pathlib.PurePosixPath("/a"),)}) + assert result == b'{"paths":["/a"]}' + + def test_json_with_bytes_returning_pathlike(self): + """PR review (yhmo): os.PathLike.__fspath__() is allowed to return + bytes, not just str. os.fspath() would leave that as raw bytes, + which orjson still can't serialize; os.fsdecode() must be used to + normalize to text.""" + + class BytesPath(os.PathLike): + def __fspath__(self): + return b"/tmp/bytes-path" + + result = convert_to_json({"p": BytesPath()}) + assert result == b'{"p":"/tmp/bytes-path"}' + + def test_json_with_deeply_nested_uuid(self): + """PR review (shashvat-singham): convert_to_json falls back to stdlib + json past orjson's recursion limit (~500 levels), and stdlib json has + no native UUID support. Unlike PathLike, UUID previously had no + conversion branch in preprocess_numpy_types and relied on orjson's + native UUID support, so a UUID nested past that depth still raised + the raw TypeError this function exists to avoid.""" + + def nest(leaf, depth): + o = leaf + for _ in range(depth): + o = {"n": o} + return o + + u = uuid.uuid4() + result = convert_to_json(nest(u, 600)) + # >500 levels overflows orjson's recursion limit, so convert_to_json + # falls back to stdlib json for serialization. + assert result == json.dumps(nest(str(u), 600)).encode() + + def test_entity_to_field_data_varchar_uuid_and_pathlike(self): + """GH-2917 end-to-end: a column of uuid.UUID/PathLike values for a + VARCHAR field is coerced to strings through the real column-insert + path (entity_to_field_data -> entity_to_str_arr -> + convert_to_str_array), not just the helper functions in isolation.""" + u = uuid.uuid4() + p = pathlib.PurePosixPath("/tmp/foo") + entity = {"name": "meta", "type": DataType.VARCHAR, "values": [u, p]} + field_info = {"name": "meta", "params": {"max_length": 256}} + + result = entity_to_field_data(entity, field_info, 2) + + assert list(result.scalars.string_data.data) == [str(u), "/tmp/foo"] + # Tests from TestPackExceptionsMock def test_pack_exceptions_mock(self): """Test exception handling in pack_field_value_to_field_data using mocks""" diff --git a/tests/unit/orm/test_types.py b/tests/unit/orm/test_types.py index c44238be3..829e5c4be 100644 --- a/tests/unit/orm/test_types.py +++ b/tests/unit/orm/test_types.py @@ -1,3 +1,6 @@ +import pathlib +import uuid + import numpy as np import pytest from pymilvus.client.types import DataType @@ -102,6 +105,15 @@ class TestInferDtypeByScalarData: (b"\x00\x01", None, DataType.BINARY_VECTOR), # unknown type -> UNKNOWN (object(), None, DataType.UNKNOWN), + # uuid.UUID -> VARCHAR (GH-2917) + (uuid.uuid4(), None, DataType.VARCHAR), + # pathlib.Path variants -> VARCHAR (GH-2917). Pure variants are used + # here since concrete PosixPath/WindowsPath can only be instantiated + # on their matching OS on Python < 3.13; pathlib.Path (the OS-native + # concrete class) is exercised separately below. + (pathlib.PurePosixPath("/tmp/foo"), None, DataType.VARCHAR), + (pathlib.PureWindowsPath("C:\\Users\\a"), None, DataType.VARCHAR), + (pathlib.Path("/tmp/foo/bar.txt"), None, DataType.VARCHAR), ], ) def test_inference(self, data, dtype, expected): @@ -255,6 +267,18 @@ def __getitem__(self, idx): assert infer_dtype_bydata(NotListLikeFloat()) == DataType.FLOAT + def test_pathlike_returns_varchar_instead_of_crashing(self): + """GH-2917: pandas' is_scalar() is False for PurePath, so without the + explicit PathLike guard this used to fall through to the data[0] probe, + which raises TypeError (PurePath isn't subscriptable) since only + IndexError was caught.""" + assert infer_dtype_bydata(pathlib.PurePosixPath("/tmp/foo")) == DataType.VARCHAR + + def test_uuid_returns_varchar(self): + """GH-2917: uuid.UUID is scalar per pandas, but infer_dtype_by_scalar_data + had no branch for it and fell through to UNKNOWN.""" + assert infer_dtype_bydata(uuid.uuid4()) == DataType.VARCHAR + # --------------------------------------------------------------------------- # map_numpy_dtype_to_datatype diff --git a/tests/unit/test_client_entity_helper.py b/tests/unit/test_client_entity_helper.py index 78699f7d8..df19c7f5f 100644 --- a/tests/unit/test_client_entity_helper.py +++ b/tests/unit/test_client_entity_helper.py @@ -1,4 +1,7 @@ +import os +import pathlib import time +import uuid from typing import ClassVar, Dict, List from unittest.mock import patch @@ -103,6 +106,48 @@ def test_convert_to_str_array(self): result = convert_to_str_array([123, "test"], field_info, check=False) assert len(result) == 2 + def test_convert_to_str_array_coerces_uuid_and_pathlike(self): + """GH-2917: uuid.UUID/os.PathLike values are coerced to str, not rejected.""" + field_info = {"name": "test_field", "params": {Config.MaxVarCharLengthKey: 64}} + u = uuid.uuid4() + p = pathlib.PurePosixPath("/tmp/foo") + + result = convert_to_str_array([u, p, "plain"], field_info) + + assert result == [str(u), "/tmp/foo", "plain"] + + # Non-coercible non-string input is still rejected + with pytest.raises(ParamError, match="expects string input"): + convert_to_str_array([123], field_info) + + def test_convert_to_str_array_coerces_scalar_row_input(self): + """GH-2917: convert_to_str_array is also called with a bare scalar (not a + list) for row-based inserts via _ROW_SCALAR_NORMALIZERS; UUID/PathLike + scalars must be coerced there too, not just inside a list.""" + field_info = {"name": "test_field", "params": {Config.MaxVarCharLengthKey: 64}} + u = uuid.uuid4() + p = pathlib.PureWindowsPath("C:\\Users\\a") + + assert convert_to_str_array(u, field_info) == str(u) + assert convert_to_str_array(p, field_info) == "C:\\Users\\a" + # Plain string scalars are unaffected + assert convert_to_str_array("hello", field_info) == "hello" + + def test_convert_to_str_array_coerces_bytes_returning_pathlike(self): + """PR review (yhmo): os.PathLike.__fspath__() is allowed to return + bytes, not just str. Coercing via os.fspath() would leave raw bytes + behind, which still fails the VARCHAR string check; os.fsdecode() + must be used to normalize to text.""" + + class BytesPath(os.PathLike): + def __fspath__(self): + return b"/tmp/bytes-path" + + field_info = {"name": "test_field", "params": {Config.MaxVarCharLengthKey: 64}} + + assert convert_to_str_array(BytesPath(), field_info) == "/tmp/bytes-path" + assert convert_to_str_array([BytesPath()], field_info) == ["/tmp/bytes-path"] + @patch("pymilvus.client.entity_helper.Config") def test_convert_to_str_array_with_encoding(self, mock_config): """Test string array conversion with different encoding""" @@ -206,6 +251,23 @@ def test_pack_field_value_to_field_data_scalars(self): assert field_data.type == DataType.INT64 assert field_data.scalars.long_data.data[0] == 42 + def test_pack_field_value_to_field_data_uuid_varchar(self): + """GH-2917 end-to-end: a bare uuid.UUID row value for a VARCHAR field + is coerced to string rather than rejected/crashing. This exercises the + real row-insert path (pack_field_value_to_field_data -> + _ROW_SCALAR_NORMALIZERS -> convert_to_str_array), not just the helper + function in isolation, since that's the gap a narrower fix could miss.""" + field_data = schema_pb2.FieldData() + field_data.type = DataType.VARCHAR + field_data.field_name = "id_str" + field_info = {"name": "id_str", "params": {Config.MaxVarCharLengthKey: 64}} + vector_bytes_cache: Dict[int, List[bytes]] = {} + u = uuid.uuid4() + + pack_field_value_to_field_data(u, field_data, field_info, vector_bytes_cache) + + assert field_data.scalars.string_data.data[0] == str(u) + def test_extract_field_info(self): """Test extracting primary field from schema""" # Create schema with primary field