diff --git a/cpp/Makefile b/cpp/Makefile index 082ede2..5ed2843 100644 --- a/cpp/Makefile +++ b/cpp/Makefile @@ -9,7 +9,10 @@ build: conan install . -if build --build=missing conan install . conan build . - +debug: + conan install . -if build --build=missing -s build_type=Debug + conan install . + conan build . --build=missing --configure -s build_type=Debug clean: rm -rf build @@ -20,6 +23,7 @@ test: format: find ./src -type f ! -name "*.pb.h" -iname *.h -o -iname *.cpp | xargs clang-format -i + find ./test -type f ! -name "*.pb.h" -iname *.h -o -iname *.cpp | xargs clang-format -i tidy-check: build diff --git a/cpp/src/common/result.h b/cpp/src/common/result.h index f5b3d7a..e79b69c 100644 --- a/cpp/src/common/result.h +++ b/cpp/src/common/result.h @@ -73,7 +73,13 @@ class Result { return std::move(value_.value()); } - Status& status() { return status_.value(); } + Status& status() { + if (!status_.has_value() && value_.has_value()) { + status_ = Status::OK(); + } + assert(status_.has_value()); + return status_.value(); + } template static Result FromArrowResult(arrow::Result arrow_result); diff --git a/cpp/src/common/utils.cpp b/cpp/src/common/utils.cpp index b189649..4d371f1 100644 --- a/cpp/src/common/utils.cpp +++ b/cpp/src/common/utils.cpp @@ -21,6 +21,7 @@ Result ToProtobufType(arrow::Type::type type) { std::unique_ptr ToProtobufMetadata(const arrow::KeyValueMetadata* metadata) { auto proto_metadata = std::make_unique(); + assert(metadata != nullptr); for (const auto& key : metadata->keys()) { proto_metadata->add_keys(key); } @@ -45,6 +46,10 @@ Result> ToProtobufField(const arrow::Field* Status SetTypeValues(schema_proto::DataType* proto_type, const arrow::DataType* type) { switch (type->id()) { + case arrow::Type::INT64: { + proto_type->set_logic_type(schema_proto::LogicType::INT64); + break; + } case arrow::Type::FIXED_SIZE_BINARY: { auto real_type = dynamic_cast(type); auto fixed_size_binary_type = new schema_proto::FixedSizeBinaryType(); diff --git a/cpp/src/file/fragment.h b/cpp/src/file/fragment.h index e989a14..e5a8b4a 100644 --- a/cpp/src/file/fragment.h +++ b/cpp/src/file/fragment.h @@ -33,6 +33,10 @@ class Fragment { void set_id(int64_t id); + bool operator==(const Fragment& other) const { + return (fragment_id_ == other.fragment_id_ && files_ == other.files_); + } + private: std::int64_t fragment_id_; std::vector files_; diff --git a/cpp/src/storage/schema.cpp b/cpp/src/storage/schema.cpp index b08a267..b06f51f 100644 --- a/cpp/src/storage/schema.cpp +++ b/cpp/src/storage/schema.cpp @@ -38,6 +38,7 @@ Result> Schema::ToProtobuf() { Status Schema::FromProtobuf(const schema_proto::Schema& schema) { ASSIGN_OR_RETURN_NOT_OK(schema_, FromProtobufSchema(schema.arrow_schema())); + options_ = std::make_shared(); options_->FromProtobuf(schema.schema_options()); RETURN_NOT_OK(BuildScalarSchema()); RETURN_NOT_OK(BuildVectorSchema()); diff --git a/cpp/test/CMakeLists.txt b/cpp/test/CMakeLists.txt index b63a7e2..61a2465 100644 --- a/cpp/test/CMakeLists.txt +++ b/cpp/test/CMakeLists.txt @@ -5,7 +5,7 @@ add_executable( options_test.cpp schema_test.cpp manifest_test.cpp -) + space_test.cpp) target_link_libraries( milvus_test storage GTest::gtest_main diff --git a/cpp/test/manifest_test.cpp b/cpp/test/manifest_test.cpp index 887794e..c8cc4f0 100644 --- a/cpp/test/manifest_test.cpp +++ b/cpp/test/manifest_test.cpp @@ -1,44 +1,104 @@ +#include +#include +#include #include "gtest/gtest.h" #include "storage/manifest.h" #include "gmock/gmock.h" #include "google/protobuf/util/message_differencer.h" +#include using ::testing::ElementsAre; namespace milvus_storage { -// TEST(ManifestTest, ManifestGetSetTest) { -// std::shared_ptr options = std::make_shared(); -// std::shared_ptr schema = std::make_shared(); -// Manifest manifest(options, schema); -// manifest.add_scalar_files({"scalar_file1", "scalar_file2"}); -// manifest.add_vector_files({"vector_file1", "vector_file2"}); -// manifest.add_delete_file("delete_file"); -// ASSERT_THAT(manifest.scalar_files(), ElementsAre("scalar_file1", "scalar_file2")); -// ASSERT_THAT(manifest.vector_files(), ElementsAre("vector_file1", "vector_file2")); -// ASSERT_THAT(manifest.delete_files(), ElementsAre("delete_file")); -// ASSERT_EQ(manifest.schema(), schema); -// ASSERT_EQ(manifest.space_options(), options); -// } -// -// TEST(ManifestTest, ManifestProtoTest) { -// std::shared_ptr options = std::make_shared(); -// options->uri = "file:///tmp/test"; -// std::shared_ptr schema = std::make_shared(); -// Manifest manifest(options, schema); -// auto proto_manifest = manifest.ToProtobuf(); -// manifest.add_scalar_files({"scalar_file"}); -// manifest.add_vector_files({"vector_file"}); -// manifest.add_delete_file("delete_file"); -// -// manifest_proto::Manifest expected_proto_manifest; -// expected_proto_manifest.set_allocated_options(options->ToProtobuf().release()); -// auto status = schema->ToProtobuf(); -// ASSERT_TRUE(status.ok()); -// expected_proto_manifest.set_allocated_schema(status.value().release()); -// expected_proto_manifest.add_scalar_files("scalar_file"); -// expected_proto_manifest.add_vector_files("vector_file"); -// expected_proto_manifest.add_delete_files("delete_file"); -// -// ASSERT_TRUE(google::protobuf::util::MessageDifferencer::Equals(expected_proto_manifest, proto_manifest.value())); -// } +TEST(ManifestTest, ManifestGetSetTest) { + std::shared_ptr options = std::make_shared(); + std::shared_ptr schema = std::make_shared(); + Manifest manifest(options, schema); + + Fragment fragment1(1); + fragment1.add_file("scalar_file1"); + fragment1.add_file("scalar_file2"); + manifest.add_scalar_fragment(std::move(fragment1)); + + Fragment fragment2(2); + fragment2.add_file("vector_file1"); + fragment2.add_file("vector_file2"); + + manifest.add_vector_fragment(std::move(fragment2)); + + Fragment fragment3(3); + fragment3.add_file("delete_file"); + manifest.add_delete_fragment(std::move(fragment3)); + + ASSERT_THAT(manifest.scalar_fragments(), ElementsAre(fragment1)); + ASSERT_THAT(manifest.vector_fragments(), ElementsAre(fragment2)); + ASSERT_THAT(manifest.delete_fragments(), ElementsAre(fragment3)); + ASSERT_EQ(manifest.schema(), schema); + ASSERT_EQ(manifest.space_options(), options); +} + +TEST(ManifestTest, ManifestProtoTest) { + // Create Fields + std::shared_ptr metadata = arrow::KeyValueMetadata::Make( + std::vector{"key1", "key2"}, std::vector{"value1", "value2"}); + + std::shared_ptr pk_field = arrow::field("pk_field", arrow::int64(), /*nullable=*/false, metadata); + + std::shared_ptr ts_field = arrow::field("ts_field", arrow::int64(), /*nullable=*/false, metadata); + + std::shared_ptr vec_field = + arrow::field("vec_field", arrow::fixed_size_binary(10), /*nullable=*/false, metadata); + + // Create Arrow Schema + arrow::SchemaBuilder schema_builder; + auto status = schema_builder.AddField(pk_field); + ASSERT_TRUE(status.ok()); + status = schema_builder.AddField(ts_field); + ASSERT_TRUE(status.ok()); + status = schema_builder.AddField(vec_field); + ASSERT_TRUE(status.ok()); + auto schema_metadata = + arrow::KeyValueMetadata(std::vector{"key1", "key2"}, std::vector{"value1", "value2"}); + auto metadata_status = schema_builder.AddMetadata(schema_metadata); + ASSERT_TRUE(metadata_status.ok()); + auto arrow_schema = schema_builder.Finish().ValueOrDie(); + + // Create Options + auto options = std::make_shared(); + options->uri = "file:///tmp/test"; + auto schema_options = std::make_shared(); + schema_options->primary_column = "pk_field"; + schema_options->version_column = "ts_field"; + schema_options->vector_column = "vec_field"; + + // Create Schema + auto space_schema1 = std::make_shared(arrow_schema, schema_options); + auto space_schema2 = std::make_shared(arrow_schema, schema_options); + + auto sp_status = space_schema1->Validate(); + ASSERT_TRUE(sp_status.ok()); + + // Create Manifest + Manifest manifest1(options, space_schema1); + // Add Fragments + Fragment fragment1(1); + fragment1.add_file("test_file1"); + fragment1.add_file("test_file2"); + manifest1.add_scalar_fragment(std::move(fragment1)); + manifest1.add_vector_fragment(std::move(fragment1)); + manifest1.add_delete_fragment(std::move(fragment1)); + // Serialize Manifest + auto proto_manifest = manifest1.ToProtobuf(); + ASSERT_TRUE(proto_manifest.ok()); + + // Create Manifest + Manifest manifest2(options, space_schema2); + // Deserialize Manifest + manifest2.FromProtobuf(proto_manifest.value()); + + // Compare Manifests + ASSERT_EQ(manifest2.delete_fragments().size(), manifest1.delete_fragments().size()); + ASSERT_EQ(manifest2.scalar_fragments().size(), manifest1.scalar_fragments().size()); + ASSERT_EQ(manifest2.vector_fragments().size(), manifest1.vector_fragments().size()); +} } // namespace milvus_storage diff --git a/cpp/test/space_test.cpp b/cpp/test/space_test.cpp index 48e01f3..374d490 100644 --- a/cpp/test/space_test.cpp +++ b/cpp/test/space_test.cpp @@ -7,7 +7,6 @@ #include "arrow/array/builder_binary.h" #include "arrow/array/builder_primitive.h" #include "filter/constant_filter.h" -#include "storage/default_space.h" #include "gtest/gtest.h" TEST(SpaceTest, SpaceCtor) {