diff --git a/python/tests/detail/params_helper.py b/python/tests/detail/params_helper.py index 24e7d891a..3a9f339c6 100644 --- a/python/tests/detail/params_helper.py +++ b/python/tests/detail/params_helper.py @@ -139,7 +139,7 @@ for param in params ] -COLLECTION_NAME_MAX_LENGTH = 64 +COLLECTION_NAME_MAX_LENGTH = 256 COLLECTION_NAME_VALID_LIST = [ "col", @@ -148,17 +148,20 @@ "collection_2", "123collection-", "a" * COLLECTION_NAME_MAX_LENGTH, -] - -COLLECTION_NAME_INVALID_LIST = [ "l", "1C", - "", " ", - None, - "abcdefghijklmnopqrstuvwxzy123456abcdefghijklmnopqrstuvwxzy1234561", "test/", "!@#$%^&*()test", + "集合名称", +] + +COLLECTION_NAME_INVALID_LIST = [ + "", + None, + "a" * (COLLECTION_NAME_MAX_LENGTH + 1), + "collection\0name", + "collection\nname", ] FIELD_NAME_VALID_LIST = [ @@ -178,7 +181,7 @@ "", " ", None, - "abcdefghijklmnopqrstuvwxzy1234561", + "a" * 65, "test/", "!@#$%^&*()test", "name@with#special$chars", @@ -208,12 +211,12 @@ INCOMPATIBLE_CONSTRUCTOR_ERROR_MSG = "incompatible constructor arguments" -SCHEMA_VALIDATE_ERROR_MSG = "schema validate failed" +SCHEMA_VALIDATE_ERROR_MSG = "Invalid schema" CREATE_READ_ONLY_ERROR_MSG = "Unable to create collection with read-only mode" INCOMPATIBLE_FUNCTION_ERROR_MSG = "incompatible function arguments" INVALID_PATH_ERROR_MSG = "path validate failed" INDEX_NON_EXISTENT_COLUMN_ERROR_MSG = "not found in schema" ACCESS_DESTROYED_COLLECTION_ERROR_MSG = "is already destroyed" COLLECTION_PATH_NOT_EXIST_ERROR_MSG = "not exist" -NOT_SUPPORT_ADD_COLUMN_ERROR_MSG = "Only support basic numeric data type" -NOT_EXIST_COLUMN_TO_DROP_ERROR_MSG = "Column not exists" +NOT_SUPPORT_ADD_COLUMN_ERROR_MSG = "this operation requires a numeric field" +NOT_EXIST_COLUMN_TO_DROP_ERROR_MSG = "field.*not found" diff --git a/python/tests/detail/test_collection_ddl.py b/python/tests/detail/test_collection_ddl.py index a2c682f46..b3e07ebd8 100644 --- a/python/tests/detail/test_collection_ddl.py +++ b/python/tests/detail/test_collection_ddl.py @@ -457,7 +457,7 @@ def check_error_message(exc_info, invalid_name): [ ("", ""), # Empty string (" ", " "), # Space only - ("v" * 33, "v" * 33), # Too long (33 characters, exceeds 32) + ("v" * 33, "v" * 33), # Field does not exist. ("vector name", "vector_name"), # Contains space ("vector@name", "vector@name"), # Contains special character ("vector/name", "vector/name"), # Contains slash @@ -1202,7 +1202,7 @@ def test_add_column_with_index_param(self, basic_collection: Collection): "field_name", [ "a", # Minimum length - "a" * 32, # Maximum length (32 characters) + "a" * 64, # Maximum field name length. "valid_field_name_123", # Alphanumeric with underscore "Valid-Field-Name", # With hyphens "_underscore_start", # Starting with underscore @@ -1241,7 +1241,7 @@ def test_add_column_with_valid_field_names( [ "", # Empty string " ", # Space only - "a" * 33, # Too long (33 characters, exceeds 32) + "a" * 65, # Exceeds the field name byte limit. "field name", # Contains space "field.name", # Contains dot "field@name", # Contains special character @@ -1263,7 +1263,7 @@ def test_add_column_with_invalid_field_names( ) if invalid_field_name is None: - assert "validate failed" in str(exc_info.value), ( + assert "Invalid schema:" in str(exc_info.value), ( "Error message is unreasonable: e=" + str(exc_info.value) ) else: @@ -1303,7 +1303,7 @@ def test_alter_column_non_exist(self, basic_collection: Collection): new_name="new_name", field_schema=FieldSchema("new_name", DataType.STRING), ) - assert "column non_existing not found" in str(exc_info.value), ( + assert "Invalid schema: field[non_existing] not found" in str(exc_info.value), ( "Error message is unreasonable: e=" + str(exc_info.value) ) @@ -1378,9 +1378,9 @@ def test_alter_column_with_various_concurrency_options( [ ("a", "new_a"), # Minimum length ( - "abcdefghijklmnopqrstuvwxyz123456", - "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - ), # Maximum length (32 characters) + "a" * 64, + "b" * 64, + ), # Maximum field name length. ("valid_field_name_123", "new_valid_field"), # Alphanumeric with underscore ("Valid-Field-Name", "New-Field-Name"), # With hyphens ("_underscore_start", "new_underscore"), # Starting with underscore @@ -1427,7 +1427,7 @@ def test_alter_column_field_name_valid( "valid_old_name,invalid_new_name", [ ("temp_field", ""), # Empty new name - ("temp_field", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"), # Too long new name + ("temp_field", "a" * 65), # Exceeds the field name byte limit. ("temp_field", "field name"), # New name with space ("temp_field", "field.name"), # New name with dot ("temp_field", "field@name"), # New name with special character @@ -1499,15 +1499,14 @@ def test_drop_column_exist(self, basic_collection: Collection): assert SCHEMA_VALIDATE_ERROR_MSG in str(exc_info.value) def test_drop_column_non_exist(self, basic_collection: Collection): - with pytest.raises(Exception) as exc_info: + with pytest.raises(Exception, match=NOT_EXIST_COLUMN_TO_DROP_ERROR_MSG): basic_collection.drop_column("non_existing_column") - assert NOT_EXIST_COLUMN_TO_DROP_ERROR_MSG in str(exc_info.value) @pytest.mark.parametrize( "field_name", [ "a", # Minimum length - "a" * 32, # Maximum length (32 characters) + "a" * 64, # Maximum field name length. "valid_field_name_123", # Alphanumeric with underscore "Valid-Field-Name", # With hyphens "_underscore_start", # Starting with underscore diff --git a/python/tests/detail/test_collection_dml.py b/python/tests/detail/test_collection_dml.py index e1b0c97e2..9bd0597d2 100644 --- a/python/tests/detail/test_collection_dml.py +++ b/python/tests/detail/test_collection_dml.py @@ -26,14 +26,18 @@ "123abc", "-!@#$%+=.123abc_+", "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ123456789012", + "()qsd123", + " ", + "/&AS12", + "订单:2026", + "a" * 1024, ] DOCID_INVALID_LIST = [ None, "", - "()qsd123", - " ", - "/&AS12", - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890121", + "a" * 1025, + "doc\0id", + "doc\nid", ] FIELD_VALUE_VALID_LIST = [ diff --git a/python/tests/test_name_validation.py b/python/tests/test_name_validation.py new file mode 100644 index 000000000..b9d584d49 --- /dev/null +++ b/python/tests/test_name_validation.py @@ -0,0 +1,263 @@ +# Copyright 2025-present the zvec project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import re + +import pytest +import zvec + + +@pytest.fixture +def collection(tmp_path): + schema = zvec.CollectionSchema( + "validation", fields=zvec.FieldSchema("text", zvec.DataType.STRING) + ) + coll = zvec.create_and_open(str(tmp_path / "collection"), schema) + try: + yield coll + finally: + coll.destroy() + + +@pytest.mark.parametrize("name", ["x", "ab", "集合 / 2026 🔎", "集" * 85 + "a"]) +def test_collection_names_round_trip(tmp_path, name): + path = str(tmp_path / "collection") + schema = zvec.CollectionSchema( + name, fields=zvec.FieldSchema("text", zvec.DataType.STRING) + ) + coll = zvec.create_and_open(path, schema) + try: + assert coll.schema.name == name + coll.close() + coll = zvec.open(path) + assert coll.schema.name == name + finally: + coll.close() + + +def test_document_ids_round_trip_without_normalization(collection): + ids = [ + "user:123", + "https://example.com/articles/42?a=b", + "订单-2026-🙂", + "界" * 341 + "a", # 1024 UTF-8 bytes. + "doc", + " doc", + "doc ", + " ", + "é", + "e\u0301", + ] + text = "正文\nsecond line\tvalue" + statuses = collection.insert( + [zvec.Doc(id=doc_id, fields={"text": text}) for doc_id in ids] + ) + assert all(status.ok() for status in statuses) + collection.flush() + fetched = collection.fetch(ids) + assert set(fetched) == set(ids) + for doc_id in ids: + assert fetched[doc_id].id == doc_id + assert fetched[doc_id].field("text") == text + + +@pytest.mark.parametrize("operation", ["insert", "update", "upsert"]) +@pytest.mark.parametrize( + "doc_id,reason", + [ + ("", "must not be empty"), + ("doc\0id", "null character"), + ("doc\nid", "newline"), + ("界" * 341 + "ab", "exceeds 1024 bytes (got 1025)"), + (b"\xff", "not valid UTF-8"), + ("\ud800", "not valid UTF-8"), + ], +) +def test_invalid_id_rejects_batch_before_writing(collection, operation, doc_id, reason): + if operation == "update": + assert collection.insert(zvec.Doc("valid", fields={"text": "before"})).ok() + + docs = [ + zvec.Doc("valid", fields={"text": "after"}), + zvec.Doc(doc_id, fields={"text": "invalid"}), + ] + with pytest.raises(ValueError) as exc_info: + getattr(collection, operation)(docs) + + message = str(exc_info.value) + assert message.startswith("Invalid doc:") + assert reason in message + assert "document at index" not in message + assert "offset" not in message + if doc_id: + assert "id[" in message + assert "\n" not in message + assert "\0" not in message + fetched = collection.fetch("valid") + if operation == "update": + assert fetched["valid"].field("text") == "before" + else: + assert fetched == {} + + +@pytest.mark.parametrize("operation", ["insert", "update", "upsert"]) +def test_conversion_type_error_rejects_batch_before_writing(collection, operation): + if operation == "update": + assert collection.insert(zvec.Doc("valid", fields={"text": "before"})).ok() + + docs = [ + zvec.Doc("valid", fields={"text": "after"}), + zvec.Doc("invalid", fields={"text": 42}), + ] + with pytest.raises(TypeError) as exc_info: + getattr(collection, operation)(docs) + + message = str(exc_info.value) + assert "Field 'text': expected STRING" in message + assert "document at index" not in message + fetched = collection.fetch("valid") + if operation == "update": + assert fetched["valid"].field("text") == "before" + else: + assert fetched == {} + + +@pytest.mark.parametrize( + "name,reason", + [ + ("", "must not be empty"), + ("collection\0name", "null character"), + ("collection\nname", "newline"), + ("集" * 86, "exceeds 256 bytes (got 258)"), + ], +) +def test_invalid_collection_names_report_the_reason(tmp_path, name, reason): + schema = zvec.CollectionSchema( + name, fields=zvec.FieldSchema("text", zvec.DataType.STRING) + ) + with pytest.raises(ValueError) as exc_info: + zvec.create_and_open(str(tmp_path / "collection"), schema) + message = str(exc_info.value) + assert message.startswith("Invalid schema:") + assert "collection name" in message + assert reason in message + assert "offset" not in message + if name: + assert "collection name[" in message + assert "\n" not in message + assert "\0" not in message + + +@pytest.mark.parametrize( + "doc_id,preview", + [ + ("order\n[123]", r"order\n\[123\]"), + (b"\xff", r"\xFF"), + ("x" * 1025, "x" * 32 + "..."), + ], +) +def test_id_error_includes_safe_preview(collection, doc_id, preview): + with pytest.raises(ValueError) as exc_info: + collection.insert(zvec.Doc(doc_id, fields={"text": "value"})) + assert f"Invalid doc: id[{preview}]" in str(exc_info.value) + + +def test_long_field_name_and_rejected_rename_preserve_data(tmp_path): + field_name = "f" * 64 + schema = zvec.CollectionSchema( + "fields", fields=zvec.FieldSchema(field_name, zvec.DataType.INT32) + ) + coll = zvec.create_and_open(str(tmp_path / "collection"), schema) + try: + assert coll.insert(zvec.Doc("doc", fields={field_name: 42})).ok() + with pytest.raises(ValueError) as exc_info: + coll.alter_column(field_name, new_name="f" * 65) + message = str(exc_info.value) + assert message.startswith("Invalid schema:") + assert "exceeds 64 bytes (got 65)" in message + assert "offset" not in message + assert coll.schema.field(field_name) is not None + assert coll.fetch("doc")["doc"].field(field_name) == 42 + finally: + coll.destroy() + + +@pytest.mark.parametrize("operation", ["insert", "update", "upsert"]) +def test_surrogate_id_has_a_readable_encoding_error(collection, operation): + with pytest.raises( + ValueError, + match="^" + re.escape(r"Invalid doc: id['\ud800'] is not valid UTF-8") + "$", + ): + getattr(collection, operation)(zvec.Doc("\ud800", fields={"text": "value"})) + assert collection.stats.doc_count == 0 + + +@pytest.mark.parametrize("kind", ["collection", "field", "vector"]) +def test_surrogate_schema_name_has_a_readable_encoding_error(kind): + with pytest.raises( + ValueError, match="^Invalid schema: .* is not valid UTF-8$" + ) as exc_info: + if kind == "collection": + zvec.CollectionSchema("\ud800") + elif kind == "field": + zvec.FieldSchema("\ud800", zvec.DataType.INT32) + else: + zvec.VectorSchema("\ud800", zvec.DataType.VECTOR_FP32, dimension=2) + assert r"['\ud800']" in str(exc_info.value) + + +@pytest.mark.parametrize("invalid", [0, False, [], {}, b""]) +@pytest.mark.parametrize("argument", ["new_name", "field_schema"]) +def test_falsey_alter_arguments_are_not_silently_ignored(tmp_path, invalid, argument): + schema = zvec.CollectionSchema( + "fields", fields=zvec.FieldSchema("value", zvec.DataType.INT32) + ) + coll = zvec.create_and_open(str(tmp_path / "collection"), schema) + try: + assert coll.insert(zvec.Doc("doc", fields={"value": 42})).ok() + kwargs = ( + { + "new_name": invalid, + "field_schema": zvec.FieldSchema("renamed", zvec.DataType.INT32), + } + if argument == "new_name" + else {"new_name": "renamed", "field_schema": invalid} + ) + with pytest.raises(TypeError, match="^Invalid schema:"): + coll.alter_column("value", **kwargs) + assert coll.schema.field("value") is not None + assert coll.schema.field("renamed") is None + assert coll.fetch("doc")["doc"].field("value") == 42 + finally: + coll.destroy() + + +@pytest.mark.parametrize("kind", ["field", "vector"]) +@pytest.mark.parametrize("name", ["bad\nname", "x" * 10000]) +def test_duplicate_name_errors_are_escaped_and_bounded(kind, name): + item = ( + zvec.FieldSchema(name, zvec.DataType.INT32) + if kind == "field" + else zvec.VectorSchema(name, zvec.DataType.VECTOR_FP32, dimension=2) + ) + with pytest.raises(ValueError) as exc_info: + zvec.CollectionSchema("duplicates", **{kind + "s": [item, item]}) + message = str(exc_info.value) + assert message.startswith("Invalid schema: duplicate") + assert "\n" not in message + assert len(message) < 256 + if "\n" in name: + assert "\\n" in message + else: + assert "..." in message diff --git a/python/tests/test_uniform_quantization_support_matrix.py b/python/tests/test_uniform_quantization_support_matrix.py index 8b851f664..a64281102 100644 --- a/python/tests/test_uniform_quantization_support_matrix.py +++ b/python/tests/test_uniform_quantization_support_matrix.py @@ -49,7 +49,7 @@ def test_uniform_rejects_unsupported_field_types( ) ], ) - with pytest.raises(ValueError, match="schema validate failed:.*quantiz"): + with pytest.raises(ValueError, match="Invalid schema:.*quantiz"): collection = zvec.create_and_open(str(tmp_path / "collection"), schema=schema) collection.close() diff --git a/python/zvec/model/_validation.py b/python/zvec/model/_validation.py new file mode 100644 index 000000000..1d2e399de --- /dev/null +++ b/python/zvec/model/_validation.py @@ -0,0 +1,31 @@ +# Copyright 2025-present the zvec project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +from __future__ import annotations + + +def explain_utf8_conversion_error(value: object, context: str) -> None: + """Explain a failed native string conversion without rescanning valid inputs.""" + if isinstance(value, str): + try: + value.encode("utf-8") + except UnicodeEncodeError: + raise ValueError( + f"{context}[{format_name_for_error(value)}] is not valid UTF-8" + ) from None + + +def format_name_for_error(name: str) -> str: + """Keep a user-supplied name readable, escaped, and bounded in errors.""" + preview = repr(name[:32]) + return preview + "..." if len(name) > 32 else preview diff --git a/python/zvec/model/collection.py b/python/zvec/model/collection.py index 5db24cb19..33620dd3f 100644 --- a/python/zvec/model/collection.py +++ b/python/zvec/model/collection.py @@ -24,6 +24,7 @@ from ..executor import QueryContext, QueryExecutor from ..extension import ReRanker from ..typing import Status +from ._validation import explain_utf8_conversion_error from .convert import convert_to_cpp_doc, convert_to_py_doc from .doc import Doc, DocList, GroupResult from .param import ( @@ -304,12 +305,21 @@ def alter_column( >>> new_schema = FieldSchema(name="doc_id", dtype=DataType.INT64) >>> collection.alter_column("id", field_schema=new_schema) """ - self._obj.AlterColumn( - old_name, - new_name or "", - field_schema._get_object() if field_schema else None, - option, - ) + if new_name is not None and not isinstance(new_name, str): + raise TypeError("Invalid schema: new column name must be str") + if field_schema is not None and not isinstance(field_schema, FieldSchema): + raise TypeError("Invalid schema: field_schema must be a FieldSchema") + try: + self._obj.AlterColumn( + old_name, + "" if new_name is None else new_name, + field_schema._get_object() if field_schema is not None else None, + option, + ) + except TypeError: + explain_utf8_conversion_error(old_name, "Invalid schema: column name") + explain_utf8_conversion_error(new_name, "Invalid schema: field name") + raise self._schema = CollectionSchema._from_core(self._obj.Schema()) self._querier._schema = self._schema diff --git a/python/zvec/model/convert.py b/python/zvec/model/convert.py index 421bd1741..8dc1a7c28 100644 --- a/python/zvec/model/convert.py +++ b/python/zvec/model/convert.py @@ -13,6 +13,7 @@ from zvec._zvec import _Doc +from ._validation import explain_utf8_conversion_error, format_name_for_error from .doc import Doc from .schema import CollectionSchema @@ -24,14 +25,18 @@ def convert_to_cpp_doc(doc: Doc, collection_schema: CollectionSchema) -> _Doc: _doc = _Doc() # set pk - _doc.set_pk(doc.id) + try: + _doc.set_pk(doc.id) + except TypeError: + explain_utf8_conversion_error(doc.id, "Invalid doc: id") + raise # set scalar fields for k, v in doc.fields.items(): field_schema = collection_schema.field(k) if not field_schema: raise ValueError( - f"schema validate failed: {k} not found in collection schema" + f"Invalid schema: {format_name_for_error(k)} not found in collection schema" ) _doc.set_any(k, field_schema._get_object(), v) @@ -40,7 +45,7 @@ def convert_to_cpp_doc(doc: Doc, collection_schema: CollectionSchema) -> _Doc: vector_schema = collection_schema.vector(k) if not vector_schema: raise ValueError( - f"schema validate failed: {k} not found in collection schema" + f"Invalid schema: {format_name_for_error(k)} not found in collection schema" ) _doc.set_any(k, vector_schema._get_object(), v) return _doc diff --git a/python/zvec/model/schema/collection_schema.py b/python/zvec/model/schema/collection_schema.py index 3e8971040..8e2db897e 100644 --- a/python/zvec/model/schema/collection_schema.py +++ b/python/zvec/model/schema/collection_schema.py @@ -18,6 +18,7 @@ from zvec._zvec.schema import _CollectionSchema, _FieldSchema +from .._validation import explain_utf8_conversion_error, format_name_for_error from .field_schema import FieldSchema, VectorSchema __all__ = [ @@ -64,7 +65,7 @@ def __init__( ): if name is None or not isinstance(name, str): raise ValueError( - f"schema validate failed: collection name must be str, got {type(name).__name__}" + f"Invalid schema: collection name must be str, got {type(name).__name__}" ) # handle fields @@ -75,10 +76,14 @@ def __init__( self._check_vectors(vectors, _fields_name, _fields_list) # init - self._cpp_obj = _CollectionSchema( - name=name, - fields=_fields_list, - ) + try: + self._cpp_obj = _CollectionSchema( + name=name, + fields=_fields_list, + ) + except TypeError: + explain_utf8_conversion_error(name, "Invalid schema: collection name") + raise def _check_fields( self, @@ -96,20 +101,20 @@ def _check_fields( field_items = [] else: raise TypeError( - f"schema validate failed: invalid 'fields' type, expected FieldSchema or list[FieldSchema], " + f"Invalid schema: invalid 'fields' type, expected FieldSchema or list[FieldSchema], " f"got {type(fields).__name__}" ) for idx, field in enumerate(field_items): if not isinstance(field, FieldSchema): raise TypeError( - f"schema validate failed: invalid field type in 'fields' list, expected FieldSchema, " + f"Invalid schema: invalid field type in 'fields' list, expected FieldSchema, " f"got {type(field).__name__} at index {idx}" ) if field.name in _fields_name: raise ValueError( - f"schema validate failed: duplicate field name '{field.name}': field names must be unique" + f"Invalid schema: duplicate field name {format_name_for_error(field.name)}: field names must be unique" ) _fields_name.append(field.name) _fields_list.append(field._get_object()) @@ -129,20 +134,20 @@ def _check_vectors( vectors_items = [] else: raise TypeError( - f"schema validate failed: invalid 'vectors' type, expected VectorSchema or list[VectorSchema], " + f"Invalid schema: invalid 'vectors' type, expected VectorSchema or list[VectorSchema], " f"got {type(vectors).__name__}" ) for idx, vector in enumerate(vectors_items): if not isinstance(vector, VectorSchema): raise TypeError( - f"schema validate failed: invalid vector type in 'vectors' list, expected VectorSchema, " + f"Invalid schema: invalid vector type in 'vectors' list, expected VectorSchema, " f"got {type(vector).__name__} at index {idx}" ) if vector.name in _fields_name: raise ValueError( - f"schema validate failed: duplicate vector name '{vector.name}', vector names must be unique " + f"Invalid schema: duplicate vector name {format_name_for_error(vector.name)}, vector names must be unique " f"(conflicts with existing field or vector)" ) _fields_name.append(vector.name) @@ -152,7 +157,7 @@ def _check_vectors( def _from_core(cls, core_collection_schema: _CollectionSchema): inst = cls.__new__(cls) if not core_collection_schema: - raise ValueError("schema validate failed: schema is null") + raise ValueError("Invalid schema: schema is null") inst._cpp_obj = core_collection_schema return inst diff --git a/python/zvec/model/schema/field_schema.py b/python/zvec/model/schema/field_schema.py index 0005459ba..c0b135ca9 100644 --- a/python/zvec/model/schema/field_schema.py +++ b/python/zvec/model/schema/field_schema.py @@ -28,6 +28,8 @@ ) from zvec.typing import DataType +from .._validation import explain_utf8_conversion_error, format_name_for_error + __all__ = [ "FieldSchema", "VectorSchema", @@ -104,28 +106,32 @@ def __init__( ): if name is None or not isinstance(name, str): raise ValueError( - f"schema validate failed: field name must be str, got {type(name).__name__}" + f"Invalid schema: field name must be str, got {type(name).__name__}" ) if data_type not in SUPPORT_SCALAR_DATA_TYPE: raise ValueError( - f"schema validate failed: scalar_field's data_type must be one of " + f"Invalid schema: scalar_field's data_type must be one of " f"{', '.join(str(dt) for dt in SUPPORT_SCALAR_DATA_TYPE)}, " - f"but field[{name}]'s data_type is {data_type}" + f"but field[{format_name_for_error(name)}]'s data_type is {data_type}" ) - self._cpp_obj = _FieldSchema( - name=name, - data_type=data_type, - dimension=0, - nullable=nullable, - index_param=index_param, - ) + try: + self._cpp_obj = _FieldSchema( + name=name, + data_type=data_type, + dimension=0, + nullable=nullable, + index_param=index_param, + ) + except TypeError: + explain_utf8_conversion_error(name, "Invalid schema: field name") + raise @classmethod def _from_core(cls, core_field_schema: _FieldSchema): if core_field_schema is None: - raise ValueError("schema validate failed: field schema is None") + raise ValueError("Invalid schema: field schema is None") inst = cls.__new__(cls) inst._cpp_obj = core_field_schema return inst @@ -229,29 +235,33 @@ def __init__( ): if name is None or not isinstance(name, str): raise ValueError( - f"schema validate failed: field name must be str, got {type(name).__name__}" + f"Invalid schema: field name must be str, got {type(name).__name__}" ) if not isinstance(dimension, int) or dimension < 0: - raise ValueError("schema validate failed: vector's dimension must be >= 0") + raise ValueError("Invalid schema: vector's dimension must be >= 0") if data_type not in SUPPORT_VECTOR_DATA_TYPE: raise ValueError( - f"schema validate failed: vector's data_type must be one of " + f"Invalid schema: vector's data_type must be one of " f"{', '.join(str(dt) for dt in SUPPORT_VECTOR_DATA_TYPE)}, " - f"but field[{name}]'s data_type is {data_type}" + f"but field[{format_name_for_error(name)}]'s data_type is {data_type}" ) if index_param is None: index_param = FlatIndexParam() - self._cpp_obj = _FieldSchema( - name=name, - data_type=data_type, - dimension=dimension, - nullable=False, - index_param=index_param, - ) + try: + self._cpp_obj = _FieldSchema( + name=name, + data_type=data_type, + dimension=dimension, + nullable=False, + index_param=index_param, + ) + except TypeError: + explain_utf8_conversion_error(name, "Invalid schema: field name") + raise @classmethod def _from_core(cls, core_field_schema: _FieldSchema): diff --git a/src/binding/c/c_api.cc b/src/binding/c/c_api.cc index 88c021cd5..bf9150875 100644 --- a/src/binding/c/c_api.cc +++ b/src/binding/c/c_api.cc @@ -725,7 +725,7 @@ zvec_error_code_t zvec_initialize(const zvec_config_data_t *config) { // Initialize global configuration auto status = zvec::GlobalConfig::Instance().initialize(cpp_config); if (!status.ok()) { - set_last_error(status.message()); + SET_LAST_ERROR(ZVEC_ERROR_INTERNAL_ERROR, status.message()); return ZVEC_ERROR_INTERNAL_ERROR; } @@ -827,6 +827,15 @@ static zvec_error_code_t status_to_error_code(const zvec::Status &status) { return static_cast(status.code()); } +// Record a returned core error without changing the per-document status mapping. +static zvec_error_code_t handle_status(const zvec::Status &status) { + const auto code = status_to_error_code(status); + if (code != ZVEC_OK) { + set_last_error_details(code, status.message()); + } + return code; +} + // Helper function: handle Expected results template static zvec_error_code_t handle_expected_result( @@ -837,8 +846,7 @@ static zvec_error_code_t handle_expected_result( } return ZVEC_OK; } else { - set_last_error(result.error().message()); - return status_to_error_code(result.error()); + return handle_status(result.error()); } } @@ -910,21 +918,6 @@ static zvec_error_code_t build_write_results( return ZVEC_OK; } -static std::vector collect_doc_pks(const zvec_doc_t **docs, - size_t doc_count) { - std::vector pks; - pks.reserve(doc_count); - for (size_t i = 0; i < doc_count; ++i) { - if (!docs[i]) { - pks.emplace_back(""); - continue; - } - auto *doc_ptr = reinterpret_cast(docs[i]); - pks.emplace_back(doc_ptr->pk_ref()); - } - return pks; -} - // ============================================================================= // Type conversion helpers // ============================================================================= @@ -2325,16 +2318,16 @@ zvec_error_code_t zvec_field_schema_set_index_params( zvec_error_code_t zvec_field_schema_validate(const zvec_field_schema_t *schema, zvec_string_t **error_msg) { + if (error_msg) { + *error_msg = nullptr; + } + if (!schema) { SET_LAST_ERROR(ZVEC_ERROR_INVALID_ARGUMENT, "Field schema pointer cannot be null"); return ZVEC_ERROR_INVALID_ARGUMENT; } - if (error_msg) { - *error_msg = nullptr; - } - ZVEC_TRY_RETURN_ERROR( "Failed to validate field schema", auto *cpp_schema = reinterpret_cast(schema); @@ -2342,7 +2335,7 @@ zvec_error_code_t zvec_field_schema_validate(const zvec_field_schema_t *schema, if (error_msg) { *error_msg = zvec_string_create(status.message().c_str()); } - return status_to_error_code(status); + return handle_status(status); }) return ZVEC_OK; @@ -2430,7 +2423,7 @@ zvec_error_code_t zvec_collection_schema_add_field(zvec_collection_schema_t *sch // Clone the field schema auto cloned_field = std::make_shared(*cpp_field); auto status = cpp_schema->add_field(cloned_field); - return status_to_error_code(status);) + return handle_status(status);) } zvec_error_code_t zvec_collection_schema_alter_field( @@ -2451,7 +2444,7 @@ zvec_error_code_t zvec_collection_schema_alter_field( auto cloned_field = std::make_shared(*cpp_new_field); auto status = cpp_schema->alter_field(std::string(field_name), cloned_field); - return status_to_error_code(status);) + return handle_status(status);) } zvec_error_code_t zvec_collection_schema_drop_field(zvec_collection_schema_t *schema, @@ -2466,7 +2459,7 @@ zvec_error_code_t zvec_collection_schema_drop_field(zvec_collection_schema_t *sc "Failed to drop field", auto *cpp_schema = reinterpret_cast(schema); auto status = cpp_schema->drop_field(std::string(field_name)); - return status_to_error_code(status);) + return handle_status(status);) } bool zvec_collection_schema_has_field(const zvec_collection_schema_t *schema, @@ -2744,16 +2737,16 @@ zvec_error_code_t zvec_collection_schema_set_max_doc_count_per_segment( zvec_error_code_t zvec_collection_schema_validate( const zvec_collection_schema_t *schema, zvec_string_t **error_msg) { + if (error_msg) { + *error_msg = nullptr; + } + if (!schema) { SET_LAST_ERROR(ZVEC_ERROR_INVALID_ARGUMENT, "Collection schema pointer cannot be null"); return ZVEC_ERROR_INVALID_ARGUMENT; } - if (error_msg) { - *error_msg = nullptr; - } - ZVEC_TRY_RETURN_ERROR( "Failed to validate schema", auto *cpp_schema = @@ -2762,7 +2755,7 @@ zvec_error_code_t zvec_collection_schema_validate( if (error_msg) { *error_msg = zvec_string_create(status.message().c_str()); } - return status_to_error_code(status); + return handle_status(status); } return ZVEC_OK;) } @@ -2783,7 +2776,7 @@ zvec_error_code_t zvec_collection_schema_add_index( auto cpp_index_params = convert_c_index_params_to_cpp(index_params); auto status = cpp_schema->add_index(std::string(field_name), cpp_index_params); - return status_to_error_code(status);) + return handle_status(status);) } zvec_error_code_t zvec_collection_schema_drop_index(zvec_collection_schema_t *schema, @@ -3187,8 +3180,14 @@ std::vector extract_binary_array(const void *value, return binary_array; } -static std::vector convert_zvec_docs_to_internal( +static zvec::Result> convert_zvec_docs_to_internal( const zvec_doc_t **zvec_docs, size_t doc_count) { + for (size_t i = 0; i < doc_count; ++i) { + if (!zvec_docs[i]) { + return tl::make_unexpected(zvec::Status::InvalidArgument( + "Invalid doc: document must not be null")); + } + } std::vector docs; docs.reserve(doc_count); @@ -4792,10 +4791,12 @@ zvec_error_code_t zvec_doc_to_detail_string(const zvec_doc_t *doc, char **detail zvec_error_code_t zvec_collection_create_and_open( const char *path, const zvec_collection_schema_t *schema, const zvec_collection_options_t *options, zvec_collection_t **collection) { + if (collection) *collection = nullptr; ZVEC_TRY_RETURN_ERROR( "Exception in zvec_collection_create_and_open_with_schema", if (!path || !schema || !collection) { - set_last_error("Path, schema, or collection cannot be null"); + SET_LAST_ERROR(ZVEC_ERROR_INVALID_ARGUMENT, + "Path, schema, or collection cannot be null"); return ZVEC_ERROR_INVALID_ARGUMENT; } @@ -4804,7 +4805,7 @@ zvec_error_code_t zvec_collection_create_and_open( auto status = convert_zvec_collection_schema_to_internal(schema, schema_ptr); if (!status.ok()) { - set_last_error(status.message()); + SET_LAST_ERROR(ZVEC_ERROR_INVALID_ARGUMENT, status.message()); return ZVEC_ERROR_INVALID_ARGUMENT; } @@ -4879,9 +4880,7 @@ zvec_error_code_t zvec_collection_destroy(zvec_collection_t *collection) { auto &coll = *reinterpret_cast *>(collection); zvec::Status status = coll->destroy(); - if (!status.ok()) { set_last_error(status.message()); } - - return status_to_error_code(status);) + return handle_status(status);) } zvec_error_code_t zvec_collection_flush(zvec_collection_t *collection) { @@ -4896,9 +4895,7 @@ zvec_error_code_t zvec_collection_flush(zvec_collection_t *collection) { *reinterpret_cast *>(collection); zvec::Status status = coll->flush(); - if (!status.ok()) { set_last_error(status.message()); } - - return status_to_error_code(status);) + return handle_status(status);) } zvec_error_code_t zvec_collection_get_schema(const zvec_collection_t *collection, @@ -6805,7 +6802,7 @@ zvec_error_code_t zvec_collection_create_index( reinterpret_cast(index_params); auto index_params_ptr = cpp_params->clone(); auto status = (*coll_ptr)->create_index(field_name_str, index_params_ptr); - return status_to_error_code(status);) + return handle_status(status);) } /** @@ -6827,9 +6824,7 @@ zvec_error_code_t zvec_collection_drop_index(zvec_collection_t *collection, auto coll_ptr = reinterpret_cast *>(collection); zvec::Status status = (*coll_ptr)->drop_index(column_name); - if (!status.ok()) { set_last_error(status.message()); } - - return status_to_error_code(status);) + return handle_status(status);) } /** @@ -6848,9 +6843,7 @@ zvec_error_code_t zvec_collection_optimize(zvec_collection_t *collection) { auto coll_ptr = reinterpret_cast *>(collection); zvec::Status status = (*coll_ptr)->optimize(); - if (!status.ok()) { set_last_error(status.message()); } - - return status_to_error_code(status);) + return handle_status(status);) } // ============================================================================= @@ -6887,9 +6880,7 @@ zvec_error_code_t zvec_collection_add_column(zvec_collection_t *collection, std::string expr = expression ? expression : ""; zvec::Status status = (*coll_ptr)->add_column(schema, expr); - if (!status.ok()) { set_last_error(status.message()); } - - return status_to_error_code(status);) + return handle_status(status);) } /** @@ -6912,9 +6903,7 @@ zvec_error_code_t zvec_collection_drop_column(zvec_collection_t *collection, reinterpret_cast *>(collection); zvec::Status status = (*coll_ptr)->drop_column(column_name); - if (!status.ok()) { set_last_error(status.message()); } - - return status_to_error_code(status);) + return handle_status(status);) } zvec_error_code_t zvec_collection_alter_column( @@ -6944,9 +6933,7 @@ zvec_error_code_t zvec_collection_alter_column( zvec::Status status = (*coll_ptr)->alter_column(column_name, rename, schema); - if (!status.ok()) { set_last_error(status.message()); } - - return status_to_error_code(status);) + return handle_status(status);) } // ============================================================================= @@ -6957,9 +6944,11 @@ zvec_error_code_t zvec_collection_insert(zvec_collection_t *collection, const zvec_doc_t **docs, size_t doc_count, size_t *success_count, size_t *error_count) { + if (success_count) *success_count = 0; + if (error_count) *error_count = doc_count; if (!collection || !docs || doc_count == 0 || !success_count || !error_count) { - set_last_error( + SET_LAST_ERROR(ZVEC_ERROR_INVALID_ARGUMENT, "Invalid arguments: collection, docs, doc_count, success_count and " "error_count cannot be null/zero"); return ZVEC_ERROR_INVALID_ARGUMENT; @@ -6970,8 +6959,11 @@ zvec_error_code_t zvec_collection_insert(zvec_collection_t *collection, auto coll_ptr = reinterpret_cast *>(collection); - std::vector internal_docs = - convert_zvec_docs_to_internal(docs, doc_count); + auto converted_docs = convert_zvec_docs_to_internal(docs, doc_count); + if (!converted_docs.has_value()) { + return handle_status(converted_docs.error()); + } + auto &internal_docs = converted_docs.value(); auto result = (*coll_ptr)->insert(internal_docs); zvec_error_code_t error_code = handle_expected_result(result); @@ -6999,24 +6991,25 @@ zvec_error_code_t zvec_collection_insert_with_results(zvec_collection_t *collect size_t doc_count, zvec_write_result_t **results, size_t *result_count) { + if (results) *results = nullptr; + if (result_count) *result_count = 0; if (!collection || !docs || doc_count == 0 || !results || !result_count) { - set_last_error( + SET_LAST_ERROR(ZVEC_ERROR_INVALID_ARGUMENT, "Invalid arguments: collection, docs, doc_count, results and " "result_count cannot be null/zero"); return ZVEC_ERROR_INVALID_ARGUMENT; } - *results = nullptr; - *result_count = 0; - ZVEC_TRY_RETURN_ERROR( "Exception in zvec_collection_insert_with_results", auto coll_ptr = reinterpret_cast *>(collection); - std::vector internal_docs = - convert_zvec_docs_to_internal(docs, doc_count); - std::vector pks = collect_doc_pks(docs, doc_count); + auto converted_docs = convert_zvec_docs_to_internal(docs, doc_count); + if (!converted_docs.has_value()) { + return handle_status(converted_docs.error()); + } + auto &internal_docs = converted_docs.value(); auto result = (*coll_ptr)->insert(internal_docs); zvec_error_code_t error_code = handle_expected_result(result); @@ -7030,9 +7023,11 @@ zvec_error_code_t zvec_collection_update(zvec_collection_t *collection, const zvec_doc_t **docs, size_t doc_count, size_t *success_count, size_t *error_count) { + if (success_count) *success_count = 0; + if (error_count) *error_count = doc_count; if (!collection || !docs || doc_count == 0 || !success_count || !error_count) { - set_last_error( + SET_LAST_ERROR(ZVEC_ERROR_INVALID_ARGUMENT, "Invalid arguments: collection, docs, doc_count, success_count and " "error_count cannot be null/zero"); return ZVEC_ERROR_INVALID_ARGUMENT; @@ -7043,8 +7038,11 @@ zvec_error_code_t zvec_collection_update(zvec_collection_t *collection, auto coll_ptr = reinterpret_cast *>(collection); - std::vector internal_docs = - convert_zvec_docs_to_internal(docs, doc_count); + auto converted_docs = convert_zvec_docs_to_internal(docs, doc_count); + if (!converted_docs.has_value()) { + return handle_status(converted_docs.error()); + } + auto &internal_docs = converted_docs.value(); auto result = (*coll_ptr)->update(internal_docs); zvec_error_code_t error_code = handle_expected_result(result); @@ -7069,24 +7067,25 @@ zvec_error_code_t zvec_collection_update_with_results(zvec_collection_t *collect size_t doc_count, zvec_write_result_t **results, size_t *result_count) { + if (results) *results = nullptr; + if (result_count) *result_count = 0; if (!collection || !docs || doc_count == 0 || !results || !result_count) { - set_last_error( + SET_LAST_ERROR(ZVEC_ERROR_INVALID_ARGUMENT, "Invalid arguments: collection, docs, doc_count, results and " "result_count cannot be null/zero"); return ZVEC_ERROR_INVALID_ARGUMENT; } - *results = nullptr; - *result_count = 0; - ZVEC_TRY_RETURN_ERROR( "Exception in zvec_collection_update_with_results", auto coll_ptr = reinterpret_cast *>(collection); - std::vector internal_docs = - convert_zvec_docs_to_internal(docs, doc_count); - std::vector pks = collect_doc_pks(docs, doc_count); + auto converted_docs = convert_zvec_docs_to_internal(docs, doc_count); + if (!converted_docs.has_value()) { + return handle_status(converted_docs.error()); + } + auto &internal_docs = converted_docs.value(); auto result = (*coll_ptr)->update(internal_docs); zvec_error_code_t error_code = handle_expected_result(result); @@ -7100,9 +7099,11 @@ zvec_error_code_t zvec_collection_upsert(zvec_collection_t *collection, const zvec_doc_t **docs, size_t doc_count, size_t *success_count, size_t *error_count) { + if (success_count) *success_count = 0; + if (error_count) *error_count = doc_count; if (!collection || !docs || doc_count == 0 || !success_count || !error_count) { - set_last_error( + SET_LAST_ERROR(ZVEC_ERROR_INVALID_ARGUMENT, "Invalid arguments: collection, docs, doc_count, success_count and " "error_count cannot be null/zero"); return ZVEC_ERROR_INVALID_ARGUMENT; @@ -7113,8 +7114,11 @@ zvec_error_code_t zvec_collection_upsert(zvec_collection_t *collection, auto coll_ptr = reinterpret_cast *>(collection); - std::vector internal_docs = - convert_zvec_docs_to_internal(docs, doc_count); + auto converted_docs = convert_zvec_docs_to_internal(docs, doc_count); + if (!converted_docs.has_value()) { + return handle_status(converted_docs.error()); + } + auto &internal_docs = converted_docs.value(); auto result = (*coll_ptr)->upsert(internal_docs); zvec_error_code_t error_code = handle_expected_result(result); @@ -7139,24 +7143,25 @@ zvec_error_code_t zvec_collection_upsert_with_results(zvec_collection_t *collect size_t doc_count, zvec_write_result_t **results, size_t *result_count) { + if (results) *results = nullptr; + if (result_count) *result_count = 0; if (!collection || !docs || doc_count == 0 || !results || !result_count) { - set_last_error( + SET_LAST_ERROR(ZVEC_ERROR_INVALID_ARGUMENT, "Invalid arguments: collection, docs, doc_count, results and " "result_count cannot be null/zero"); return ZVEC_ERROR_INVALID_ARGUMENT; } - *results = nullptr; - *result_count = 0; - ZVEC_TRY_RETURN_ERROR( "Exception in zvec_collection_upsert_with_results", auto coll_ptr = reinterpret_cast *>(collection); - std::vector internal_docs = - convert_zvec_docs_to_internal(docs, doc_count); - std::vector pks = collect_doc_pks(docs, doc_count); + auto converted_docs = convert_zvec_docs_to_internal(docs, doc_count); + if (!converted_docs.has_value()) { + return handle_status(converted_docs.error()); + } + auto &internal_docs = converted_docs.value(); auto result = (*coll_ptr)->upsert(internal_docs); zvec_error_code_t error_code = handle_expected_result(result); @@ -7259,8 +7264,7 @@ zvec_error_code_t zvec_collection_delete_by_filter(zvec_collection_t *collection reinterpret_cast *>(collection); auto status = (*coll_ptr)->delete_by_filter(filter); if (!status.ok()) { - set_last_error(status.message()); - return status_to_error_code(status); + return handle_status(status); } return ZVEC_OK;) } diff --git a/src/binding/python/model/python_doc.cc b/src/binding/python/model/python_doc.cc index ae03adaaf..48e5faadf 100644 --- a/src/binding/python/model/python_doc.cc +++ b/src/binding/python/model/python_doc.cc @@ -52,7 +52,7 @@ void ZVecPyDoc::bind_doc(py::module_ &m) { doc.def(py::init([]() { return std::make_shared(); })) .def("set_pk", &Doc::set_pk) - .def("pk", &Doc::pk) + .def("pk", &Doc::pk_ref) .def("set_score", &Doc::set_score) .def("score", &Doc::score) .def("has_field", &Doc::has) @@ -325,7 +325,7 @@ py::tuple ZVecPyDoc::doc_to_tuple_with_fields( const FieldSchemaPtrList &vector_fields) { py::tuple result(4); // 1. set doc id and score - result[0] = py::str(self.pk()); + result[0] = py::str(self.pk_ref()); result[1] = py::float_(self.score()); if (self.is_empty()) { @@ -372,4 +372,4 @@ py::tuple ZVecPyDoc::doc_to_tuple_with_fields( } return result; } -} // namespace zvec \ No newline at end of file +} // namespace zvec diff --git a/src/db/collection.cc b/src/db/collection.cc index c6782b11d..a96964494 100644 --- a/src/db/collection.cc +++ b/src/db/collection.cc @@ -42,9 +42,11 @@ #include "db/common/global_resource.h" #include "db/common/profiler.h" #include "db/common/typedef.h" +#include "db/common/utils.h" #include "db/doc_iterator_internal.h" #include "db/index/common/delete_store.h" #include "db/index/common/id_map.h" +#include "db/index/common/identifier_validation.h" #include "db/index/common/index_filter.h" #include "db/index/common/type_helper.h" #include "db/index/common/version_manager.h" @@ -1192,9 +1194,9 @@ Status CollectionImpl::validate(const std::string &column, if (field->data_type() < DataType::INT32 || field->data_type() > DataType::DOUBLE) { return Status::InvalidArgument( - "Only support basic numeric data type [int32, int64, uint32, uint64, " - "float, double]: ", - field->to_string()); + "Invalid schema: this operation requires a numeric field; field[", + field->name(), "] has type ", + DataTypeCodeBook::AsString(field->data_type())); } return Status::OK(); }; @@ -1202,15 +1204,13 @@ Status CollectionImpl::validate(const std::string &column, switch (op) { case ColumnOp::ADD: { if (schema == nullptr) { - return Status::InvalidArgument("Column schema is null"); + return Status::InvalidArgument( + "Invalid schema: field schema must not be null"); } - if (schema->name().empty()) { - return Status::InvalidArgument("Column name is empty"); - } if (schema_->has_field(schema->name())) { - return Status::InvalidArgument("column already exists: ", - schema->name()); + return Status::InvalidArgument("Invalid schema: field[", schema->name(), + "] already exists"); } auto s = schema->validate(); @@ -1219,26 +1219,34 @@ Status CollectionImpl::validate(const std::string &column, s = check_data_type(schema.get()); CHECK_RETURN_STATUS(s); - if (expression.empty() && !schema->nullable()) { + if (schema_->forward_fields().size() >= kMaxScalarFieldSize) { return Status::InvalidArgument( - "Add column is not supported for non-nullable column: ", - schema->name()); + "Invalid schema: cannot add field; collection already has ", + kMaxScalarFieldSize, " scalar fields"); + } + + if (expression.empty() && !schema->nullable()) { + return Status::InvalidArgument("Invalid schema: non-nullable field[", + schema->name(), + "] requires an expression when added"); } break; } case ColumnOp::ALTER: { if (column.empty()) { - return Status::InvalidArgument("column name is empty"); + return Status::InvalidArgument( + "Invalid schema: field name must not be empty"); } if (!schema_->has_field(column)) { - return Status::InvalidArgument("column ", column, " not found"); + return Status::InvalidArgument("Invalid schema: field[", + format_name(column), "] not found"); } if (!rename.empty() && schema) { return Status::InvalidArgument( - "cannot specify both rename and new column schema"); + "Invalid schema: cannot specify both rename and new column schema"); } auto *old_field_schema = schema_->get_field(column); @@ -1247,27 +1255,25 @@ Status CollectionImpl::validate(const std::string &column, if (!rename.empty()) { // rename case + s = validate_field_name(rename); + CHECK_RETURN_STATUS(s); if (schema_->has_field(rename)) { - return Status::InvalidArgument("new column name ", rename, - " already exists"); + return Status::InvalidArgument("Invalid schema: field[", rename, + "] already exists"); } } else { // schema change case if (!schema) { - return Status::InvalidArgument("New column schema is null"); + return Status::InvalidArgument( + "Invalid schema: field schema must not be null"); } s = schema->validate(); CHECK_RETURN_STATUS(s); - if (schema->name().empty()) { - return Status::InvalidArgument("new column schema name is empty"); - } - if (!schema->nullable() && old_field_schema->nullable()) { return Status::InvalidArgument( - "new column schema is not nullable, but old column schema is " - "nullable"); + "Invalid schema: cannot make a nullable field non-nullable"); } if (*old_field_schema == *schema) { @@ -1283,7 +1289,13 @@ Status CollectionImpl::validate(const std::string &column, } case ColumnOp::DROP: { if (!schema_->has_field(column)) { - return Status::InvalidArgument("Column not exists: ", column); + return Status::InvalidArgument("Invalid schema: field[", + format_name(column), "] not found"); + } + + if (schema_->fields().size() <= 1) { + return Status::InvalidArgument( + "Invalid schema: cannot drop the last field in a collection"); } auto *old_field_schema = schema_->get_field(column); @@ -1311,15 +1323,16 @@ Status CollectionImpl::add_column(const FieldSchema::Ptr &column_schema, CHECK_DESTROY_RETURN_STATUS(destroyed_, false); CHECK_CLOSED_RETURN_STATUS(closed_, false); - // validate - auto s = validate("", column_schema, expression, "", ColumnOp::ADD); + auto field_copy = + column_schema ? std::make_shared(*column_schema) : nullptr; + auto s = validate("", field_copy, expression, "", ColumnOp::ADD); CHECK_RETURN_STATUS(s); // forbidden writing until index is ready std::lock_guard write_lock(write_mtx_); auto new_schema = std::make_shared(*schema_); - s = new_schema->add_field(column_schema); + s = new_schema->add_field(field_copy); CHECK_RETURN_STATUS(s); if (writing_segment_->has_record()) { @@ -1330,7 +1343,7 @@ Status CollectionImpl::add_column(const FieldSchema::Ptr &column_schema, Version new_version = version_manager_->get_current_version(); // add column on segment manager - s = segment_manager_->add_column(column_schema, expression, + s = segment_manager_->add_column(field_copy, expression, options.concurrency_); CHECK_RETURN_STATUS(s); @@ -1465,25 +1478,23 @@ Status CollectionImpl::alter_column(const std::string &column_name, CHECK_DESTROY_RETURN_STATUS(destroyed_, false); CHECK_CLOSED_RETURN_STATUS(closed_, false); - // validate - auto s = - validate(column_name, new_column_schema, "", rename, ColumnOp::ALTER); + auto field_copy = new_column_schema + ? std::make_shared(*new_column_schema) + : nullptr; + auto s = validate(column_name, field_copy, "", rename, ColumnOp::ALTER); CHECK_RETURN_STATUS(s); // forbidden writing until index is ready std::lock_guard write_lock(write_mtx_); - std::shared_ptr new_field_schema{nullptr}; if (!rename.empty()) { - new_field_schema = + field_copy = std::make_shared(*schema_->get_field(column_name)); - new_field_schema->set_name(rename); - } else { - new_field_schema = std::make_shared(*new_column_schema); + field_copy->set_name(rename); } auto new_schema = std::make_shared(*schema_); - s = new_schema->alter_field(column_name, new_field_schema); + s = new_schema->alter_field(column_name, field_copy); CHECK_RETURN_STATUS(s); if (writing_segment_->has_record()) { @@ -1494,7 +1505,7 @@ Status CollectionImpl::alter_column(const std::string &column_name, Version new_version = version_manager_->get_current_version(); // alter column on segment manager - s = segment_manager_->alter_column(column_name, new_field_schema, + s = segment_manager_->alter_column(column_name, field_copy, options.concurrency_); CHECK_RETURN_STATUS(s); @@ -1557,7 +1568,7 @@ Status CollectionImpl::internal_fetch_by_doc(const Doc &doc, // Called from handle_update(), i.e. under write_impl()'s write_mtx_. auto segments = get_all_segments_unsafe(); uint64_t doc_id; - bool has = id_map_->has(doc.pk(), &doc_id); + bool has = id_map_->has(doc.pk_ref(), &doc_id); if (!has) { return Status::NotFound("Document not found"); } diff --git a/src/db/common/CMakeLists.txt b/src/db/common/CMakeLists.txt index cc2110aa3..089fd515f 100644 --- a/src/db/common/CMakeLists.txt +++ b/src/db/common/CMakeLists.txt @@ -9,6 +9,7 @@ cc_library( zvec_ailego roaring rocksdb + utf8proc INCS . VERSION "${PROXIMA_ZVEC_VERSION}" ) diff --git a/src/db/common/constants.h b/src/db/common/constants.h index 3aa0512a5..2e023264a 100644 --- a/src/db/common/constants.h +++ b/src/db/common/constants.h @@ -14,7 +14,6 @@ #pragma once #include -#include #include namespace zvec { @@ -32,6 +31,10 @@ const std::string GLOBAL_DOC_ID = "_zvec_g_doc_id_"; const std::string USER_ID = "_zvec_uid_"; +const std::string FIELD_SCORE = "_zvec_score"; + +const std::string FIELD_GROUP_ID = "_zvec_group_id"; + const int kSparseMaxDimSize = 16384; const int64_t kMaxRecordBatchNumRows = 4096; @@ -40,12 +43,6 @@ constexpr uint32_t MAX_ARRAY_FIELD_LEN = 32; const float COMPACT_DELETE_RATIO_THRESHOLD = 0.3f; -const std::regex COLLECTION_NAME_REGEX("^[a-zA-Z0-9_-]{3,64}$"); - -const std::regex FIELD_NAME_REGEX("^[a-zA-Z0-9_-]{1,32}$"); - -const std::regex DOC_PK_REGEX("^[a-zA-Z0-9_!@#$%+=.-]{1,64}$"); - constexpr uint32_t kMaxDenseDimSize = 20000; constexpr uint32_t kMaxScalarFieldSize = 1024; diff --git a/src/db/common/utils.cc b/src/db/common/utils.cc index a1b83a114..6524e5707 100644 --- a/src/db/common/utils.cc +++ b/src/db/common/utils.cc @@ -13,6 +13,9 @@ // limitations under the License. #include "utils.h" +#include +#include + namespace zvec { @@ -20,4 +23,75 @@ std::string indent(int level) { return std::string(level * 2, ' '); } -} // namespace zvec \ No newline at end of file +std::string format_name(std::string_view value) { + constexpr size_t kMaxPreviewBytes = 32; + constexpr char kHexDigits[] = "0123456789ABCDEF"; + auto length = std::min(value.size(), kMaxPreviewBytes); + std::string preview; + preview.reserve(length); + size_t i = 0; + while (i < length) { + auto byte = static_cast(value[i]); + if (byte >= 0x80) { + utf8proc_int32_t codepoint; + auto bytes = utf8proc_iterate( + reinterpret_cast(value.data() + i), + static_cast(std::min(value.size() - i, size_t{4})), + &codepoint); + if (bytes > 0) { + auto codepoint_bytes = static_cast(bytes); + if (i + codepoint_bytes > length) { + break; + } + switch (utf8proc_category(codepoint)) { + case UTF8PROC_CATEGORY_CC: + case UTF8PROC_CATEGORY_CF: + case UTF8PROC_CATEGORY_CN: + case UTF8PROC_CATEGORY_ZL: + case UTF8PROC_CATEGORY_ZP: + break; + default: + preview.append(value.data() + i, codepoint_bytes); + i += codepoint_bytes; + continue; + } + } + } + switch (byte) { + case '\0': + preview += "\\0"; + break; + case '\n': + preview += "\\n"; + break; + case '\r': + preview += "\\r"; + break; + case '\t': + preview += "\\t"; + break; + case '\\': + case '[': + case ']': + preview += '\\'; + preview += static_cast(byte); + break; + default: + if (byte >= 0x20 && byte <= 0x7E) { + preview += static_cast(byte); + } else { + preview += "\\x"; + preview += kHexDigits[byte >> 4]; + preview += kHexDigits[byte & 0x0F]; + } + break; + } + ++i; + } + if (i < value.size()) { + preview += "..."; + } + return preview; +} + +} // namespace zvec diff --git a/src/db/common/utils.h b/src/db/common/utils.h index c238b928d..8baab3826 100644 --- a/src/db/common/utils.h +++ b/src/db/common/utils.h @@ -14,8 +14,14 @@ #pragma once #include +#include + namespace zvec { + std::string indent(int level); -} // namespace zvec \ No newline at end of file +// Format a name for clearer display. +std::string format_name(std::string_view value); + +} // namespace zvec diff --git a/src/db/index/common/doc.cc b/src/db/index/common/doc.cc index f76ad1d86..48742ba2f 100644 --- a/src/db/index/common/doc.cc +++ b/src/db/index/common/doc.cc @@ -12,18 +12,17 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include #include #include #include #include -#include -#include #include #include #include #include #include "db/common/constants.h" +#include "db/common/utils.h" +#include "db/index/common/identifier_validation.h" #include "db/index/common/type_helper.h" #if defined(__BYTE_ORDER__) && __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ @@ -732,20 +731,15 @@ Status Doc::validate_and_sanitize(const CollectionSchema::Ptr &schema, return Status::InternalError("schema is null during doc validation"); } - if (pk_.empty()) { - return Status::InvalidArgument("Invalid doc: id (primary key) is not set"); - } - - if (!std::regex_match(pk_, DOC_PK_REGEX)) { - return Status::InvalidArgument("Invalid doc: doc[", pk_, - "] contains invalid characters"); + if (auto s = validate_document_id(pk_); !s.ok()) { + return s; } // check doc fields match schema for (auto &[name, value] : fields_) { if (!schema->has_field(name)) { return Status::InvalidArgument( - "Invalid doc[", pk_, "]: field[", name, + "Invalid doc[", format_name(pk_), "]: field[", format_name(name), "] does not exist in the collection schema"); } } @@ -758,16 +752,16 @@ Status Doc::validate_and_sanitize(const CollectionSchema::Ptr &schema, if (field_schema->nullable() || is_update) { continue; } - return Status::InvalidArgument("Invalid doc[", pk_, "]: field[", - field_name, + return Status::InvalidArgument("Invalid doc[", format_name(pk_), + "]: field[", field_name, "] is required but not provided"); } else { if (std::holds_alternative(field_pair->second)) { if (field_schema->nullable()) { continue; } - return Status::InvalidArgument("Invalid doc[", pk_, "]: field[", - field_name, + return Status::InvalidArgument("Invalid doc[", format_name(pk_), + "]: field[", field_name, "] is required but its value is null"); } } @@ -898,21 +892,21 @@ Status Doc::validate_and_sanitize(const CollectionSchema::Ptr &schema, field_value); if (sparse_values.size() != sparse_indices.size()) { return Status::InvalidArgument( - "Invalid doc[", pk_, "]: sparse vector field[", field_name, - "] has mismatched indices and values sizes"); + "Invalid doc[", format_name(pk_), "]: sparse vector field[", + field_name, "] has mismatched indices and values sizes"); } if (sparse_indices.size() > kSparseMaxDimSize) { return Status::InvalidArgument( - "Invalid doc[", pk_, "]: sparse vector field[", field_name, - "] exceeds the maximum number of sparse indices (", + "Invalid doc[", format_name(pk_), "]: sparse vector field[", + field_name, "] exceeds the maximum number of sparse indices (", kSparseMaxDimSize, ")"); } auto status = need_sanitize_sparse(sparse_indices.data(), sparse_indices.size()); if (status == SparseIndicesStatus::kHasDuplicate) { return Status::InvalidArgument( - "Invalid doc[", pk_, "]: sparse vector field[", field_name, - "] contains duplicate indices"); + "Invalid doc[", format_name(pk_), "]: sparse vector field[", + field_name, "] contains duplicate indices"); } if (status == SparseIndicesStatus::kNeedSort) { if (sort_and_find_duplicates( @@ -920,8 +914,8 @@ Status Doc::validate_and_sanitize(const CollectionSchema::Ptr &schema, reinterpret_cast(sparse_values.data()), sparse_indices.size(), sizeof(float16_t))) { return Status::InvalidArgument( - "Invalid doc[", pk_, "]: sparse vector field[", field_name, - "] contains duplicate indices"); + "Invalid doc[", format_name(pk_), "]: sparse vector field[", + field_name, "] contains duplicate indices"); } } } @@ -936,21 +930,21 @@ Status Doc::validate_and_sanitize(const CollectionSchema::Ptr &schema, field_value); if (sparse_values.size() != sparse_indices.size()) { return Status::InvalidArgument( - "Invalid doc[", pk_, "]: sparse vector field[", field_name, - "] has mismatched indices and values sizes"); + "Invalid doc[", format_name(pk_), "]: sparse vector field[", + field_name, "] has mismatched indices and values sizes"); } if (sparse_indices.size() > kSparseMaxDimSize) { return Status::InvalidArgument( - "Invalid doc[", pk_, "]: sparse vector field[", field_name, - "] exceeds the maximum number of sparse indices (", + "Invalid doc[", format_name(pk_), "]: sparse vector field[", + field_name, "] exceeds the maximum number of sparse indices (", kSparseMaxDimSize, ")"); } auto status = need_sanitize_sparse(sparse_indices.data(), sparse_indices.size()); if (status == SparseIndicesStatus::kHasDuplicate) { return Status::InvalidArgument( - "Invalid doc[", pk_, "]: sparse vector field[", field_name, - "] contains duplicate indices"); + "Invalid doc[", format_name(pk_), "]: sparse vector field[", + field_name, "] contains duplicate indices"); } if (status == SparseIndicesStatus::kNeedSort) { if (sort_and_find_duplicates( @@ -958,23 +952,23 @@ Status Doc::validate_and_sanitize(const CollectionSchema::Ptr &schema, reinterpret_cast(sparse_values.data()), sparse_indices.size(), sizeof(float))) { return Status::InvalidArgument( - "Invalid doc[", pk_, "]: sparse vector field[", field_name, - "] contains duplicate indices"); + "Invalid doc[", format_name(pk_), "]: sparse vector field[", + field_name, "] contains duplicate indices"); } } } break; } default: - return Status::InvalidArgument("Invalid doc[", pk_, "]: field[", - field_name, + return Status::InvalidArgument("Invalid doc[", format_name(pk_), + "]: field[", field_name, "] has unsupported data type"); break; } if (!type_match) { return Status::InvalidArgument( - "Invalid doc[", pk_, "]: field[", field_name, + "Invalid doc[", format_name(pk_), "]: field[", field_name, "] type mismatch, expected ", DataTypeCodeBook::AsString(expected_type), " but got ", get_value_type_name(field_value, field_schema->is_vector_field())); @@ -982,7 +976,7 @@ Status Doc::validate_and_sanitize(const CollectionSchema::Ptr &schema, if (field_schema->is_dense_vector()) { if (value_dimension != field_schema->dimension()) { return Status::InvalidArgument( - "Invalid doc[", pk_, "]: field[", field_name, + "Invalid doc[", format_name(pk_), "]: field[", field_name, "] dimension mismatch, expected ", field_schema->dimension(), " but got ", value_dimension); } diff --git a/src/db/index/common/identifier_validation.cc b/src/db/index/common/identifier_validation.cc new file mode 100644 index 000000000..0d06120be --- /dev/null +++ b/src/db/index/common/identifier_validation.cc @@ -0,0 +1,132 @@ +// Copyright 2025-present the zvec project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "identifier_validation.h" +#include +#include +#include +#include "db/common/constants.h" +#include "db/common/utils.h" + + +namespace zvec { + + +namespace { + +const char *forbidden_codepoint_reason(utf8proc_int32_t codepoint) { + if (codepoint == 0) { + return "contains a null character"; + } + if (codepoint == '\n' || codepoint == '\r') { + return "contains a newline"; + } + if (codepoint == '\t') { + return "contains a tab"; + } + if (codepoint <= 0x1F || (codepoint >= 0x7F && codepoint <= 0x9F)) { + return "contains a control character"; + } + if (codepoint == 0x2028) { + return "contains a line separator"; + } + if (codepoint == 0x2029) { + return "contains a paragraph separator"; + } + return nullptr; +} + +Status validate_utf8_name(std::string_view value, size_t max_bytes, + const char *prefix) { + if (value.empty()) { + return Status::InvalidArgument(prefix, " must not be empty"); + } + if (value.size() > max_bytes) { + return Status::InvalidArgument(prefix, "[", format_name(value), + "] exceeds ", max_bytes, " bytes (got ", + value.size(), ")"); + } + + const auto *data = reinterpret_cast(value.data()); + size_t position = 0; + while (position < value.size()) { + utf8proc_int32_t codepoint; + auto bytes = utf8proc_iterate( + data + position, static_cast(value.size() - position), + &codepoint); + if (bytes <= 0) { + return Status::InvalidArgument(prefix, "[", format_name(value), + "] is not valid UTF-8"); + } + if (const char *reason = forbidden_codepoint_reason(codepoint)) { + return Status::InvalidArgument(prefix, "[", format_name(value), "] ", + reason); + } + position += static_cast(bytes); + } + return Status::OK(); +} + +bool is_reserved_field_name(std::string_view name) { + static const std::array reserved_names{ + LOCAL_ROW_ID, GLOBAL_DOC_ID, USER_ID, FIELD_SCORE, FIELD_GROUP_ID}; + return std::find(reserved_names.begin(), reserved_names.end(), name) != + reserved_names.end(); +} + +} // namespace + + +Status validate_document_id(std::string_view id) { + return validate_utf8_name(id, kMaxDocumentIdBytes, "Invalid doc: id"); +} + +Status validate_collection_name(std::string_view name) { + return validate_utf8_name(name, kMaxCollectionNameBytes, + "Invalid schema: collection name"); +} + +Status validate_field_name(std::string_view name) { + if (name.empty()) { + return Status::InvalidArgument( + "Invalid schema: field name must not be empty"); + } + if (name.size() > kMaxFieldNameBytes) { + return Status::InvalidArgument("Invalid schema: field[", format_name(name), + "] exceeds ", kMaxFieldNameBytes, + " bytes (got ", name.size(), ")"); + } + for (unsigned char byte : name) { + if ((byte >= 'A' && byte <= 'Z') || (byte >= 'a' && byte <= 'z') || + (byte >= '0' && byte <= '9') || byte == '_' || byte == '-') { + continue; + } + const char *reason = byte >= 0x80 ? "contains a non-ASCII character" + : forbidden_codepoint_reason(byte); + if (!reason) { + reason = byte == ' ' ? "contains a space" + : "contains an unsupported character"; + } + return Status::InvalidArgument( + "Invalid schema: field[", format_name(name), "] ", reason, + "; use letters (A-Z, a-z), digits, underscores (_) or hyphens (-)"); + } + if (is_reserved_field_name(name)) { + return Status::InvalidArgument("Invalid schema: field[", format_name(name), + "] is reserved; use a different name"); + } + return Status::OK(); +} + +} // namespace zvec diff --git a/src/db/index/common/identifier_validation.h b/src/db/index/common/identifier_validation.h new file mode 100644 index 000000000..0342550d1 --- /dev/null +++ b/src/db/index/common/identifier_validation.h @@ -0,0 +1,31 @@ +// Copyright 2025-present the zvec project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#pragma once + +#include +#include +#include + +namespace zvec { + +inline constexpr size_t kMaxDocumentIdBytes = 1024; +inline constexpr size_t kMaxCollectionNameBytes = 256; +inline constexpr size_t kMaxFieldNameBytes = 64; + +Status validate_document_id(std::string_view id); +Status validate_collection_name(std::string_view name); +Status validate_field_name(std::string_view name); + +} // namespace zvec diff --git a/src/db/index/common/schema.cc b/src/db/index/common/schema.cc index cbd8f1c67..dab3b402c 100644 --- a/src/db/index/common/schema.cc +++ b/src/db/index/common/schema.cc @@ -12,7 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include #include #include #include @@ -26,6 +25,7 @@ #include "db/common/utils.h" #include "db/index/column/fts_column/fts_types.h" #include "db/index/column/fts_column/tokenizer/tokenizer_factory.h" +#include "db/index/common/identifier_validation.h" #include "db/index/common/type_helper.h" namespace zvec { @@ -62,7 +62,7 @@ static Status validate_fts_index_params(const FieldSchema &field) { auto params = std::dynamic_pointer_cast(field.index_params()); if (!params) { return Status::InvalidArgument( - "schema validate failed: FTS index requires FtsIndexParams, but field[", + "Invalid schema: FTS index requires FtsIndexParams, but field[", field.name(), "] has incompatible index params"); } @@ -74,30 +74,25 @@ static Status validate_fts_index_params(const FieldSchema &field) { auto pipeline = fts::TokenizerFactory::create(internal_params); if (!pipeline.has_value()) { return Status::InvalidArgument( - "schema validate failed: invalid FTS index params for field[", - field.name(), "]: ", pipeline.error().message()); + "Invalid schema: invalid FTS index params for field[", field.name(), + "]: ", pipeline.error().message()); } return Status::OK(); } Status FieldSchema::validate() const { + if (auto s = validate_field_name(name_); !s.ok()) { + return s; + } + if (data_type_ == DataType::UNDEFINED) { - return Status::InvalidArgument("schema validate failed: field[", name_, + return Status::InvalidArgument("Invalid schema: field[", name_, "]'s data_type is not defined"); } - if (name_.empty()) { - return Status::InvalidArgument("schema validate failed: field[", name_, - "]'s name is empty"); - } - if (!std::regex_match(name_, FIELD_NAME_REGEX)) { - return Status::InvalidArgument( - "schema validate failed: field[", name_, - "]'s name cannot pass the regex verification"); - } if (is_vector_field()) { auto is_sparse = is_sparse_vector(); if (!is_sparse && (dimension_ == 0 || dimension() > kMaxDenseDimSize)) { - return Status::InvalidArgument("schema validate failed: field[", name_, + return Status::InvalidArgument("Invalid schema: field[", name_, "]'s dimension must be in (0,20000]"); } @@ -105,7 +100,7 @@ Status FieldSchema::validate() const { if (support_dense_vector_type.find(data_type_) == support_dense_vector_type.end()) { return Status::InvalidArgument( - "schema validate failed: dense_vector's data type only " + "Invalid schema: dense_vector's data type only " "support FP32, " "but field[", name_, "]'s data type is ", DataTypeCodeBook::AsString(data_type_)); @@ -114,7 +109,7 @@ Status FieldSchema::validate() const { if (support_sparse_vector_type.find(data_type_) == support_sparse_vector_type.end()) { return Status::InvalidArgument( - "schema validate failed: sparse_vector's data type only " + "Invalid schema: sparse_vector's data type only " "support FP32, " "but field[", name_, "]'s data type is ", DataTypeCodeBook::AsString(data_type_)); @@ -129,7 +124,7 @@ Status FieldSchema::validate() const { if (support_sparse_vector_index.find(index_params_->type()) == support_sparse_vector_index.end()) { return Status::InvalidArgument( - "schema validate failed: sparse_vector's index_params only " + "Invalid schema: sparse_vector's index_params only " "support FLAT|HNSW index, " "but field[", name_, "]'s index_type is ", @@ -137,7 +132,7 @@ Status FieldSchema::validate() const { } if (vector_index_params->metric_type() != MetricType::IP) { return Status::InvalidArgument( - "schema validate failed: sparse_vector's index_params only " + "Invalid schema: sparse_vector's index_params only " "support IP metric, but " "field[", name_, "]'s metric is ", @@ -148,7 +143,7 @@ Status FieldSchema::validate() const { if (support_dense_vector_index.find(index_params_->type()) == support_dense_vector_index.end()) { return Status::InvalidArgument( - "schema validate failed: dense_vector's index_params only " + "Invalid schema: dense_vector's index_params only " "support FLAT|HNSW|HNSW_RABITQ|IVF|IVF_RABITQ|DISKANN|VAMANA " "index, but " "field[", @@ -161,20 +156,20 @@ Status FieldSchema::validate() const { index_params_->type() == IndexType::IVF_RABITQ) { if (dimension_ < kMinRabitqDimSize || dimension_ > kMaxRabitqDimSize) { return Status::InvalidArgument( - "schema validate failed: RabitQ index only support " + "Invalid schema: RabitQ index only support " "dimension in [", kMinRabitqDimSize, ", ", kMaxRabitqDimSize, "]"); } if (data_type_ != DataType::VECTOR_FP32) { return Status::InvalidArgument( - "schema validate failed: RabitQ index only support FP32 " + "Invalid schema: RabitQ index only support FP32 " "data types"); } auto metric_type = vector_index_params->metric_type(); if (metric_type != MetricType::L2 && metric_type != MetricType::IP && metric_type != MetricType::COSINE) { return Status::InvalidArgument( - "schema validate failed: RabitQ index only support " + "Invalid schema: RabitQ index only support " "L2/IP/COSINE metric"); } #if !RABITQ_SUPPORTED @@ -196,17 +191,17 @@ Status FieldSchema::validate() const { std::dynamic_pointer_cast(index_params_); if (!ivf_rabitq_params) { return Status::InvalidArgument( - "schema validate failed: IVF_RABITQ index requires " + "Invalid schema: IVF_RABITQ index requires " "IvfRabitqIndexParams"); } if (ivf_rabitq_params->nlist() <= 0) { return Status::InvalidArgument( - "schema validate failed: IVF_RABITQ nlist must be greater than " + "Invalid schema: IVF_RABITQ nlist must be greater than " "0"); } if (ivf_rabitq_params->sample_count() < 0) { return Status::InvalidArgument( - "schema validate failed: IVF_RABITQ sample_count must be " + "Invalid schema: IVF_RABITQ sample_count must be " "greater than or equal to 0"); } } @@ -214,7 +209,7 @@ Status FieldSchema::validate() const { if (index_params_->type() == IndexType::IVF && vector_index_params->quantize_type() == QuantizeType::RABITQ) { return Status::InvalidArgument( - "schema validate failed: IVF index does not support RABITQ " + "Invalid schema: IVF index does not support RABITQ " "quantization; use the dedicated IVF_RABITQ index instead"); } @@ -246,20 +241,20 @@ Status FieldSchema::validate() const { flat_data_type != DataType::VECTOR_FP16 && flat_data_type != DataType::VECTOR_UINT8) { return Status::InvalidArgument( - "schema validate failed: field[", name_, + "Invalid schema: field[", name_, "]'s flat_data_type must be VECTOR_FP32, VECTOR_FP16, " "or VECTOR_UINT8, but got ", DataTypeCodeBook::AsString(flat_data_type)); } if (is_sparse && flat_data_type != DataType::VECTOR_FP32) { return Status::InvalidArgument( - "schema validate failed: non-FP32 flat_data_type is only " + "Invalid schema: non-FP32 flat_data_type is only " "supported for dense vector fields"); } if (!is_sparse && flat_data_type == DataType::VECTOR_UINT8 && vector_index_params->metric_type() != MetricType::L2) { return Status::InvalidArgument( - "schema validate failed: field[", name_, + "Invalid schema: field[", name_, "] can only use VECTOR_UINT8 Flat reference storage with L2 " "metric"); } @@ -278,8 +273,7 @@ Status FieldSchema::validate() const { index_params_->type() == IndexType::IVF || index_params_->type() == IndexType::DISKANN)) { return Status::InvalidArgument( - "schema validate failed: ", - QuantizeTypeCodeBook::AsString(quantize_type), + "Invalid schema: ", QuantizeTypeCodeBook::AsString(quantize_type), " quantization is not supported with ", IndexTypeCodeBook::AsString(index_params_->type()), " index, field[", name_, "]"); @@ -287,8 +281,7 @@ Status FieldSchema::validate() const { if (is_uniform && vector_index_params->metric_type() != MetricType::L2) { return Status::InvalidArgument( - "schema validate failed: ", - QuantizeTypeCodeBook::AsString(quantize_type), + "Invalid schema: ", QuantizeTypeCodeBook::AsString(quantize_type), " quantize only supports L2 metric, but field[", name_, "]'s metric is ", MetricTypeCodeBook::AsString(vector_index_params->metric_type())); @@ -296,8 +289,7 @@ Status FieldSchema::validate() const { auto iter = quantize_type_map.find(data_type_); if (iter == quantize_type_map.end()) { return Status::InvalidArgument( - "schema validate failed: ", - is_sparse ? "sparse_vector" : "dense_vector", + "Invalid schema: ", is_sparse ? "sparse_vector" : "dense_vector", "'s index_params of ", DataTypeCodeBook::AsString(data_type_), " do not support quantize, but field[", name_, "]'s quantize_type is ", @@ -307,7 +299,7 @@ Status FieldSchema::validate() const { if (iter->second.find(vector_index_params->quantize_type()) == iter->second.end()) { return Status::InvalidArgument( - "schema validate failed: ", + "Invalid schema: ", is_sparse ? "sparse_vector" : "dense_vector", "'s index_params of ", DataTypeCodeBook::AsString(data_type_), " support ", QuantizeTypeCodeBook::AsString(iter->second), @@ -322,7 +314,7 @@ Status FieldSchema::validate() const { if (data_type_ != DataType::VECTOR_FP16 && data_type_ != DataType::VECTOR_FP32) { return Status::InvalidArgument( - "schema validate failed: IVF index only support FP32/FP16 data " + "Invalid schema: IVF index only support FP32/FP16 data " "types according to the IP metric"); } } @@ -330,7 +322,7 @@ Status FieldSchema::validate() const { if (data_type_ != DataType::VECTOR_FP16 && data_type_ != DataType::VECTOR_FP32) { return Status::InvalidArgument( - "schema validate failed: cosine metric only supports FP32/FP16 " + "Invalid schema: cosine metric only supports FP32/FP16 " "data types, but field[", name_, "]'s data type is ", DataTypeCodeBook::AsString(data_type_)); @@ -341,14 +333,14 @@ Status FieldSchema::validate() const { if (index_params_) { if (index_params_->is_vector_index_type()) { return Status::InvalidArgument( - "schema validate failed: scalar field[", name_, + "Invalid schema: scalar field[", name_, "] does not support vector index params, but got index_type ", IndexTypeCodeBook::AsString(index_params_->type())); } if (index_params_->type() == IndexType::FTS && data_type_ != DataType::STRING) { return Status::InvalidArgument( - "schema validate failed: FTS index only supports STRING data type, " + "Invalid schema: FTS index only supports STRING data type, " "but field[", name_, "]'s data_type is ", DataTypeCodeBook::AsString(data_type_)); } @@ -412,32 +404,39 @@ std::string FieldSchema::to_string_formatted(int indent_level) const { } Status CollectionSchema::validate() const { - if (name_.empty()) { - return Status::InvalidArgument("schema validate failed: name is empty"); + if (auto s = validate_collection_name(name_); !s.ok()) { + return s; } - if (!std::regex_match(name_, COLLECTION_NAME_REGEX)) { - return Status::InvalidArgument( - "schema validate failed: collection[", name_, - "]'s name cannot pass the regex verification"); + std::unordered_set names; + for (const auto &field : fields_) { + if (!field) { + return Status::InvalidArgument( + "Invalid schema: field schema must not be null"); + } + if (!names.insert(field->name()).second) { + return Status::InvalidArgument("Invalid schema: duplicate field name [", + format_name(field->name()), + "]; field names must be unique"); + } } if (forward_fields().size() > kMaxScalarFieldSize) { return Status::InvalidArgument( - "schema validate failed: collection[", name_, + "Invalid schema: collection[", format_name(name_), "]'s field size must <= ", kMaxScalarFieldSize); } if (max_doc_count_per_segment_ < MAX_DOC_COUNT_PER_SEGMENT_MIN_THRESHOLD) { return Status::InvalidArgument( - "schema validate failed: max_doc_count_per_segment must >= ", + "Invalid schema: max_doc_count_per_segment must >= ", MAX_DOC_COUNT_PER_SEGMENT_MIN_THRESHOLD); } if (fields_.empty()) { - return Status::InvalidArgument("schema validate failed: collection[", name_, - "] has no fields"); + return Status::InvalidArgument("Invalid schema: collection[", + format_name(name_), "] has no fields"); } auto v_fields = vector_fields(); if (v_fields.size() > kMaxVectorFieldSize) { return Status::InvalidArgument( - "schema validate failed: collection[", name_, + "Invalid schema: collection[", format_name(name_), "]'s vector field size must <= ", kMaxVectorFieldSize); } for (auto &field : fields_) { @@ -485,6 +484,10 @@ std::string CollectionSchema::to_string_formatted(int indent_level) const { } Status CollectionSchema::add_field(FieldSchema::Ptr column_schema) { + if (!column_schema) { + return Status::InvalidArgument( + "Invalid schema: field schema must not be null"); + } // Check if field already exists if (has_field(column_schema->name())) { return Status::AlreadyExists("field[", column_schema->name(), @@ -507,9 +510,14 @@ Status CollectionSchema::add_field(FieldSchema::Ptr column_schema) { Status CollectionSchema::alter_field( const std::string &column_name, const FieldSchema::Ptr &new_column_options) { + if (!new_column_options) { + return Status::InvalidArgument( + "Invalid schema: field schema must not be null"); + } // Check if field exists if (!has_field(column_name)) { - return Status::NotFound("field[", column_name, "] not found in schema"); + return Status::NotFound("field[", format_name(column_name), + "] not found in schema"); } std::string new_column_name = new_column_options->name(); @@ -540,7 +548,8 @@ Status CollectionSchema::alter_field( Status CollectionSchema::drop_field(const std::string &column_name) { // Check if field exists if (!has_field(column_name)) { - return Status::NotFound("field[", column_name, "] not found in schema"); + return Status::NotFound("field[", format_name(column_name), + "] not found in schema"); } // Remove from map @@ -723,7 +732,8 @@ Status CollectionSchema::add_index(const std::string &column, if (field) { field->set_index_params(index_params); } else { - return Status::NotFound("field[", column, "] not found in schema"); + return Status::NotFound("field[", format_name(column), + "] not found in schema"); } return Status::OK(); @@ -739,7 +749,8 @@ Status CollectionSchema::drop_index(const std::string &column) { field->set_index_params(nullptr); } } else { - return Status::NotFound("field[", column, "] not found in schema"); + return Status::NotFound("field[", format_name(column), + "] not found in schema"); } return Status::OK(); diff --git a/src/db/index/segment/segment.cc b/src/db/index/segment/segment.cc index 8d5fc4297..efc27f758 100644 --- a/src/db/index/segment/segment.cc +++ b/src/db/index/segment/segment.cc @@ -915,7 +915,7 @@ Status SegmentImpl::internal_insert(Doc &doc) { } // write idmap - auto s = id_map_->upsert(doc.pk(), g_doc_id); + auto s = id_map_->upsert(doc.pk_ref(), g_doc_id); CHECK_RETURN_STATUS(s); // write forward @@ -952,7 +952,7 @@ Status SegmentImpl::internal_update(Doc &doc) { Status SegmentImpl::internal_upsert(Doc &doc) { uint64_t g_doc_id; - bool exist = id_map_->has(doc.pk(), &g_doc_id); + bool exist = id_map_->has(doc.pk_ref(), &g_doc_id); if (exist) { delete_store_->mark_deleted(g_doc_id); } @@ -961,15 +961,15 @@ Status SegmentImpl::internal_upsert(Doc &doc) { Status SegmentImpl::internal_delete(const Doc &doc) { delete_store_->mark_deleted(doc.doc_id()); - id_map_->remove(doc.pk()); + id_map_->remove(doc.pk_ref()); return Status::OK(); } Status SegmentImpl::Insert(Doc &doc) { std::lock_guard lock(seg_mtx_); - if (id_map_ && id_map_->has(doc.pk())) { - return Status::AlreadyExists("insert failed: doc_id[", doc.pk(), + if (id_map_ && id_map_->has(doc.pk_ref())) { + return Status::AlreadyExists("insert failed: doc_id[", doc.pk_ref(), "] already exists in collection"); } @@ -985,8 +985,8 @@ Status SegmentImpl::Insert(Doc &doc) { Status SegmentImpl::Update(Doc &doc) { std::lock_guard lock(seg_mtx_); uint64_t g_doc_id; - if (!id_map_->has(doc.pk(), &g_doc_id)) { - return Status::NotFound("update failed: doc_id[", doc.pk(), + if (!id_map_->has(doc.pk_ref(), &g_doc_id)) { + return Status::NotFound("update failed: doc_id[", doc.pk_ref(), "] not found in collection"); } @@ -4430,10 +4430,10 @@ Status SegmentImpl::append_wal(const Doc &doc) { auto ret = wal_file_->append(std::string(buf.begin(), buf.end())); if (ret != 0) { LOG_ERROR("WAL append failed: segment[%d], pk[%s], operator[%d], ret[%d]", - id(), doc.pk().c_str(), static_cast(doc.get_operator()), + id(), doc.pk_ref().c_str(), static_cast(doc.get_operator()), ret); return Status::InternalError("Failed to append WAL: segment[", id(), - "], pk[", doc.pk(), "], operator[", + "], pk[", doc.pk_ref(), "], operator[", static_cast(doc.get_operator()), "], ret[", ret, "]"); } diff --git a/src/db/index/storage/memory_forward_store.cc b/src/db/index/storage/memory_forward_store.cc index 61e9f9666..a532a3c9b 100644 --- a/src/db/index/storage/memory_forward_store.cc +++ b/src/db/index/storage/memory_forward_store.cc @@ -105,7 +105,7 @@ arrow::Status MemForwardStore::append_doc_to_builder( // user id(pk) auto uid_builder = dynamic_cast(rb_builder->GetField(1)); - ARROW_RETURN_NOT_OK(uid_builder->Append(doc.pk())); + ARROW_RETURN_NOT_OK(uid_builder->Append(doc.pk_ref())); // other fields for (size_t idx = 2; idx < fields.size(); ++idx) { diff --git a/src/db/reranker/reranker.cc b/src/db/reranker/reranker.cc index 684130123..0eeacb12d 100644 --- a/src/db/reranker/reranker.cc +++ b/src/db/reranker/reranker.cc @@ -45,7 +45,7 @@ Result score_based_rerank(const ScoreFn &score_fn, const auto &docs = results[field_idx]; for (size_t rank = 0; rank < docs.size(); ++rank) { const auto &doc = docs[rank]; - const std::string &doc_id = doc->pk(); + const std::string &doc_id = doc->pk_ref(); auto rs = score_fn(static_cast(doc->score()), static_cast(rank), field_idx); if (!rs.has_value()) { diff --git a/src/db/sqlengine/common/util.h b/src/db/sqlengine/common/util.h index 84edde3cd..ef6979f87 100644 --- a/src/db/sqlengine/common/util.h +++ b/src/db/sqlengine/common/util.h @@ -19,12 +19,7 @@ namespace zvec::sqlengine { -static const constexpr char *kFieldScore = "_zvec_score"; -static const constexpr char *kFieldVector = "_zvec_vector"; -static const constexpr char *kFieldSparseIndices = "_zvec_sindices"; -static const constexpr char *kFieldSparseValues = "_zvec_svalues"; static const constexpr char *kFieldIsValid = "_zvec_is_valid"; -static const constexpr char *kFieldGroupId = "_zvec_group_id"; static const inline std::string kCheckNotFiltered = "check_not_filtered"; static const inline std::string kFetchVector = "fetch_vector"; diff --git a/src/db/sqlengine/planner/fts_recall_node.cc b/src/db/sqlengine/planner/fts_recall_node.cc index 45313d9e0..6a94ebeb3 100644 --- a/src/db/sqlengine/planner/fts_recall_node.cc +++ b/src/db/sqlengine/planner/fts_recall_node.cc @@ -32,7 +32,7 @@ FtsRecallNode::FtsRecallNode(Segment::Ptr segment, QueryInfo::Ptr query_info, auto table = segment_->fetch(fetched_columns_, std::vector{}); // Append BM25 score column so downstream fill_doc_score() surfaces it to // the Python Doc.score, matching the vector-recall path. - schema_ = Util::append_field(*table->schema(), kFieldScore, arrow::float32()); + schema_ = Util::append_field(*table->schema(), FIELD_SCORE, arrow::float32()); } arrow::AsyncGenerator> FtsRecallNode::gen() { @@ -94,7 +94,7 @@ arrow::AsyncGenerator> FtsRecallNode::gen() { } auto record_batch = std::move(batch.ValueUnsafe()); auto with_score = - record_batch->AddColumn(record_batch->num_columns(), kFieldScore, + record_batch->AddColumn(record_batch->num_columns(), FIELD_SCORE, score_array.MoveValueUnsafe()); if (!with_score.ok()) { return arrow::Future>::MakeFinished( diff --git a/src/db/sqlengine/planner/query_planner.cc b/src/db/sqlengine/planner/query_planner.cc index d539b76ae..4a6aee766 100644 --- a/src/db/sqlengine/planner/query_planner.cc +++ b/src/db/sqlengine/planner/query_planner.cc @@ -446,7 +446,7 @@ Result QueryPlanner::make_physical_plan( "order_by", {std::move(node)}, ac::OrderByNodeOptions{cp::Ordering{{cp::SortKey{ - kFieldScore, vector_is_reverse ? cp::SortOrder::Descending + FIELD_SCORE, vector_is_reverse ? cp::SortOrder::Descending : cp::SortOrder::Ascending}}}}}; } else if (has_fts) { // FTS uses BM25 where higher score = more relevant. Per-segment results @@ -455,7 +455,7 @@ Result QueryPlanner::make_physical_plan( node = ac::Declaration{"order_by", {std::move(node)}, ac::OrderByNodeOptions{cp::Ordering{{cp::SortKey{ - kFieldScore, cp::SortOrder::Descending}}}}}; + FIELD_SCORE, cp::SortOrder::Descending}}}}}; } // group by need to collect all docs diff --git a/src/db/sqlengine/planner/vector_recall_node.cc b/src/db/sqlengine/planner/vector_recall_node.cc index f58d02c1b..b1900b809 100644 --- a/src/db/sqlengine/planner/vector_recall_node.cc +++ b/src/db/sqlengine/planner/vector_recall_node.cc @@ -47,7 +47,7 @@ VectorRecallNode::VectorRecallNode(Segment::Ptr segment, : query_info_->get_selected_scalar_field_names()) { auto table = segment_->fetch(fetched_columns_, std::vector{}); schema_ = table->schema(); - schema_ = Util::append_field(*schema_, kFieldScore, arrow::float32()); + schema_ = Util::append_field(*schema_, FIELD_SCORE, arrow::float32()); if (query_info_->is_include_vector()) { for (auto &field : query_info_->selected_vector_fields()) { if (field.field_schema_ptr->is_dense_vector()) { @@ -60,7 +60,7 @@ VectorRecallNode::VectorRecallNode(Segment::Ptr segment, } } if (query_info_->group_by()) { - schema_ = Util::append_field(*schema_, kFieldGroupId, arrow::utf8()); + schema_ = Util::append_field(*schema_, FIELD_GROUP_ID, arrow::utf8()); } } @@ -245,7 +245,7 @@ VectorRecallNode::State::collect_batch() { auto record_batch = std::move(batch.ValueUnsafe()); ARROW_ASSIGN_OR_RAISE( record_batch, - record_batch->AddColumn(record_batch->num_columns(), kFieldScore, + record_batch->AddColumn(record_batch->num_columns(), FIELD_SCORE, score_array.MoveValueUnsafe())); if (self_->query_info_->is_include_vector()) { @@ -277,7 +277,7 @@ VectorRecallNode::State::collect_batch() { } ARROW_ASSIGN_OR_RAISE( record_batch, - record_batch->AddColumn(record_batch->num_columns(), kFieldGroupId, + record_batch->AddColumn(record_batch->num_columns(), FIELD_GROUP_ID, group_id_array.MoveValueUnsafe())); } diff --git a/src/db/sqlengine/sqlengine_impl.cc b/src/db/sqlengine/sqlengine_impl.cc index e74fbdc25..07530ca3f 100644 --- a/src/db/sqlengine/sqlengine_impl.cc +++ b/src/db/sqlengine/sqlengine_impl.cc @@ -521,7 +521,7 @@ Status record_batch_to_doc_list( doc_id_array != nullptr) { fill_doc_id(doc_id_array, doc_it); } - if (auto score_array = record_batch.GetColumnByName(kFieldScore); + if (auto score_array = record_batch.GetColumnByName(FIELD_SCORE); score_array != nullptr) { fill_doc_score(score_array, doc_it); } @@ -597,10 +597,10 @@ Result SQLEngineImpl::fill_group_by_result( if (!status.ok()) { return tl::make_unexpected(status); } - auto group_id_array = record_batch->GetColumnByName(kFieldGroupId); + auto group_id_array = record_batch->GetColumnByName(FIELD_GROUP_ID); if (!group_id_array) { return tl::make_unexpected(Status::InternalError( - "Column not found in record batch: [", kFieldGroupId, "]")); + "Column not found in record batch: [", FIELD_GROUP_ID, "]")); } arrow::StringArray *typed_arr = static_cast(group_id_array.get()); diff --git a/src/include/zvec/db/schema.h b/src/include/zvec/db/schema.h index 1b2219337..0b03a1c66 100644 --- a/src/include/zvec/db/schema.h +++ b/src/include/zvec/db/schema.h @@ -408,6 +408,9 @@ class ZVEC_API CollectionSchema { private: void copy_fields(const FieldSchemaPtrList &fields) { for (auto &field : fields) { + if (!field) { + continue; + } auto c = std::make_shared(*field); fields_.push_back(c); fields_map_[field->name()] = c; diff --git a/tests/c/c_api_test.c b/tests/c/c_api_test.c index 9b7ea432d..885a171a0 100644 --- a/tests/c/c_api_test.c +++ b/tests/c/c_api_test.c @@ -85,6 +85,23 @@ static int current_test_passed = 1; // Track if current test function passes } \ } while (0) +static void check_last_error(zvec_error_code_t code, const char *reason) { + char *message = NULL; + TEST_ASSERT(zvec_get_last_error(&message) == ZVEC_OK); + TEST_ASSERT(message != NULL); + if (message) { + TEST_ASSERT(strstr(message, reason) != NULL); + } + zvec_error_details_t details = {0}; + TEST_ASSERT(zvec_get_last_error_details(&details) == ZVEC_OK); + TEST_ASSERT(details.code == code); + TEST_ASSERT(details.message != NULL); + if (message && details.message) { + TEST_ASSERT(strcmp(message, details.message) == 0); + } + zvec_free(message); +} + // ============================================================================= // Helper functions tests // ============================================================================= @@ -1026,6 +1043,280 @@ void test_collection_basic_operations(void) { TEST_END(); } +void test_relaxed_name_validation(void) { + TEST_START(); + + const char *temp_dir = "./zvec_test_relaxed_name_validation"; + cleanup_temp_directory(temp_dir); + const char *collection_name = "\xe9\x9b\x86\xe5\x90\x88 / 2026"; + char field_name[65]; + memset(field_name, 'f', sizeof(field_name) - 1); + field_name[sizeof(field_name) - 1] = '\0'; + zvec_collection_schema_t *schema = + zvec_collection_schema_create(collection_name); + zvec_field_schema_t *field = + zvec_field_schema_create(field_name, ZVEC_DATA_TYPE_INT32, false, 0); + TEST_ASSERT(schema != NULL); + TEST_ASSERT(field != NULL); + TEST_ASSERT(zvec_collection_schema_add_field(schema, field) == ZVEC_OK); + zvec_field_schema_destroy(field); + + zvec_collection_t *collection = NULL; + zvec_error_code_t err = + zvec_collection_create_and_open(temp_dir, schema, NULL, &collection); + TEST_ASSERT(err == ZVEC_OK); + TEST_ASSERT(collection != NULL); + if (collection) { + char long_id[1025]; + memset(long_id, 'x', sizeof(long_id) - 1); + long_id[sizeof(long_id) - 1] = '\0'; + const char *ids[] = {"\xe8\xae\xa2\xe5\x8d\x95:2026", + "https://example.com/articles/42", long_id}; + zvec_doc_t *doc = zvec_doc_create(); + int32_t value = 42; + TEST_ASSERT(zvec_doc_add_field_by_value(doc, field_name, + ZVEC_DATA_TYPE_INT32, &value, + sizeof(value)) == ZVEC_OK); + const zvec_doc_t *docs[] = {doc}; + for (size_t i = 0; i < sizeof(ids) / sizeof(ids[0]); ++i) { + zvec_doc_set_pk(doc, ids[i]); + size_t success_count = 0, error_count = 0; + err = zvec_collection_insert(collection, docs, 1, &success_count, + &error_count); + TEST_ASSERT(err == ZVEC_OK); + TEST_ASSERT(success_count == 1); + TEST_ASSERT(error_count == 0); + + zvec_doc_t **fetched = NULL; + size_t found_count = 0; + err = zvec_collection_fetch(collection, &ids[i], 1, NULL, 0, false, + &fetched, &found_count); + TEST_ASSERT(err == ZVEC_OK); + TEST_ASSERT(found_count == 1); + if (found_count == 1) { + TEST_ASSERT(strcmp(zvec_doc_get_pk_pointer(fetched[0]), ids[i]) == 0); + } + zvec_docs_free(fetched, found_count); + } + + char oversized_id[1026]; + memset(oversized_id, 'x', sizeof(oversized_id) - 1); + oversized_id[sizeof(oversized_id) - 1] = '\0'; + const char *invalid_ids[] = {"\xff", oversized_id, "doc\nid"}; + const char *reasons[] = {"not valid UTF-8", "exceeds 1024 bytes (got 1025)", + "newline"}; + for (size_t i = 0; i < sizeof(invalid_ids) / sizeof(invalid_ids[0]); ++i) { + zvec_doc_set_pk(doc, invalid_ids[i]); + size_t success_count = 0, error_count = 0; + err = zvec_collection_insert(collection, docs, 1, &success_count, + &error_count); + TEST_ASSERT(err == ZVEC_ERROR_INVALID_ARGUMENT); + TEST_ASSERT(success_count == 0); + TEST_ASSERT(error_count == 1); + char *error_msg = NULL; + zvec_get_last_error(&error_msg); + TEST_ASSERT(error_msg != NULL); + if (error_msg) { + TEST_ASSERT(strncmp(error_msg, "Invalid doc:", 12) == 0); + TEST_ASSERT(strstr(error_msg, reasons[i]) != NULL); + TEST_ASSERT(strstr(error_msg, "offset") == NULL); + zvec_free(error_msg); + } + } + zvec_doc_destroy(doc); + zvec_collection_destroy(collection); + } + zvec_collection_schema_destroy(schema); + cleanup_temp_directory(temp_dir); + + TEST_END(); +} + +void test_collection_name_encoding_validation(void) { + TEST_START(); + + char max_name[257]; + memset(max_name, 'c', sizeof(max_name) - 1); + max_name[sizeof(max_name) - 1] = '\0'; + char oversized_name[258]; + memset(oversized_name, 'c', sizeof(oversized_name) - 1); + oversized_name[sizeof(oversized_name) - 1] = '\0'; + const char *names[] = {"x", max_name, "\xff", oversized_name}; + const char *reasons[] = {NULL, NULL, "not valid UTF-8", + "exceeds 256 bytes (got 257)"}; + for (size_t i = 0; i < sizeof(names) / sizeof(names[0]); ++i) { + zvec_collection_schema_t *schema = zvec_collection_schema_create(names[i]); + zvec_field_schema_t *field = + zvec_field_schema_create("value", ZVEC_DATA_TYPE_INT32, false, 0); + TEST_ASSERT(schema != NULL); + TEST_ASSERT(field != NULL); + TEST_ASSERT(zvec_collection_schema_add_field(schema, field) == ZVEC_OK); + zvec_field_schema_destroy(field); + + zvec_string_t *error_msg = NULL; + zvec_error_code_t err = zvec_collection_schema_validate(schema, &error_msg); + if (reasons[i]) { + TEST_ASSERT(err == ZVEC_ERROR_INVALID_ARGUMENT); + TEST_ASSERT(error_msg != NULL); + if (error_msg) { + const char *message = zvec_string_c_str(error_msg); + TEST_ASSERT(strncmp(message, "Invalid schema:", 15) == 0); + TEST_ASSERT(strstr(message, "collection name") != NULL); + TEST_ASSERT(strstr(message, reasons[i]) != NULL); + TEST_ASSERT(strstr(message, "offset") == NULL); + } + } else { + TEST_ASSERT(err == ZVEC_OK); + TEST_ASSERT(error_msg == NULL); + } + zvec_free_string(error_msg); + zvec_collection_schema_destroy(schema); + } + + TEST_END(); +} + +void test_validation_last_error(void) { + TEST_START(); + + zvec_collection_schema_t *schema = zvec_collection_schema_create("\xff"); + zvec_field_schema_t *field = + zvec_field_schema_create("bad name", ZVEC_DATA_TYPE_INT32, false, 0); + TEST_ASSERT(schema != NULL); + TEST_ASSERT(field != NULL); + + zvec_clear_error(); + TEST_ASSERT(zvec_collection_schema_validate(schema, NULL) == + ZVEC_ERROR_INVALID_ARGUMENT); + check_last_error(ZVEC_ERROR_INVALID_ARGUMENT, + "Invalid schema: collection name[\\xFF] is not valid UTF-8"); + // Replacing a previous error must update both text and code. + TEST_ASSERT(zvec_field_schema_validate(field, NULL) == + ZVEC_ERROR_INVALID_ARGUMENT); + check_last_error(ZVEC_ERROR_INVALID_ARGUMENT, + "Invalid schema: field[bad name] contains a space"); + + zvec_string_t *error = NULL; + TEST_ASSERT(zvec_collection_schema_validate(schema, &error) == + ZVEC_ERROR_INVALID_ARGUMENT); + TEST_ASSERT(error != NULL); + if (error) { + check_last_error(ZVEC_ERROR_INVALID_ARGUMENT, zvec_string_c_str(error)); + } + zvec_free_string(error); + + error = (zvec_string_t *)(uintptr_t)1; + TEST_ASSERT(zvec_collection_schema_validate(NULL, &error) == + ZVEC_ERROR_INVALID_ARGUMENT); + TEST_ASSERT(error == NULL); + check_last_error(ZVEC_ERROR_INVALID_ARGUMENT, "cannot be null"); + error = (zvec_string_t *)(uintptr_t)1; + TEST_ASSERT(zvec_field_schema_validate(NULL, &error) == + ZVEC_ERROR_INVALID_ARGUMENT); + TEST_ASSERT(error == NULL); + + zvec_collection_t *collection = (zvec_collection_t *)(uintptr_t)1; + TEST_ASSERT(zvec_collection_create_and_open("./zvec_test_invalid_utf8_name", + schema, NULL, &collection) == + ZVEC_ERROR_INVALID_ARGUMENT); + TEST_ASSERT(collection == NULL); + check_last_error(ZVEC_ERROR_INVALID_ARGUMENT, + "Invalid schema: collection name[\\xFF] is not valid UTF-8"); + + zvec_field_schema_destroy(field); + zvec_collection_schema_destroy(schema); + TEST_END(); +} + +void test_batch_validation_errors(void) { + TEST_START(); + + typedef zvec_error_code_t(ZVEC_CALL * count_write_fn)( + zvec_collection_t *, const zvec_doc_t **, size_t, size_t *, size_t *); + typedef zvec_error_code_t(ZVEC_CALL * result_write_fn)( + zvec_collection_t *, const zvec_doc_t **, size_t, zvec_write_result_t **, + size_t *); + count_write_fn count_ops[] = {zvec_collection_insert, zvec_collection_update, + zvec_collection_upsert}; + result_write_fn result_ops[] = {zvec_collection_insert_with_results, + zvec_collection_update_with_results, + zvec_collection_upsert_with_results}; + for (size_t op = 0; op < 3; ++op) { + zvec_write_result_t *results = (zvec_write_result_t *)(uintptr_t)1; + size_t result_count = 123; + TEST_ASSERT(result_ops[op](NULL, NULL, 1, &results, &result_count) == + ZVEC_ERROR_INVALID_ARGUMENT); + TEST_ASSERT(results == NULL); + TEST_ASSERT(result_count == 0); + check_last_error(ZVEC_ERROR_INVALID_ARGUMENT, "Invalid arguments:"); + } + const char *path = "./zvec_test_batch_validation_errors"; + cleanup_temp_directory(path); + zvec_collection_schema_t *schema = zvec_collection_schema_create("batch"); + zvec_field_schema_t *field = + zvec_field_schema_create("value", ZVEC_DATA_TYPE_INT32, true, 0); + TEST_ASSERT(zvec_collection_schema_add_field(schema, field) == ZVEC_OK); + zvec_field_schema_destroy(field); + zvec_collection_t *collection = NULL; + TEST_ASSERT(zvec_collection_create_and_open(path, schema, NULL, + &collection) == ZVEC_OK); + TEST_ASSERT(collection != NULL); + if (collection) { + zvec_doc_t *valid_doc = zvec_doc_create(); + zvec_doc_t *invalid_doc = zvec_doc_create(); + zvec_doc_set_pk(valid_doc, "valid_before_error"); + zvec_doc_set_pk(invalid_doc, "\xff"); + const zvec_doc_t *invalid_inputs[] = {NULL, invalid_doc}; + const char *reasons[] = {"Invalid doc: document must not be null", + "id[\\xFF] is not valid UTF-8"}; + for (size_t i = 0; i < 2; ++i) { + const zvec_doc_t *docs[] = {valid_doc, invalid_inputs[i]}; + for (size_t op = 0; op < 3; ++op) { + size_t success_count = 123, error_count = 456; + TEST_ASSERT(count_ops[op](collection, docs, 2, &success_count, + &error_count) == ZVEC_ERROR_INVALID_ARGUMENT); + TEST_ASSERT(success_count == 0); + TEST_ASSERT(error_count == 2); + check_last_error(ZVEC_ERROR_INVALID_ARGUMENT, reasons[i]); + if (i == 0) { + zvec_error_details_t details = {0}; + TEST_ASSERT(zvec_get_last_error_details(&details) == ZVEC_OK); + TEST_ASSERT(details.message && + strcmp(details.message, reasons[i]) == 0); + } + + zvec_write_result_t *results = (zvec_write_result_t *)(uintptr_t)1; + size_t result_count = 123; + TEST_ASSERT( + result_ops[op](collection, docs, 2, &results, &result_count) == + ZVEC_ERROR_INVALID_ARGUMENT); + TEST_ASSERT(results == NULL); + TEST_ASSERT(result_count == 0); + check_last_error(ZVEC_ERROR_INVALID_ARGUMENT, reasons[i]); + if (i == 0) { + zvec_error_details_t details = {0}; + TEST_ASSERT(zvec_get_last_error_details(&details) == ZVEC_OK); + TEST_ASSERT(details.message && + strcmp(details.message, reasons[i]) == 0); + } + } + } + const char *ids[] = {"valid_before_error"}; + zvec_doc_t **fetched = NULL; + size_t found_count = 0; + TEST_ASSERT(zvec_collection_fetch(collection, ids, 1, NULL, 0, false, + &fetched, &found_count) == ZVEC_OK); + TEST_ASSERT(found_count == 0); + zvec_docs_free(fetched, found_count); + zvec_doc_destroy(valid_doc); + zvec_doc_destroy(invalid_doc); + zvec_collection_destroy(collection); + } + zvec_collection_schema_destroy(schema); + cleanup_temp_directory(path); + TEST_END(); +} + void test_collection_edge_cases(void) { TEST_START(); @@ -6791,6 +7082,10 @@ int main(void) { // Collection-related tests test_collection_basic_operations(); + test_relaxed_name_validation(); + test_collection_name_encoding_validation(); + test_validation_last_error(); + test_batch_validation_errors(); test_collection_edge_cases(); test_collection_delete_by_filter(); test_collection_stats(); diff --git a/tests/db/collection_test.cc b/tests/db/collection_test.cc index e811be6fc..a67788af4 100644 --- a/tests/db/collection_test.cc +++ b/tests/db/collection_test.cc @@ -45,6 +45,7 @@ #include #include #include "db/collection_query_internal.h" +#include "db/common/constants.h" #include "db/common/file_helper.h" #include "db/index/common/type_helper.h" #include "db/index/common/version_manager.h" @@ -74,9 +75,64 @@ class CollectionTest : public ::testing::Test { } void TearDown() override { + collection_.reset(); FileHelper::RemoveDirectory(col_path); ailego::FileHelper::RemoveDirectory("demo"); } + + CollectionSchema MakeSchema(const std::string &name = "x", + const std::string &field = "value") { + CollectionSchema schema(name); + EXPECT_TRUE(schema + .add_field(std::make_shared( + field, DataType::INT32, false)) + .ok()); + return schema; + } + + void Create(const CollectionSchema &schema) { + auto result = Collection::CreateAndOpen(col_path, schema, options_); + ASSERT_TRUE(result.has_value()) << result.error().message(); + collection_ = std::move(result).value(); + } + + void Reopen() { + collection_.reset(); + auto result = Collection::Open(col_path, options_); + ASSERT_TRUE(result.has_value()) << result.error().message(); + collection_ = std::move(result).value(); + } + + Doc MakeDoc(const std::string &id, int32_t value, + const std::string &field = "value") { + Doc doc; + doc.set_pk(id); + EXPECT_TRUE(doc.set(field, value)); + return doc; + } + + void ExpectWrite(const Result &result, size_t count) { + ASSERT_TRUE(result.has_value()) << result.error().message(); + ASSERT_EQ(result.value().size(), count); + for (const auto &status : result.value()) { + ASSERT_TRUE(status.ok()) << status.message(); + } + } + + void ExpectValue(const std::string &id, int32_t expected, + const std::string &field = "value") { + auto result = collection_->fetch({id}); + ASSERT_TRUE(result.has_value()) << result.error().message(); + ASSERT_EQ(result.value().size(), 1u); + auto found = result.value().find(id); + ASSERT_NE(found, result.value().end()); + ASSERT_NE(found->second, nullptr); + EXPECT_EQ(found->second->pk(), id); + EXPECT_EQ(found->second->get(field), expected); + } + + CollectionOptions options_; + Collection::Ptr collection_; }; class DirectoryWriteBlockerForTest { @@ -329,6 +385,46 @@ TEST_F(CollectionTest, Feature_OpenReadOnly_WithReadOnlyLockFile) { fs::perm_options::replace, ec); } +TEST_F(CollectionTest, Feature_CreateAndOpen_NameBoundaries) { + for (const auto &name : std::vector{ + "x", "xy", u8"集", std::string(253, 'n') + u8"集"}) { + SCOPED_TRACE(name); + ASSERT_NO_FATAL_FAILURE(Create(MakeSchema(name))); + std::vector docs{MakeDoc("id", 1)}; + ASSERT_NO_FATAL_FAILURE(ExpectWrite(collection_->insert(docs), 1)); + auto status = collection_->flush(); + ASSERT_TRUE(status.ok()) << status.message(); + ASSERT_NO_FATAL_FAILURE(Reopen()); + auto schema = collection_->schema(); + ASSERT_TRUE(schema.has_value()) << schema.error().message(); + EXPECT_EQ(schema.value().name(), name); + ASSERT_NO_FATAL_FAILURE(ExpectValue("id", 1)); + status = collection_->destroy(); + ASSERT_TRUE(status.ok()) << status.message(); + collection_.reset(); + } +} + +TEST_F(CollectionTest, Feature_CreateAndOpen_InvalidSchema) { + auto result = Collection::CreateAndOpen( + col_path, MakeSchema("x", "_zvec_uid_"), options_); + ASSERT_FALSE(result.has_value()); + EXPECT_EQ(result.error().code(), StatusCode::INVALID_ARGUMENT); + EXPECT_NE(result.error().message().find("is reserved"), std::string::npos); + EXPECT_FALSE(ailego::FileHelper::IsExist(col_path.c_str())); + + CollectionSchema duplicate( + "x", {std::make_shared("value", DataType::INT32), + std::make_shared("value", DataType::VECTOR_FP32, 4, + false)}); + result = Collection::CreateAndOpen(col_path, duplicate, options_); + ASSERT_FALSE(result.has_value()); + EXPECT_EQ(result.error().code(), StatusCode::INVALID_ARGUMENT); + EXPECT_NE(result.error().message().find("duplicate field name [value]"), + std::string::npos); + EXPECT_FALSE(ailego::FileHelper::IsExist(col_path.c_str())); +} + TEST_F(CollectionTest, Feature_CreateAndOpen_Empty) { int doc_count = 0; int loop_count = 100; @@ -505,6 +601,36 @@ TEST_F(CollectionTest, Feature_CreateAndOpen_MultiThread) { ASSERT_FALSE(has_error.load()); } +TEST_F(CollectionTest, Feature_Write_InvalidIdRejectsWholeBatch) { + ASSERT_NO_FATAL_FAILURE(Create(MakeSchema())); + std::vector initial{MakeDoc("existing", 1)}; + ASSERT_NO_FATAL_FAILURE(ExpectWrite(collection_->insert(initial), 1)); + for (int operation = 0; operation < 3; ++operation) { + SCOPED_TRACE(operation); + const std::string first_id = operation == 0 ? "new:id" : "existing"; + std::vector batch{MakeDoc(first_id, 99), + MakeDoc(std::string("bad\0id", 6), 100)}; + auto result = operation == 0 ? collection_->insert(batch) + : operation == 1 ? collection_->update(batch) + : collection_->upsert(batch); + ASSERT_FALSE(result.has_value()); + EXPECT_EQ(result.error().code(), StatusCode::INVALID_ARGUMENT); + EXPECT_EQ(result.error().message().find("Invalid doc:"), 0u); + EXPECT_NE(result.error().message().find("null character"), + std::string::npos); + EXPECT_NE(result.error().message().find("id[bad\\0id]"), std::string::npos); + EXPECT_EQ(result.error().message().find("offset"), std::string::npos); + ASSERT_NO_FATAL_FAILURE(ExpectValue("existing", 1)); + auto missing = collection_->fetch({"new:id"}); + ASSERT_TRUE(missing.has_value()) << missing.error().message(); + ASSERT_EQ(missing.value().size(), 1u); + EXPECT_EQ(missing.value().at("new:id"), nullptr); + } + ASSERT_NO_FATAL_FAILURE(Reopen()); + ASSERT_NO_FATAL_FAILURE(ExpectValue("existing", 1)); + EXPECT_EQ(collection_->stats().value().doc_count, 1u); +} + TEST_F(CollectionTest, Feature_Write_Batch_Validate) { FileHelper::RemoveDirectory(col_path); @@ -538,6 +664,50 @@ TEST_F(CollectionTest, Feature_Write_Batch_Validate) { ASSERT_FALSE(upsert_exceed_status.ok()); } +TEST_F(CollectionTest, Feature_Write_Utf8IdsAndReopen) { + const std::string name = u8"测试 集合/v1"; + ASSERT_NO_FATAL_FAILURE(Create(MakeSchema(name))); + const std::vector ids = { + "user:123", "https://example.com/document/42", + u8"订单-😀", std::string(1021, 'i') + u8"中", + "doc", " doc", + "doc ", " ", + u8"café", u8"cafe\u0301", + "DOC"}; + std::vector docs; + for (size_t i = 0; i < ids.size(); ++i) { + docs.push_back(MakeDoc(ids[i], static_cast(i))); + } + ASSERT_NO_FATAL_FAILURE(ExpectWrite(collection_->insert(docs), docs.size())); + + std::vector updates{MakeDoc(ids[0], 100)}; + ASSERT_NO_FATAL_FAILURE(ExpectWrite(collection_->update(updates), 1)); + std::vector upserts{MakeDoc(ids[3], 103), MakeDoc(u8"新增:文档", 200)}; + ASSERT_NO_FATAL_FAILURE(ExpectWrite(collection_->upsert(upserts), 2)); + ASSERT_NO_FATAL_FAILURE(ExpectWrite(collection_->delete_({ids[1]}), 1)); + + auto flush_status = collection_->flush(); + ASSERT_TRUE(flush_status.ok()) << flush_status.message(); + ASSERT_NO_FATAL_FAILURE(Reopen()); + auto schema = collection_->schema(); + ASSERT_TRUE(schema.has_value()) << schema.error().message(); + EXPECT_EQ(schema.value().name(), name); + for (size_t i = 0; i < ids.size(); ++i) { + if (i == 1) { + continue; + } + int32_t expected = static_cast(i); + if (i == 0) expected = 100; + if (i == 3) expected = 103; + ASSERT_NO_FATAL_FAILURE(ExpectValue(ids[i], expected)); + } + ASSERT_NO_FATAL_FAILURE(ExpectValue(u8"新增:文档", 200)); + auto deleted = collection_->fetch({ids[1]}); + ASSERT_TRUE(deleted.has_value()) << deleted.error().message(); + ASSERT_EQ(deleted.value().size(), 1u); + EXPECT_EQ(deleted.value().at(ids[1]), nullptr); +} + TEST_F(CollectionTest, Feature_Insert_General) { auto func = [&](bool enable_mmap, bool schema_nullable, bool doc_nullable, int doc_count = 1000) { @@ -1590,6 +1760,26 @@ TEST_F(CollectionTest, Feature_Update_Empty) { } } +TEST_F(CollectionTest, Feature_FetchAndDelete_InvalidIdsRemainMissing) { + ASSERT_NO_FATAL_FAILURE(Create(MakeSchema())); + // These are invalid for a new document, but lookup must retain its existing + // missing-key behavior rather than introducing input validation errors. + const std::vector absent_ids{"", std::string("bad\0id", 6), + std::string(1025, 'x'), "\xff"}; + auto fetched = collection_->fetch(absent_ids); + ASSERT_TRUE(fetched.has_value()) << fetched.error().message(); + ASSERT_EQ(fetched.value().size(), absent_ids.size()); + for (const auto &id : absent_ids) { + EXPECT_EQ(fetched.value().at(id), nullptr); + } + auto deleted = collection_->delete_(absent_ids); + ASSERT_TRUE(deleted.has_value()) << deleted.error().message(); + ASSERT_EQ(deleted.value().size(), absent_ids.size()); + for (const auto &status : deleted.value()) { + EXPECT_EQ(status.code(), StatusCode::NOT_FOUND); + } +} + TEST_F(CollectionTest, Feature_Delete_General) { auto func = [&](bool enable_mmap, int doc_count) { auto schema = TestHelper::CreateNormalSchema(); @@ -4407,6 +4597,76 @@ TEST_F(CollectionTest, Feature_Query_Validate) { } } +TEST_F(CollectionTest, Feature_Query_MaximumLengthFieldNames) { +#ifdef __ANDROID__ + GTEST_SKIP() << "Skipped on Android: emulator filesystem lacks hardlink " + "support (needed by RocksDB checkpoint)"; +#endif + auto check = [&](bool enable_mmap) { + SCOPED_TRACE(enable_mmap); + options_.enable_mmap_ = enable_mmap; + const std::string scalar = "s" + std::string(63, 'a'); + const std::string second_scalar = scalar.substr(0, 63) + "b"; + const std::string vector = "v" + std::string(63, 'b'); + auto schema = MakeSchema("x", scalar); + ASSERT_TRUE(schema + .add_field(std::make_shared( + second_scalar, DataType::INT32, false)) + .ok()); + ASSERT_TRUE(schema + .add_field(std::make_shared( + vector, DataType::VECTOR_FP32, 4, false, + std::make_shared(MetricType::L2))) + .ok()); + ASSERT_NO_FATAL_FAILURE(Create(schema)); + const std::vector values{1.0f, 2.0f, 3.0f, 4.0f}; + Doc doc = MakeDoc(u8"文档:1", 42, scalar); + ASSERT_TRUE(doc.set(second_scalar, 7)); + ASSERT_TRUE(doc.set>(vector, values)); + std::vector docs{doc}; + ASSERT_NO_FATAL_FAILURE(ExpectWrite(collection_->insert(docs), 1)); + auto status = collection_->flush(); + ASSERT_TRUE(status.ok()) << status.message(); + status = collection_->create_index(scalar, + std::make_shared()); + ASSERT_TRUE(status.ok()) << status.message(); + status = collection_->create_index( + vector, std::make_shared(MetricType::L2)); + ASSERT_TRUE(status.ok()) << status.message(); + status = collection_->create_index(second_scalar, + std::make_shared()); + ASSERT_TRUE(status.ok()) << status.message(); + ASSERT_NO_FATAL_FAILURE(Reopen()); + + // Fetch supplies the query vector, covering lookup by a newly allowed ID. + auto fetched = collection_->fetch({doc.pk()}); + ASSERT_TRUE(fetched.has_value()) << fetched.error().message(); + ASSERT_EQ(fetched.value().size(), 1u); + const auto stored_vector = + fetched.value().at(doc.pk())->get>(vector); + ASSERT_TRUE(stored_vector.has_value()); + EXPECT_EQ(stored_vector.value(), values); + SearchQuery query; + query.topk_ = 1; + query.target_.field_name_ = vector; + query.target_.set_vector( + std::string(reinterpret_cast(stored_vector->data()), + stored_vector->size() * sizeof(float))); + query.filter_ = scalar + " = 42 AND " + second_scalar + " = 7"; + query.output_fields_ = std::vector{scalar, second_scalar}; + auto matches = collection_->query(query); + ASSERT_TRUE(matches.has_value()) << matches.error().message(); + ASSERT_EQ(matches.value().size(), 1u); + EXPECT_EQ(matches.value()[0]->pk(), doc.pk()); + EXPECT_EQ(matches.value()[0]->get(scalar), 42); + EXPECT_EQ(matches.value()[0]->get(second_scalar), 7); + collection_.reset(); + FileHelper::RemoveDirectory(col_path); + }; + ASSERT_NO_FATAL_FAILURE(check(true)); + ASSERT_NO_FATAL_FAILURE(check(false)); +} + TEST_F(CollectionTest, Feature_Query_General) { auto func = [&](bool enable_mmap, std::string field_name) { FileHelper::RemoveDirectory(col_path); @@ -5215,6 +5475,135 @@ TEST_F(CollectionTest, Feature_MultiQuery_CallbackReranker) { TEST_F(CollectionTest, Feature_GroupByQuery) {} +TEST_F(CollectionTest, Feature_ColumnDDL_DuplicateNamesKeepData) { + auto schema = MakeSchema(); + ASSERT_TRUE(schema + .add_field(std::make_shared( + "other", DataType::INT32, true)) + .ok()); + ASSERT_TRUE(schema + .add_field(std::make_shared( + "embedding", DataType::VECTOR_FP32, 4, true)) + .ok()); + ASSERT_NO_FATAL_FAILURE(Create(schema)); + std::vector docs{MakeDoc("id", 42)}; + ASSERT_TRUE(docs[0].set>("embedding", {1, 2, 3, 4})); + ASSERT_NO_FATAL_FAILURE(ExpectWrite(collection_->insert(docs), 1)); + const auto before = collection_->schema().value(); + for (const std::string name : {"other", "embedding"}) { + SCOPED_TRACE(name); + auto field = std::make_shared(name, DataType::INT32, true); + auto status = collection_->add_column(field, ""); + EXPECT_EQ(status.code(), StatusCode::INVALID_ARGUMENT); + EXPECT_NE(status.message().find("already exists"), std::string::npos); + status = collection_->alter_column("value", name); + EXPECT_EQ(status.code(), StatusCode::INVALID_ARGUMENT); + EXPECT_NE(status.message().find("already exists"), std::string::npos); + status = collection_->alter_column("value", "", field); + EXPECT_EQ(status.code(), StatusCode::ALREADY_EXISTS); + EXPECT_NE(status.message().find("already exists"), std::string::npos); + EXPECT_EQ(collection_->schema().value(), before); + ASSERT_NO_FATAL_FAILURE(ExpectValue("id", 42)); + } + ASSERT_NO_FATAL_FAILURE(Reopen()); + EXPECT_EQ(collection_->schema().value(), before); + ASSERT_NO_FATAL_FAILURE(ExpectValue("id", 42)); +} + +TEST_F(CollectionTest, Feature_ColumnDDL_ReservedNamesKeepData) { + ASSERT_NO_FATAL_FAILURE(Create(MakeSchema())); + std::vector docs{MakeDoc("id", 42)}; + ASSERT_NO_FATAL_FAILURE(ExpectWrite(collection_->insert(docs), 1)); + const auto before = collection_->schema().value(); + const std::string name = "_zvec_uid_"; + auto status = collection_->add_column( + std::make_shared(name, DataType::INT32, true), ""); + EXPECT_EQ(status.code(), StatusCode::INVALID_ARGUMENT); + EXPECT_NE(status.message().find("is reserved"), std::string::npos); + status = collection_->alter_column("value", name); + EXPECT_EQ(status.code(), StatusCode::INVALID_ARGUMENT); + EXPECT_NE(status.message().find("is reserved"), std::string::npos); + status = collection_->alter_column( + "value", "", std::make_shared(name, DataType::INT32)); + EXPECT_EQ(status.code(), StatusCode::INVALID_ARGUMENT); + EXPECT_NE(status.message().find("is reserved"), std::string::npos); + EXPECT_EQ(collection_->schema().value(), before); + ASSERT_NO_FATAL_FAILURE(ExpectValue("id", 42)); + ASSERT_NO_FATAL_FAILURE(Reopen()); + EXPECT_EQ(collection_->schema().value(), before); + ASSERT_NO_FATAL_FAILURE(ExpectValue("id", 42)); +} + +TEST_F(CollectionTest, Feature_ColumnDDL_FieldCountLimits) { + CollectionSchema schema("x"); + for (uint32_t i = 0; i < kMaxScalarFieldSize; ++i) { + ASSERT_TRUE(schema + .add_field(std::make_shared( + "f" + std::to_string(i), DataType::INT32, true)) + .ok()); + } + ASSERT_NO_FATAL_FAILURE(Create(schema)); + auto status = collection_->add_column( + std::make_shared("excess", DataType::INT32, true), ""); + EXPECT_EQ(status.code(), StatusCode::INVALID_ARGUMENT); + EXPECT_NE(status.message().find("1024 scalar fields"), std::string::npos); + EXPECT_EQ(collection_->schema().value(), schema); + EXPECT_FALSE(collection_->schema().value().has_field("excess")); + status = collection_->destroy(); + ASSERT_TRUE(status.ok()) << status.message(); + collection_.reset(); + + ASSERT_NO_FATAL_FAILURE(Create(MakeSchema())); + std::vector docs{MakeDoc("id", 42)}; + ASSERT_NO_FATAL_FAILURE(ExpectWrite(collection_->insert(docs), 1)); + status = collection_->drop_column("value"); + EXPECT_EQ(status.code(), StatusCode::INVALID_ARGUMENT); + EXPECT_NE(status.message().find("last field"), std::string::npos); + ASSERT_NO_FATAL_FAILURE(ExpectValue("id", 42)); + ASSERT_NO_FATAL_FAILURE(Reopen()); + EXPECT_TRUE(collection_->schema().value().has_field("value")); + ASSERT_NO_FATAL_FAILURE(ExpectValue("id", 42)); +} + +TEST_F(CollectionTest, Feature_ColumnDDL_CopiesCallerSchema) { + ASSERT_NO_FATAL_FAILURE(Create(MakeSchema())); + std::vector initial{MakeDoc("original", 1)}; + ASSERT_NO_FATAL_FAILURE(ExpectWrite(collection_->insert(initial), 1)); + auto added = std::make_shared("extra", DataType::INT32, true); + auto status = collection_->add_column(added, ""); + ASSERT_TRUE(status.ok()) << status.message(); + added->set_name("bad name"); + added->set_data_type(DataType::STRING); + added->set_nullable(false); + + auto current = collection_->schema().value(); + ASSERT_TRUE(current.has_field("extra")); + EXPECT_EQ(current.get_field("extra")->data_type(), DataType::INT32); + EXPECT_TRUE(current.get_field("extra")->nullable()); + EXPECT_FALSE(current.has_field("bad name")); + Doc doc = MakeDoc("new", 2); + ASSERT_TRUE(doc.set("extra", 7)); + std::vector docs{doc}; + ASSERT_NO_FATAL_FAILURE(ExpectWrite(collection_->insert(docs), 1)); + + auto altered = std::make_shared("extra", DataType::INT64, true); + status = collection_->alter_column("extra", "", altered); + ASSERT_TRUE(status.ok()) << status.message(); + altered->set_name("another bad name"); + altered->set_data_type(DataType::STRING); + current = collection_->schema().value(); + ASSERT_TRUE(current.has_field("extra")); + EXPECT_EQ(current.get_field("extra")->data_type(), DataType::INT64); + EXPECT_FALSE(current.has_field("another bad name")); + ASSERT_NO_FATAL_FAILURE(Reopen()); + ASSERT_NO_FATAL_FAILURE(ExpectValue("original", 1)); + ASSERT_NO_FATAL_FAILURE(ExpectValue("new", 2)); + auto fetched = collection_->fetch({"new"}); + ASSERT_TRUE(fetched.has_value()) << fetched.error().message(); + ASSERT_NE(fetched.value().at("new"), nullptr); + EXPECT_EQ(fetched.value().at("new")->get("extra"), 7); +} + TEST_F(CollectionTest, Feature_AddColumn_General) { auto func = [&](bool enable_mmap) { FileHelper::RemoveDirectory(col_path); @@ -5438,6 +5827,34 @@ TEST_F(CollectionTest, Feature_DropColumn_General) { ASSERT_TRUE(!new_schema.has_field("int32")); } +TEST_F(CollectionTest, Feature_AlterColumn_InvalidNameKeepsData) { + ASSERT_NO_FATAL_FAILURE(Create(MakeSchema())); + std::vector docs{MakeDoc("id", 42)}; + ASSERT_NO_FATAL_FAILURE(ExpectWrite(collection_->insert(docs), 1)); + const auto before = collection_->schema().value(); + for (const auto &name : std::vector{ + "user name", "../value", u8"字段", std::string(65, 'f')}) { + SCOPED_TRACE(name); + auto status = collection_->alter_column("value", name); + ASSERT_EQ(status.code(), StatusCode::INVALID_ARGUMENT); + EXPECT_EQ(status.message().find("Invalid schema:"), 0u); + EXPECT_EQ(status.message().find("offset"), std::string::npos); + EXPECT_EQ(collection_->schema().value(), before); + ASSERT_NO_FATAL_FAILURE(ExpectValue("id", 42)); + } + ASSERT_NO_FATAL_FAILURE(Reopen()); + EXPECT_EQ(collection_->schema().value(), before); + ASSERT_NO_FATAL_FAILURE(ExpectValue("id", 42)); + + const std::string renamed(64, 'r'); + auto status = collection_->alter_column("value", renamed); + ASSERT_TRUE(status.ok()) << status.message(); + ASSERT_NO_FATAL_FAILURE(Reopen()); + EXPECT_FALSE(collection_->schema().value().has_field("value")); + EXPECT_TRUE(collection_->schema().value().has_field(renamed)); + ASSERT_NO_FATAL_FAILURE(ExpectValue("id", 42, renamed)); +} + TEST_F(CollectionTest, Feature_AlterColumn_General) { // create collection int doc_count = 1000; diff --git a/tests/db/crash_recovery/write_recovery_test.cc b/tests/db/crash_recovery/write_recovery_test.cc index 467ed623c..614fb71ea 100644 --- a/tests/db/crash_recovery/write_recovery_test.cc +++ b/tests/db/crash_recovery/write_recovery_test.cc @@ -14,8 +14,12 @@ #include +#include +#include #include +#include #include +#include #include #include #include @@ -161,6 +165,64 @@ class CrashRecoveryTest : public ::testing::Test { }; +TEST_F(CrashRecoveryTest, Utf8AndLongIdsRecoverFromUnflushedWal) { + const std::vector ids{u8"订单:😀", + std::string(1021, 'x') + u8"中", " doc ", + u8"café", u8"cafe\u0301"}; + // Re-exec the child before starting collection threads. Exit without stack + // unwinding so Collection destruction cannot flush the writing segment. + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + ASSERT_EXIT( + { + CollectionSchema schema(u8"恢复 集合"); + if (!schema + .add_field(std::make_shared( + "value", DataType::INT32, false)) + .ok()) { + std::_Exit(1); + } + auto created = + Collection::CreateAndOpen(dir_path_, schema, CollectionOptions{}); + if (!created.has_value()) std::_Exit(2); + auto collection = std::move(created).value(); + std::vector docs; + for (size_t i = 0; i < ids.size(); ++i) { + Doc doc; + doc.set_pk(ids[i]); + doc.set("value", static_cast(i)); + docs.push_back(std::move(doc)); + } + auto inserted = collection->insert(docs); + if (!inserted.has_value()) std::_Exit(3); + for (const auto &status : inserted.value()) { + if (!status.ok()) std::_Exit(4); + } + std::_Exit(0); + }, + ::testing::ExitedWithCode(0), ""); + + auto opened = Collection::Open(dir_path_, CollectionOptions{}); + ASSERT_TRUE(opened.has_value()) << opened.error().message(); + auto collection = std::move(opened).value(); + EXPECT_EQ(collection->schema().value().name(), u8"恢复 集合"); + auto fetched = collection->fetch(ids); + ASSERT_TRUE(fetched.has_value()) << fetched.error().message(); + ASSERT_EQ(fetched.value().size(), ids.size()); + for (size_t i = 0; i < ids.size(); ++i) { + const auto found = fetched.value().find(ids[i]); + ASSERT_NE(found, fetched.value().end()); + ASSERT_NE(found->second, nullptr); + EXPECT_EQ(found->second->pk_ref(), ids[i]); + EXPECT_EQ(found->second->get("value"), static_cast(i)); + } + auto status = collection->flush(); + ASSERT_TRUE(status.ok()) << status.message(); + collection.reset(); + auto reopened = Collection::Open(dir_path_, CollectionOptions{}); + ASSERT_TRUE(reopened.has_value()) << reopened.error().message(); + EXPECT_EQ(reopened.value()->stats().value().doc_count, ids.size()); +} + TEST_F(CrashRecoveryTest, BasicInsertAndReopen) { { auto schema = CreateTestSchema(collection_name_); diff --git a/tests/db/index/common/doc_test.cc b/tests/db/index/common/doc_test.cc index e47174a43..32e875651 100644 --- a/tests/db/index/common/doc_test.cc +++ b/tests/db/index/common/doc_test.cc @@ -14,6 +14,7 @@ #include "zvec/db/doc.h" #include +#include #include #include #include @@ -770,7 +771,7 @@ TEST_F(DocDetailedTest, ValidateAndSanitization) { ASSERT_TRUE(s.ok()); } - // pk with characters inside the allowed set is accepted + // Previously valid ASCII IDs remain accepted, including the old boundary. { auto schema = test::TestHelper::CreateNormalSchema(false); std::vector valid_names = { @@ -805,8 +806,10 @@ TEST_F(DocDetailedTest, ValidateAndSanitization) { "file-name_v1.2", // -, _, . allowed "a-b_c.d!@#$%+=.", // all specials in one - // Max length = 64 + // Former and new length boundaries std::string(64, 'a'), + std::string(65, 'a'), + std::string(1024, 'a'), std::string(63, 'a') + "_", "_" + std::string(62, 'x') + ".", "!" + std::string(62, '0') + "@", @@ -819,16 +822,11 @@ TEST_F(DocDetailedTest, ValidateAndSanitization) { } } - // pk that is too long or uses disallowed characters is rejected + // External IDs can contain punctuation, spaces and UTF-8 text. { auto schema = test::TestHelper::CreateNormalSchema(false); - std::vector invalid_names = { - // Too long (>64) - std::string(65, 'a'), std::string(64, 'a') + "_", - - // Illegal characters - "a b", // space - "a&b", // & not in set + std::vector valid_names = { + " ", " padded ", "a b", "a&b", "a*b", // * "a(b)", // ( ) "a:b", // : @@ -851,12 +849,35 @@ TEST_F(DocDetailedTest, ValidateAndSanitization) { "a,b", // , "用户", // non-ASCII (Chinese) "αβγ", // Greek - "résumé", // accented chars (é not in [a-zA-Z]) + "résumé", // accented characters + }; + for (const auto &pk : valid_names) { + auto doc = test::TestHelper::CreateDoc(1, *schema, pk); + auto s = doc.validate_and_sanitize(schema); + ASSERT_TRUE(s.ok()) << "expected valid pk: " << pk << ": " << s.message(); + } + } + + // Invalid text must be rejected before it can enter storage. + { + auto schema = test::TestHelper::CreateNormalSchema(false); + const std::vector invalid_ids = { + "", + std::string(1025, 'a'), + std::string("a\0b", 3), + "a\nb", + "a\tb", + "a\rb", + std::string("\xff", 1), + std::string("\xe4\xb8", 2), }; - for (auto pk : invalid_names) { + for (const auto &pk : invalid_ids) { auto doc = test::TestHelper::CreateDoc(1, *schema, pk); + doc.set_pk(pk); // The helper generates a default ID for an empty input. auto s = doc.validate_and_sanitize(schema); - ASSERT_FALSE(s.ok()) << "expected invalid pk: " << pk; + ASSERT_EQ(s.code(), StatusCode::INVALID_ARGUMENT); + EXPECT_EQ(s.message().find("Invalid doc: "), 0u); + EXPECT_EQ(s.message().find("offset"), std::string::npos); } } } @@ -1317,12 +1338,21 @@ TEST(SearchQuery, ValidateAndSanitize) { v.size() * sizeof(float)); }; auto decode_idx = [](const std::string &buf) { - const auto *p = reinterpret_cast(buf.data()); - return std::vector(p, p + buf.size() / sizeof(uint32_t)); + EXPECT_EQ(buf.size() % sizeof(uint32_t), 0u); + std::vector indices(buf.size() / sizeof(uint32_t)); + if (!indices.empty()) { + std::memcpy(indices.data(), buf.data(), + indices.size() * sizeof(uint32_t)); + } + return indices; }; auto decode_val = [](const std::string &buf) { - const auto *p = reinterpret_cast(buf.data()); - return std::vector(p, p + buf.size() / sizeof(float)); + EXPECT_EQ(buf.size() % sizeof(float), 0u); + std::vector values(buf.size() / sizeof(float)); + if (!values.empty()) { + std::memcpy(values.data(), buf.data(), values.size() * sizeof(float)); + } + return values; }; FieldSchema schema = FieldSchema("field_name", DataType::SPARSE_VECTOR_FP32); @@ -1623,3 +1653,57 @@ TEST_F(DocDetailedTest, FieldExistenceChecks) { auto type_mismatch_opt = doc.get("existent"); EXPECT_FALSE(type_mismatch_opt.has_value()); } + + +TEST_F(DocDetailedTest, + LongFieldNamesKeepDistinctIdentitiesAcrossSerialization) { + const std::string prefix(32, 'f'); + const std::vector names{prefix, prefix + "a", + prefix + std::string(31, 'a') + "x", + prefix + std::string(31, 'a') + "y"}; + for (size_t i = 0; i < names.size(); ++i) { + ASSERT_TRUE(test_doc_->set(names[i], static_cast(i))); + } + const auto bytes = test_doc_->serialize(); + const auto restored = Doc::deserialize(bytes.data(), bytes.size()); + ASSERT_NE(restored, nullptr); + for (size_t i = 0; i < names.size(); ++i) { + const auto value = restored->get(names[i]); + ASSERT_TRUE(value.has_value()) << names[i]; + EXPECT_EQ(value.value(), static_cast(i)); + } +} + +TEST_F(DocDetailedTest, DeserializePreservesHistoricalTextWithoutRevalidation) { + Doc doc; + doc.set_pk(std::string("old\0id", 6)); + doc.set("old\nfield", std::string("\xff\0value", 7)); + const auto buffer = doc.serialize(); + const auto restored = Doc::deserialize(buffer.data(), buffer.size()); + ASSERT_NE(restored, nullptr); + EXPECT_EQ(restored->pk_ref(), doc.pk_ref()); + EXPECT_EQ(restored->get("old\nfield"), + doc.get("old\nfield")); +} + +TEST_F(DocDetailedTest, ValidationErrorsEscapeAndBoundDocumentAndFieldNames) { + auto schema = std::make_shared( + "test", FieldSchemaPtrList{ + std::make_shared("value", DataType::INT32)}); + for (const auto &name : + std::vector{std::string("bad\0field", 9), "bad\nfield", + "\xff", std::string(10000, 'x')}) { + Doc doc; + doc.set_pk(std::string(1024, 'd')); + doc.set(name, int32_t{42}); + const auto status = doc.validate_and_sanitize(schema); + EXPECT_EQ(status.code(), StatusCode::INVALID_ARGUMENT); + EXPECT_EQ(status.message().find("Invalid doc["), 0u); + EXPECT_NE(status.message().find("does not exist"), std::string::npos); + EXPECT_LT(status.message().size(), 256u); + for (unsigned char byte : status.message()) { + EXPECT_GE(byte, 0x20); + EXPECT_LE(byte, 0x7e); + } + } +} diff --git a/tests/db/index/common/identifier_validation_test.cc b/tests/db/index/common/identifier_validation_test.cc new file mode 100644 index 000000000..2446b5e8d --- /dev/null +++ b/tests/db/index/common/identifier_validation_test.cc @@ -0,0 +1,331 @@ +// Copyright 2025-present the zvec project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "db/index/common/identifier_validation.h" +#include +#include +#include +#include +#include +#include "db/common/utils.h" + +namespace zvec { +namespace { + +struct Utf8NameValidator { + Status (*validate)(std::string_view); + size_t max_bytes; + const char *prefix; +}; + +const std::array kUtf8NameValidators{{ + {validate_document_id, kMaxDocumentIdBytes, "Invalid doc: id"}, + {validate_collection_name, kMaxCollectionNameBytes, + "Invalid schema: collection name"}, +}}; + +void ExpectInvalid(const Status &status, const std::string &message) { + EXPECT_EQ(status.code(), StatusCode::INVALID_ARGUMENT); + EXPECT_EQ(status.message(), message); + EXPECT_EQ(status.message().find("offset"), std::string::npos); +} + +void ExpectInvalidName(const Utf8NameValidator &validator, + std::string_view value, const std::string &reason) { + ExpectInvalid( + validator.validate(value), + std::string(validator.prefix) + "[" + format_name(value) + "] " + reason); +} + +std::string Repeat(std::string_view text, size_t count) { + std::string result; + result.reserve(text.size() * count); + for (size_t i = 0; i < count; ++i) { + result.append(text.data(), text.size()); + } + return result; +} + +TEST(IdentifierValidationTest, AcceptsUnicodePunctuationAndSpaces) { + const std::vector values{ + "a", + "A_b-9", + "https://example.com/docs/1?lang=zh#section", + "a/b.c:d@e+f", + "'quoted' \"text\" \\ value", + " ", + " ", + " leading and trailing ", + u8"\u00A0\u3000", + u8"中文", + u8"€", + u8"😀", + u8"👩\u200D💻", + u8"é", + u8"e\u0301", + u8"\U0010FFFF", + }; + for (const auto &validator : kUtf8NameValidators) { + SCOPED_TRACE(validator.prefix); + for (const auto &value : values) { + EXPECT_TRUE(validator.validate(value).ok()); + } + } +} + +TEST(IdentifierValidationTest, RejectsEmptyNames) { + for (const auto &validator : kUtf8NameValidators) { + ExpectInvalid(validator.validate(std::string_view{}), + std::string(validator.prefix) + " must not be empty"); + } + ExpectInvalid(validate_field_name(""), + "Invalid schema: field name must not be empty"); +} + +TEST(IdentifierValidationTest, MeasuresLimitsInUtf8Bytes) { + for (const auto &validator : kUtf8NameValidators) { + SCOPED_TRACE(validator.prefix); + const auto max_bytes = validator.max_bytes; + EXPECT_TRUE(validator.validate(std::string(max_bytes, 'a')).ok()); + auto emoji = Repeat(u8"😀", max_bytes / 4); + ASSERT_EQ(emoji.size(), max_bytes); + EXPECT_TRUE(validator.validate(emoji).ok()); + EXPECT_TRUE( + validator.validate(std::string(max_bytes - 3, 'a') + u8"中").ok()); + + const auto reason = "exceeds " + std::to_string(max_bytes) + + " bytes (got " + std::to_string(max_bytes + 1) + ")"; + ExpectInvalidName(validator, std::string(max_bytes + 1, 'a'), reason); + ExpectInvalidName(validator, emoji + "a", reason); + ExpectInvalidName(validator, std::string(max_bytes - 2, 'a') + u8"中", + reason); + } +} + +TEST(IdentifierValidationTest, RejectsMalformedUtf8) { + const std::vector malformed{ + "\x80", // Isolated continuation byte. + "\xBF", + "\xC2", // Truncated sequences. + "\xE4\xB8", + "\xF0\x9F\x98", + "\xC2" + "A", // Invalid continuation byte. + "\xE2" + "A" + "\xAC", + "\xC0\x80", // Overlong NUL. + "\xC1\xBF", // Overlong two-byte sequence. + "\xE0\x80\xAF", // Overlong three-byte sequence. + "\xF0\x80\x80\xAF", // Overlong four-byte sequence. + "\xED\xA0\x80", // UTF-16 surrogate U+D800. + "\xED\xBF\xBF", // UTF-16 surrogate U+DFFF. + "\xF4\x90\x80\x80", // Above U+10FFFF. + "\xF5\x80\x80\x80", // Invalid lead byte. + "\xFE", + "\xFF", + }; + for (const auto &validator : kUtf8NameValidators) { + SCOPED_TRACE(validator.prefix); + for (const auto &value : malformed) { + ExpectInvalidName(validator, value, "is not valid UTF-8"); + ExpectInvalidName(validator, "prefix" + value, "is not valid UTF-8"); + } + } +} + +TEST(IdentifierValidationTest, HonorsStringViewLengthAndEmbeddedNulls) { + const std::string backing = std::string(u8"中文") + "\xFF"; + for (const auto &validator : kUtf8NameValidators) { + SCOPED_TRACE(validator.prefix); + EXPECT_TRUE(validator.validate(std::string_view(backing.data(), 6)).ok()); + ExpectInvalidName(validator, std::string_view(backing.data(), 5), + "is not valid UTF-8"); + ExpectInvalidName(validator, std::string("a\0b", 3), + "contains a null character"); + } +} + +TEST(IdentifierValidationTest, RejectsEveryC0AndC1ControlByCodepoint) { + for (const auto &validator : kUtf8NameValidators) { + SCOPED_TRACE(validator.prefix); + for (unsigned int codepoint = 0; codepoint <= 0x9F; ++codepoint) { + if (codepoint >= 0x20 && codepoint < 0x7F) { + continue; + } + SCOPED_TRACE(codepoint); + std::string value; + if (codepoint >= 0x80) { + value += '\xC2'; + } + value += static_cast(codepoint); + std::string reason = "contains a control character"; + if (codepoint == 0) { + reason = "contains a null character"; + } else if (codepoint == '\n' || codepoint == '\r') { + reason = "contains a newline"; + } else if (codepoint == '\t') { + reason = "contains a tab"; + } + ExpectInvalidName(validator, "a" + value + "b", reason); + } + // These continuation bytes overlap the C1 byte range, but their decoded + // codepoints are ordinary letters/symbols and must not be rejected. + EXPECT_TRUE(validator.validate(u8"中文€😀").ok()); + } +} + +TEST(IdentifierValidationTest, DistinguishesUnicodeLineAndParagraphSeparators) { + for (const auto &validator : kUtf8NameValidators) { + ExpectInvalidName(validator, u8"a\u2028b", "contains a line separator"); + ExpectInvalidName(validator, u8"a\u2029b", + "contains a paragraph separator"); + } +} + +TEST(IdentifierValidationTest, Utf8ErrorsIncludeEscapedAndBoundedPreviews) { + for (const auto &validator : kUtf8NameValidators) { + const std::string prefix = validator.prefix; + ExpectInvalid(validator.validate("order\n[123]"), + prefix + "[order\\n\\[123\\]] contains a newline"); + ExpectInvalid(validator.validate("order\xff"), + prefix + "[order\\xFF] is not valid UTF-8"); + ExpectInvalid(validator.validate(u8"订单\n"), + prefix + u8"[订单\\n] contains a newline"); + ExpectInvalid( + validator.validate(std::string(40, 'a') + "\n"), + prefix + "[" + std::string(32, 'a') + "...] contains a newline"); + const auto status = validator.validate(std::string(10000, '\xff')); + EXPECT_EQ(status.message().find(prefix + "[" + Repeat("\\xFF", 32) + + "...] exceeds "), + 0u); + EXPECT_LT(status.message().size(), 256u); + for (unsigned char byte : status.message()) { + EXPECT_GE(byte, 0x20); + EXPECT_LE(byte, 0x7E); + } + } +} + +TEST(IdentifierValidationTest, RetainsTheFieldAsciiCharacterSet) { + for (const std::string name : + {"a", "Z", "0", "_", "-", "a_b-c1", "123_test", "_zvec_custom"}) { + EXPECT_TRUE(validate_field_name(name).ok()); + } + EXPECT_TRUE(validate_field_name("ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "abcdefghijklmnopqrstuvwxyz0123456789_-") + .ok()); + EXPECT_TRUE(validate_field_name(std::string(kMaxFieldNameBytes, 'a')).ok()); + ExpectInvalid(validate_field_name(std::string(kMaxFieldNameBytes + 1, 'a')), + "Invalid schema: field[" + std::string(32, 'a') + + "...] exceeds 64 bytes (got 65)"); + ExpectInvalid(validate_field_name(std::string(10000, 'a')), + "Invalid schema: field[" + std::string(32, 'a') + + "...] exceeds 64 bytes (got 10000)"); +} + +TEST(IdentifierValidationTest, RejectsExactInternalFieldNames) { + for (const std::string name : + {"_zvec_row_id_", "_zvec_g_doc_id_", "_zvec_uid_", "_zvec_score", + "_zvec_group_id"}) { + SCOPED_TRACE(name); + ExpectInvalid(validate_field_name(name), + "Invalid schema: field[" + name + + "] is reserved; use a different name"); + // The restriction is an exact match, not a new prefix or case policy. + EXPECT_TRUE(validate_field_name(name + "_custom").ok()); + EXPECT_TRUE(validate_document_id(name).ok()); + EXPECT_TRUE(validate_collection_name(name).ok()); + } + EXPECT_TRUE(validate_field_name("_ZVEC_UID_").ok()); + EXPECT_TRUE(validate_field_name("_zvec_is_valid").ok()); +} + +TEST(IdentifierValidationTest, SharedErrorPreviewIsEscapedAndBounded) { + EXPECT_EQ(format_name(""), ""); + EXPECT_EQ(format_name(std::string("a\0\n\r\t[]\\", 8)), + "a\\0\\n\\r\\t\\[\\]\\\\"); + EXPECT_EQ(format_name(u8"中文€😀e\u0301"), u8"中文€😀e\u0301"); + EXPECT_EQ(format_name(u8"\u0085\u2028\u2029\u202E"), + "\\xC2\\x85\\xE2\\x80\\xA8\\xE2\\x80\\xA9\\xE2\\x80\\xAE"); + EXPECT_EQ(format_name(std::string(u8"中") + "\xff\xe4\xb8"), + u8"中\\xFF\\xE4\\xB8"); + EXPECT_EQ(format_name(std::string("\xff") + u8"中"), u8"\\xFF中"); + for (const std::string character : {u8"é", u8"中", u8"😀"}) { + auto prefix = std::string(32 - character.size(), 'x'); + EXPECT_EQ(format_name(prefix + character), prefix + character); + EXPECT_EQ(format_name(prefix + character + "tail"), + prefix + character + "..."); + EXPECT_EQ(format_name(prefix + "x" + character), prefix + "x..."); + } + EXPECT_EQ(format_name(std::string(30, 'x') + "\xe4\xb8"), + std::string(30, 'x') + "\\xE4\\xB8"); + EXPECT_EQ(format_name(std::string(31, 'x') + "\xc2" + "a"), + std::string(31, 'x') + "\\xC2..."); + EXPECT_EQ(format_name(std::string(10000, '\xff')), + Repeat("\\xFF", 32) + "..."); + EXPECT_EQ(format_name(std::string(10000, 'x')), std::string(32, 'x') + "..."); +} + +TEST(IdentifierValidationTest, + DescribesInvalidFieldCharactersWithSafePreviews) { + const std::string rule = + "; use letters (A-Z, a-z), digits, underscores (_) or hyphens (-)"; + ExpectInvalid(validate_field_name("user name"), + "Invalid schema: field[user name] contains a space" + rule); + ExpectInvalid( + validate_field_name("a.b"), + "Invalid schema: field[a.b] contains an unsupported character" + rule); + ExpectInvalid(validate_field_name(u8"中"), + "Invalid schema: field[中] contains a non-ASCII " + "character" + + rule); + ExpectInvalid( + validate_field_name("\x80"), + "Invalid schema: field[\\x80] contains a non-ASCII character" + rule); + ExpectInvalid( + validate_field_name(std::string("a\0b", 3)), + "Invalid schema: field[a\\0b] contains a null character" + rule); + ExpectInvalid(validate_field_name("a\nb"), + "Invalid schema: field[a\\nb] contains a newline" + rule); + ExpectInvalid(validate_field_name("a\tb"), + "Invalid schema: field[a\\tb] contains a tab" + rule); + ExpectInvalid( + validate_field_name("a\x1B" + "b"), + "Invalid schema: field[a\\x1Bb] contains a control character" + rule); + ExpectInvalid(validate_field_name("][\\\n"), + "Invalid schema: field[\\]\\[\\\\\\n] contains an unsupported " + "character" + + rule); +} + +TEST(IdentifierValidationTest, + BoundsInvalidFieldPreviewsAndNeverEchoesRawBytes) { + const std::string name(64, '\xFF'); + const auto status = validate_field_name(name); + ExpectInvalid(status, + "Invalid schema: field[" + Repeat("\\xFF", 32) + + "...] contains a non-ASCII character; use letters " + "(A-Z, a-z), digits, underscores (_) or hyphens (-)"); + EXPECT_LT(status.message().size(), 256u); + for (unsigned char byte : status.message()) { + EXPECT_GE(byte, 0x20); + EXPECT_LE(byte, 0x7E); + } +} + +} // namespace +} // namespace zvec diff --git a/tests/db/index/common/schema_test.cc b/tests/db/index/common/schema_test.cc index 84260d02d..f9877d57a 100644 --- a/tests/db/index/common/schema_test.cc +++ b/tests/db/index/common/schema_test.cc @@ -19,6 +19,79 @@ using namespace zvec; +TEST(CollectionSchemaTest, SkipsNullFieldObjectsDuringConstruction) { + CollectionSchema empty("schema", {nullptr}); + EXPECT_TRUE(empty.fields().empty()); + EXPECT_EQ(empty.validate().code(), StatusCode::INVALID_ARGUMENT); + + auto field = std::make_shared("valid", DataType::INT32); + CollectionSchema schema("schema", {nullptr, field, nullptr}); + ASSERT_EQ(schema.fields().size(), 1u); + ASSERT_NE(schema.get_field("valid"), nullptr); + EXPECT_EQ(schema.get_field("valid")->data_type(), DataType::INT32); + EXPECT_TRUE(schema.validate().ok()); +} + +TEST(CollectionSchemaTest, RejectsNullFieldObjectsInMutations) { + auto field = std::make_shared("valid", DataType::INT32); + CollectionSchema schema("schema", {field}); + const CollectionSchema before(schema); + EXPECT_EQ(schema.add_field(nullptr).code(), StatusCode::INVALID_ARGUMENT); + EXPECT_EQ(schema.alter_field("valid", nullptr).code(), + StatusCode::INVALID_ARGUMENT); + EXPECT_EQ(schema, before); + EXPECT_TRUE(schema.validate().ok()); +} + +TEST(CollectionSchemaTest, ValidatesDuplicateNamesFromConstructorsAndCopies) { + auto scalar = std::make_shared("duplicate", DataType::INT32); + auto other_scalar = + std::make_shared("duplicate", DataType::INT64); + auto vector = std::make_shared("duplicate", + DataType::VECTOR_FP32, 4, false); + auto other_vector = std::make_shared( + "duplicate", DataType::VECTOR_FP32, 8, false); + const std::vector cases{ + {scalar, std::make_shared(*scalar)}, + {scalar, other_scalar}, + {scalar, scalar}, + {vector, other_vector}, + {scalar, vector}, + {vector, scalar}, + {nullptr, scalar, nullptr, vector}}; + for (size_t i = 0; i < cases.size(); ++i) { + SCOPED_TRACE(i); + CollectionSchema schema("schema", cases[i]); + ASSERT_EQ(schema.fields().size(), 2u); + auto status = schema.validate(); + EXPECT_EQ(status.code(), StatusCode::INVALID_ARGUMENT); + EXPECT_EQ(status.message(), + "Invalid schema: duplicate field name [duplicate]; field names " + "must be unique"); + CollectionSchema copied(schema); + CollectionSchema assigned( + "old", {std::make_shared("existing", DataType::INT32)}); + assigned = schema; + EXPECT_EQ(copied.validate(), status); + EXPECT_EQ(assigned.validate(), status); + EXPECT_EQ(copied.fields().size(), 2u); + EXPECT_EQ(assigned.fields().size(), 2u); + EXPECT_FALSE(assigned.has_field("existing")); + } +} + +TEST(CollectionSchemaTest, CopyOwnsIndependentFieldObjects) { + auto original_field = + std::make_shared("value", DataType::INT32, true); + CollectionSchema schema("schema", {original_field}); + original_field->set_name("invalid name"); + original_field->set_data_type(DataType::STRING); + EXPECT_TRUE(schema.validate().ok()); + ASSERT_NE(schema.get_field("value"), nullptr); + EXPECT_EQ(schema.get_field("value")->data_type(), DataType::INT32); + EXPECT_FALSE(schema.has_field("invalid name")); +} + TEST(FieldSchemaTest, DefaultConstructor) { FieldSchema field; EXPECT_EQ(field.name(), ""); @@ -554,7 +627,9 @@ TEST(FieldSchemaTest, Validate) { "user_name", "test-123", "aBc123_-", - std::string(32, 'a'), // max len = 32 + std::string(32, 'a'), + std::string(33, 'a'), + std::string(64, 'a'), // max len = 64 "a_b-c1", "__test__", "123_test"}; @@ -571,7 +646,7 @@ TEST(FieldSchemaTest, Validate) { { std::vector invalid_names = { "", // empty — len < 1 - std::string(33, 'a'), // len > 32 + std::string(65, 'a'), // len > 64 "a b", // space "a.b", "a@b", @@ -888,7 +963,7 @@ TEST(CollectionSchemaTest, Validate) { ASSERT_EQ(s.code(), StatusCode::INVALID_ARGUMENT); CollectionSchema c2("c2", {}); - s = c1.validate(); + s = c2.validate(); ASSERT_FALSE(s.ok()); ASSERT_EQ(s.code(), StatusCode::INVALID_ARGUMENT); @@ -901,8 +976,7 @@ TEST(CollectionSchemaTest, Validate) { auto f2 = std::make_shared("f2", DataType::INT32); CollectionSchema c4("c4", {f2}); s = c4.validate(); - ASSERT_FALSE(s.ok()); - ASSERT_EQ(s.code(), StatusCode::INVALID_ARGUMENT); + ASSERT_TRUE(s.ok()); auto f3 = std::make_shared("f3", DataType::VECTOR_FP16); CollectionSchema c5("c5", {f3}); @@ -910,41 +984,45 @@ TEST(CollectionSchemaTest, Validate) { ASSERT_FALSE(s.ok()); ASSERT_EQ(s.code(), StatusCode::INVALID_ARGUMENT); - // validate collection name regex "^[a-zA-Z0-9_-]{3,32}$" + // Collection names are bounded UTF-8 strings, independent of the path. { std::vector invalid_names = { - "", // empty - "ab", // too short (<3) - std::string(65, 'a'), // too long (>64) - "a b", // space not allowed - "a.b", // dot not allowed - "a$b", // $ not allowed - "中文", // non-ASCII - "a\nb", // newline not allowed - "a\tb", // tab not allowed - "a\rb", // carriage return not allowed + "", // empty + std::string(257, 'a'), // too long (>256 bytes) + std::string("a\0b", 3), // embedded NUL + std::string("\xff", 1), // invalid UTF-8 + "a\nb", // newline not allowed + "a\tb", // tab not allowed + "a\rb", // carriage return not allowed }; for (const auto &name : invalid_names) { CollectionSchema c(name, {field}); s = c.validate(); - if (!s.ok()) { - std::cout << "Invalid name: " << name << std::endl; - } ASSERT_FALSE(s.ok()); ASSERT_EQ(s.code(), StatusCode::INVALID_ARGUMENT); } std::vector valid_names = { "test_collection_supported_vectors", + "a", + "ab", std::string(64, 'a'), + std::string(65, 'a'), + std::string(256, 'a'), + "a b", + "a.b", + "a$b", + "中文", + " ", + " padded ", "a_b", // underscore allowed "a-b", // dash allowed "a_1", // underscore and digit allowed "a-1", // dash and digit allowed "a_1b", // underscore, digit and letter allowed "a-1b", // dash, digit and letter allowed - "-start", // allowed! (regex permits leading -/_) + "-start", // leading -/_ remains allowed "_start", // also allowed "end-", "end_", // trailing -/_ allowed