diff --git a/src/db/collection.cc b/src/db/collection.cc index 33750f3..165e8b7 100644 --- a/src/db/collection.cc +++ b/src/db/collection.cc @@ -498,7 +498,7 @@ Status CollectionImpl::CreateIndex(const std::string &column_name, // forbidden writing until index is ready std::lock_guard write_lock(write_mtx_); - if (writing_segment_->doc_count() > 0) { + if (writing_segment_->has_record()) { s = switch_to_new_segment_for_writing(); CHECK_RETURN_STATUS(s); } @@ -671,7 +671,7 @@ Status CollectionImpl::DropIndex(const std::string &column_name) { // forbidden writing until index is ready std::lock_guard write_lock(write_mtx_); - if (writing_segment_->doc_count() > 0) { + if (writing_segment_->has_record()) { s = switch_to_new_segment_for_writing(); CHECK_RETURN_STATUS(s); } @@ -791,8 +791,8 @@ Status CollectionImpl::Optimize(const OptimizeOptions &options) { // forbidden writing for a while std::lock_guard write_lock(write_mtx_); - if (writing_segment_->doc_count() != 0) { - // flush and create new segment + if (writing_segment_->has_record()) { + // Flush pending records and switch only when the segment contains docs. auto s = switch_to_new_segment_for_writing(); if (!s.ok()) { return s; @@ -1151,7 +1151,7 @@ Status CollectionImpl::AddColumn(const FieldSchema::Ptr &column_schema, s = new_schema->add_field(column_schema); CHECK_RETURN_STATUS(s); - if (writing_segment_->doc_count() > 0) { + if (writing_segment_->has_record()) { s = switch_to_new_segment_for_writing(); CHECK_RETURN_STATUS(s); } @@ -1223,7 +1223,7 @@ Status CollectionImpl::DropColumn(const std::string &column_name) { s = new_schema->drop_field(column_name); CHECK_RETURN_STATUS(s); - if (writing_segment_->doc_count() > 0) { + if (writing_segment_->has_record()) { s = switch_to_new_segment_for_writing(); CHECK_RETURN_STATUS(s); } @@ -1307,7 +1307,7 @@ Status CollectionImpl::AlterColumn(const std::string &column_name, s = new_schema->alter_field(column_name, new_field_schema); CHECK_RETURN_STATUS(s); - if (writing_segment_->doc_count() > 0) { + if (writing_segment_->has_record()) { s = switch_to_new_segment_for_writing(); CHECK_RETURN_STATUS(s); } @@ -1516,6 +1516,10 @@ Status CollectionImpl::commit_schema_change_with_new_writing_segment( Status CollectionImpl::switch_to_new_segment_for_writing( const CollectionSchema::Ptr &schema) { + if (writing_segment_->doc_count() == 0) { + return writing_segment_->flush(); + } + auto s = writing_segment_->dump(); CHECK_RETURN_STATUS(s); diff --git a/src/db/index/segment/segment.cc b/src/db/index/segment/segment.cc index a4f2e5a..984d736 100644 --- a/src/db/index/segment/segment.cc +++ b/src/db/index/segment/segment.cc @@ -122,6 +122,8 @@ class SegmentImpl : public Segment, uint64_t doc_count(const IndexFilter::Ptr filter = nullptr) override; + bool has_record() override; + Status Insert(Doc &doc) override; Status Update(Doc &doc) override; @@ -599,6 +601,10 @@ uint64_t SegmentImpl::doc_count(const IndexFilter::Ptr filter) { return doc_count; } +bool SegmentImpl::has_record() { + return doc_count() > 0 || (wal_file_ != nullptr && wal_file_->has_record()); +} + template struct is_vector : std::false_type {}; diff --git a/src/db/index/segment/segment.h b/src/db/index/segment/segment.h index 0b3a8b8..8826f2b 100644 --- a/src/db/index/segment/segment.h +++ b/src/db/index/segment/segment.h @@ -68,6 +68,8 @@ class Segment { // Count documents visible to an optional global-doc-ID filter. virtual uint64_t doc_count(const IndexFilter::Ptr filter = nullptr) = 0; + virtual bool has_record() = 0; + // ---- Schema and index mutation ----------------------------------------- virtual Status add_column(FieldSchema::Ptr column_schema, const std::string &expression, diff --git a/tests/db/collection_test.cc b/tests/db/collection_test.cc index d837bb5..f98ef44 100644 --- a/tests/db/collection_test.cc +++ b/tests/db/collection_test.cc @@ -5199,6 +5199,42 @@ TEST_F(CollectionTest, Feature_AddColumn_WithUnflushedData) { ASSERT_EQ(stats.doc_count, doc_count + 1000); } +TEST_F(CollectionTest, Feature_AddColumn_WithDeleteOnlyWritingSegment) { + auto schema = TestHelper::CreateNormalSchema(); + auto options = CollectionOptions{false, true, 64 * 1024 * 1024}; + auto collection = TestHelper::CreateCollectionWithDoc(col_path, *schema, + options, 0, 10, false); + + auto setup_field = + std::make_shared("setup_col", DataType::INT32, true); + auto s = collection->AddColumn(setup_field, "", AddColumnOptions()); + ASSERT_TRUE(s.ok()) << s.message(); + + auto deleted_pk = TestHelper::MakePK(0); + auto delete_result = collection->Delete({deleted_pk}); + ASSERT_TRUE(delete_result.has_value()) << delete_result.error().message(); + ASSERT_TRUE(delete_result.value()[0].ok()); + auto fetch_result = collection->Fetch({deleted_pk}); + ASSERT_TRUE(fetch_result.has_value()) << fetch_result.error().message(); + ASSERT_EQ(fetch_result.value()[deleted_pk], nullptr); + ASSERT_EQ(collection->Stats().value().doc_count, 9u); + + auto trigger_field = + std::make_shared("trigger_col", DataType::INT64, true); + s = collection->AddColumn(trigger_field, "", AddColumnOptions()); + ASSERT_TRUE(s.ok()) << s.message(); + + collection.reset(); + auto open_result = Collection::Open(col_path, options); + ASSERT_TRUE(open_result.has_value()) << open_result.error().message(); + collection = open_result.value(); + + fetch_result = collection->Fetch({deleted_pk}); + ASSERT_TRUE(fetch_result.has_value()) << fetch_result.error().message(); + ASSERT_EQ(fetch_result.value()[deleted_pk], nullptr); + ASSERT_EQ(collection->Stats().value().doc_count, 9u); +} + TEST_F(CollectionTest, Feature_ColumnDDL_ChainedOps_MultiSegment) { int docs_per_segment = 1000; int num_segments = 3; diff --git a/tests/db/sqlengine/mock_segment.h b/tests/db/sqlengine/mock_segment.h index 130c879..d1c6499 100644 --- a/tests/db/sqlengine/mock_segment.h +++ b/tests/db/sqlengine/mock_segment.h @@ -382,6 +382,10 @@ class MockSegment : public Segment { return 0; } + bool has_record() override { + return false; + } + Status add_column(FieldSchema::Ptr column_schema, const std::string &expression, const AddColumnOptions &options) override {