Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(manifest): add unit tests for manifest by cpp. #19

Merged
merged 1 commit into from
Jul 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion cpp/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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 . -s build_type=Debug
clean:
rm -rf build

Expand All @@ -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
Expand Down
8 changes: 7 additions & 1 deletion cpp/src/common/result.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 <typename E>
static Result<E> FromArrowResult(arrow::Result<E> arrow_result);
Expand Down
5 changes: 5 additions & 0 deletions cpp/src/common/utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Result<schema_proto::LogicType> ToProtobufType(arrow::Type::type type) {

std::unique_ptr<schema_proto::KeyValueMetadata> ToProtobufMetadata(const arrow::KeyValueMetadata* metadata) {
auto proto_metadata = std::make_unique<schema_proto::KeyValueMetadata>();
assert(metadata != nullptr);
for (const auto& key : metadata->keys()) {
proto_metadata->add_keys(key);
}
Expand All @@ -45,6 +46,10 @@ Result<std::unique_ptr<schema_proto::Field>> 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<const arrow::FixedSizeBinaryType*>(type);
auto fixed_size_binary_type = new schema_proto::FixedSizeBinaryType();
Expand Down
4 changes: 4 additions & 0 deletions cpp/src/file/fragment.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string> files_;
Expand Down
1 change: 1 addition & 0 deletions cpp/src/storage/schema.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ Result<std::unique_ptr<schema_proto::Schema>> Schema::ToProtobuf() {

Status Schema::FromProtobuf(const schema_proto::Schema& schema) {
ASSIGN_OR_RETURN_NOT_OK(schema_, FromProtobufSchema(schema.arrow_schema()));
options_ = std::make_shared<SchemaOptions>();
options_->FromProtobuf(schema.schema_options());
RETURN_NOT_OK(BuildScalarSchema());
RETURN_NOT_OK(BuildVectorSchema());
Expand Down
2 changes: 1 addition & 1 deletion cpp/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
130 changes: 95 additions & 35 deletions cpp/test/manifest_test.cpp
Original file line number Diff line number Diff line change
@@ -1,44 +1,104 @@
#include <arrow/type.h>
#include <arrow/array/builder_primitive.h>
#include <numeric>
#include "gtest/gtest.h"
#include "storage/manifest.h"
#include "gmock/gmock.h"
#include "google/protobuf/util/message_differencer.h"
#include <arrow/util/key_value_metadata.h>

using ::testing::ElementsAre;

namespace milvus_storage {
// TEST(ManifestTest, ManifestGetSetTest) {
// std::shared_ptr<SpaceOptions> options = std::make_shared<SpaceOptions>();
// std::shared_ptr<Schema> schema = std::make_shared<Schema>();
// 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<SpaceOptions> options = std::make_shared<SpaceOptions>();
// options->uri = "file:///tmp/test";
// std::shared_ptr<Schema> schema = std::make_shared<Schema>();
// 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> options = std::make_shared<Options>();
std::shared_ptr<Schema> schema = std::make_shared<Schema>();
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<arrow::KeyValueMetadata> metadata = arrow::KeyValueMetadata::Make(
std::vector<std::string>{"key1", "key2"}, std::vector<std::string>{"value1", "value2"});

std::shared_ptr<arrow::Field> pk_field = arrow::field("pk_field", arrow::int64(), /*nullable=*/false, metadata);

std::shared_ptr<arrow::Field> ts_field = arrow::field("ts_field", arrow::int64(), /*nullable=*/false, metadata);

std::shared_ptr<arrow::Field> 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<std::string>{"key1", "key2"}, std::vector<std::string>{"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>();
options->uri = "file:///tmp/test";
auto schema_options = std::make_shared<SchemaOptions>();
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<Schema>(arrow_schema, schema_options);
auto space_schema2 = std::make_shared<Schema>(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
1 change: 0 additions & 1 deletion cpp/test/space_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down