diff --git a/python/tests/detail/test_collection_dql.py b/python/tests/detail/test_collection_dql.py index f4804f2..d25ab4e 100644 --- a/python/tests/detail/test_collection_dql.py +++ b/python/tests/detail/test_collection_dql.py @@ -13,28 +13,24 @@ # limitations under the License. -from zvec.typing import DataType, StatusCode, MetricType, QuantizeType +from distance_helper import * +from doc_helper import * +from fixture_helper import * +from params_helper import * +from zvec import StatusCode +from zvec.extension import QwenReRanker, RrfReRanker, WeightedReRanker from zvec.model import Collection, Doc, VectorQuery from zvec.model.param import ( CollectionOption, - InvertIndexParam, - HnswIndexParam, FlatIndexParam, - IVFIndexParam, + HnswIndexParam, HnswQueryParam, + InvertIndexParam, + IVFIndexParam, IVFQueryParam, ) - - from zvec.model.schema import FieldSchema, VectorSchema -from zvec.extension import RrfReRanker, WeightedReRanker, QwenReRanker -from distance_helper import * - -from zvec import StatusCode -from distance_helper import * -from fixture_helper import * -from doc_helper import * -from params_helper import * +from zvec.typing import DataType, MetricType, QuantizeType, StatusCode # ==================== helper ==================== @@ -848,6 +844,9 @@ class TestCollectionQuery: @pytest.mark.parametrize("doc_num", [10]) @pytest.mark.parametrize("topk", [1024]) @pytest.mark.parametrize("filter", ["int32_field >= 3 and int32_field <= 7"]) + @pytest.mark.parametrize( + "full_schema_new", [(True, True, HnswIndexParam())], indirect=True + ) def test_query_vector_with_HnswQueryParam_valid( self, full_collection_new: Collection, @@ -916,6 +915,9 @@ class TestCollectionQuery: @pytest.mark.parametrize("doc_num", [10]) @pytest.mark.parametrize("topk", [10]) @pytest.mark.parametrize("filter", ["int32_field >= 3 and int32_field <= 7"]) + @pytest.mark.parametrize( + "full_schema_ivf", [(True, True, IVFIndexParam())], indirect=True + ) def test_query_vector_with_IVFQueryParam_valid( self, full_collection_ivf: Collection, nprobe, doc_num, topk, filter ): diff --git a/python/tests/test_collection.py b/python/tests/test_collection.py index 7d021d6..d22be2b 100644 --- a/python/tests/test_collection.py +++ b/python/tests/test_collection.py @@ -13,7 +13,6 @@ # limitations under the License. from __future__ import annotations - import pytest import zvec from zvec import ( @@ -23,15 +22,15 @@ from zvec import ( Doc, FieldSchema, HnswIndexParam, + IndexOption, + IndexType, InvertIndexParam, LogLevel, LogType, - VectorSchema, - StatusCode, - IndexOption, - IndexType, - VectorQuery, OptimizeOption, + StatusCode, + VectorQuery, + VectorSchema, ) # ==================== Common ==================== @@ -469,10 +468,9 @@ class TestCollectionInsert: vectors={"dense": [1 + 0.1] * 128, "sparse": {1: 1.0, 2: 2.0, 3: 3.0}}, ) with pytest.raises(ValueError) as e: - # ValueError: doc validate failed: field[id] is configured not nullable, - # but doc does not contain this field + # ValueError: Invalid doc: field[id] is required but not provided test_collection.insert(doc) - assert "field[id] is configured not nullable" in str(e.value) + assert "field[id] is required but not provided" in str(e.value) # without name doc = Doc( @@ -484,7 +482,7 @@ class TestCollectionInsert: ) with pytest.raises(ValueError) as e: test_collection.insert(doc) - assert "field[name] is configured not nullable" in str(e.value) + assert "field[name] is required but not provided" in str(e.value) def test_collection_insert_with_nullable_true_field(self, test_collection): # id, name's nullable == False @@ -591,8 +589,7 @@ class TestCollectionUpdate: fields={"id": None}, ) with pytest.raises(ValueError) as e: - # ValueError: doc validate failed: field[id] is configured not nullable, - # but doc does not contain this field + # ValueError: Invalid doc: field[id] is required but its value is null collection_with_single_doc.update(doc) doc = Doc( diff --git a/src/db/index/common/doc.cc b/src/db/index/common/doc.cc index deb4a58..bcb401d 100644 --- a/src/db/index/common/doc.cc +++ b/src/db/index/common/doc.cc @@ -696,23 +696,24 @@ Doc::Ptr Doc::deserialize(const uint8_t *data, size_t /*size*/) { Status Doc::validate(const CollectionSchema::Ptr &schema, bool is_update) const { if (!schema) { - return Status::InternalError("doc validate failed: schema is null"); + return Status::InternalError("schema is null during doc validation"); } if (pk_.empty()) { - return Status::InvalidArgument("doc validate failed: doc_id is not set"); + return Status::InvalidArgument("Invalid doc: id (primary key) is not set"); } if (!std::regex_match(pk_, DOC_PK_REGEX)) { - return Status::InvalidArgument("doc validate failed: doc_id[", pk_, - "] cannot pass the regex verification"); + return Status::InvalidArgument("Invalid doc: doc[", pk_, + "] contains invalid characters"); } // check doc fields match schema for (auto &[name, value] : fields_) { if (!schema->has_field(name)) { - return Status::InvalidArgument("doc validate failed: field[", name, - "] does not exist in collection's schema"); + return Status::InvalidArgument( + "Invalid doc[", pk_, "]: field[", name, + "] does not exist in the collection schema"); } } @@ -724,17 +725,17 @@ Status Doc::validate(const CollectionSchema::Ptr &schema, if (field_schema->nullable() || is_update) { continue; } - return Status::InvalidArgument( - "doc validate failed: field[", field_name, - "] is configured not nullable, but doc does not contain this field"); + return Status::InvalidArgument("Invalid doc[", 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( - "doc validate failed: field[", field_name, - "] is configured not nullable, but doc's field value is empty"); + return Status::InvalidArgument("Invalid doc[", pk_, "]: field[", + field_name, + "] is required but its value is null"); } } @@ -864,14 +865,14 @@ Status Doc::validate(const CollectionSchema::Ptr &schema, field_value); if (sparse_values.size() != sparse_indices.size()) { return Status::InvalidArgument( - "doc validate failed: field[", field_name, - "]'s sparse vector indices and values size not match"); + "Invalid doc[", pk_, "]: sparse vector field[", field_name, + "] has mismatched indices and values sizes"); } if (sparse_indices.size() > kSparseMaxDimSize) { return Status::InvalidArgument( - "doc validate failed: vector[", field_name, - "], the number of sparse indices exceeds the maximum limit ", - kSparseMaxDimSize); + "Invalid doc[", pk_, "]: sparse vector field[", field_name, + "] exceeds the maximum number of sparse indices (", + kSparseMaxDimSize, ")"); } } break; @@ -885,38 +886,38 @@ Status Doc::validate(const CollectionSchema::Ptr &schema, field_value); if (sparse_values.size() != sparse_indices.size()) { return Status::InvalidArgument( - "doc validate failed: field[", field_name, - "]'s sparse vector indices and values size not match"); + "Invalid doc[", pk_, "]: sparse vector field[", field_name, + "] has mismatched indices and values sizes"); } if (sparse_indices.size() > kSparseMaxDimSize) { return Status::InvalidArgument( - "doc validate failed: vector[", field_name, - "], the number of sparse indices exceeds the maximum limit ", - kSparseMaxDimSize); + "Invalid doc[", pk_, "]: sparse vector field[", field_name, + "] exceeds the maximum number of sparse indices (", + kSparseMaxDimSize, ")"); } } break; } default: - return Status::InvalidArgument("doc validate failed: field[", + return Status::InvalidArgument("Invalid doc[", pk_, "]: field[", field_name, - "]'s value type is not supported"); + "] has unsupported data type"); break; } if (!type_match) { return Status::InvalidArgument( - "doc validate failed: field[", field_name, - "]'s value type mismatch, it should be ", - DataTypeCodeBook::AsString(expected_type), ", but got type: ", + "Invalid doc[", pk_, "]: field[", field_name, + "] type mismatch, expected ", + DataTypeCodeBook::AsString(expected_type), " but got ", get_value_type_name(field_value, field_schema->is_vector_field())); } if (field_schema->is_dense_vector()) { if (value_dimension != field_schema->dimension()) { return Status::InvalidArgument( - "doc validate failed: field[", field_name, - "]'s dimension mismatch, it should be ", field_schema->dimension(), - ", but got dimension: ", value_dimension); + "Invalid doc[", pk_, "]: field[", field_name, + "] dimension mismatch, expected ", field_schema->dimension(), + " but got ", value_dimension); } } } @@ -1216,18 +1217,19 @@ Status VectorQuery::validate(const FieldSchema *schema) const { if (schema == nullptr) { if (query_vector_.empty() && query_sparse_indices_.empty()) { - // No vector provided — this is a scalar-only filter query. + // Scalar-only filter query return Status::OK(); } else { // If a query vector was provided, the field must exist as a vector field - // since we are doing vector similarity search. + // since we are performing a vector similarity search. return Status::InvalidArgument( "Invalid query: query vector is provided, but query field[", field_name_, "] does not exist or is not a vector field in the collection"); } } - // validate dense/sparse vector + + // Vector query if (schema->is_dense_vector()) { // Validate dimension auto dim = schema->dimension(); @@ -1282,6 +1284,15 @@ Status VectorQuery::validate(const FieldSchema *schema) const { return Status::InvalidArgument("Invalid query: field[", field_name_, "] is not a vector field"); } + // Validate query_params type + if (query_params_ && query_params_->type() != schema->index_type()) { + return Status::InvalidArgument( + "Invalid query: query params type does not match the index type of " + "vector field[", + field_name_, "], expected ", + IndexTypeCodeBook::AsString(schema->index_type()), " but got ", + IndexTypeCodeBook::AsString(query_params_->type())); + } return Status::OK(); } diff --git a/tests/db/index/common/doc_test.cc b/tests/db/index/common/doc_test.cc index 9098478..986d766 100644 --- a/tests/db/index/common/doc_test.cc +++ b/tests/db/index/common/doc_test.cc @@ -15,7 +15,6 @@ #include "zvec/db/doc.h" #include #include -#include #include #include #include "utils/utils.h" @@ -786,7 +785,8 @@ TEST_F(DocDetailedTest, Validate) { auto schema = test::TestHelper::CreateNormalSchema(false); std::vector invalid_names = { // Too long (>64) - std::string(65, 'a'), std::string(64, 'a') + "_", + std::string(65, 'a'), + std::string(64, 'a') + "_", // Illegal characters "a b", // space @@ -1270,6 +1270,33 @@ TEST(VectorQuery, Validate) { s = query.validate(&schema); EXPECT_TRUE(s.ok()); } + + // validate query_params type matches index type + { + VectorQuery query; + query.field_name_ = "embedding"; + query.topk_ = 10; + std::vector query_vector(128, 1.0f); + query.query_vector_ = + std::string(reinterpret_cast(query_vector.data()), + query_vector.size() * sizeof(float)); + FieldSchema schema = + FieldSchema("embedding", DataType::VECTOR_FP32, 128, false, + std::make_shared(MetricType::L2)); + + query.query_params_ = std::make_shared(150); + auto s = query.validate(&schema); + EXPECT_TRUE(s.ok()); + + query.query_params_ = std::make_shared(50); + s = query.validate(&schema); + EXPECT_FALSE(s.ok()); + EXPECT_EQ(s.code(), StatusCode::INVALID_ARGUMENT); + + query.query_params_ = nullptr; + s = query.validate(&schema); + EXPECT_TRUE(s.ok()); + } } // Test null value