fix(fts): allow global doc id gaps during compaction (#588)

src/db/index/column/fts_column/fts_rocksdb_reducer.cc

@@ -132,12 +132,14 @@ Result<void> FtsRocksdbReducer::feed(
     return {};
   }
 
-  // Require consecutive global doc_id ranges between non-empty segments so
-  // the shared delete_row_id_bitmap stays aligned with input scan order.
+  // Global doc_id gaps are valid after deleted documents have been removed
+  // during compaction.  Only require ordered, non-overlapping ranges; the
+  // shared delete bitmap is aligned by feed-order scan position and doc_count.
   if (!segment_stats_.empty() &&
-      segment_stats.min_doc_id != segment_stats_.back().max_doc_id + 1) {
+      segment_stats.min_doc_id <= segment_stats_.back().max_doc_id) {
     return tl::make_unexpected(Status::InternalError(
-        "FtsRocksdbReducer: segments not in consecutive doc_id order. field=",
+        "FtsRocksdbReducer: segments have overlapping or unordered doc_id "
+        "ranges. field=",
         field_name_));
   }
 

src/db/index/column/fts_column/fts_rocksdb_reducer.h

@@ -52,7 +52,9 @@ class FtsRocksdbReducer {
   Result<void> cleanup();
 
   /*! Feed a source segment to be merged.
-   *  Segments must be fed in consecutive doc_id order.
+   *  Segments must be fed in ascending, non-overlapping global doc_id order.
+   *  Gaps between global doc_id ranges are allowed because deleted documents
+   *  may already have been removed by an earlier compaction.
    *  \param segment_stats       Stats of the source segment (min/max doc_id)
    *  \param src_ctx             RocksdbContext owning the source CFs
    *  \param src_postings_cf     Source postings CF (must be BitPacked)

src/db/index/column/fts_column/fts_types.h

@@ -37,8 +37,8 @@ struct FtsQueryParams {
 };
 
 /*! Per-segment statistics needed by the FTS reducer for doc_id remapping.
- *  - min_doc_id / max_doc_id: GLOBAL doc_id range used by the delete filter
- *    (filter.is_filtered() takes a global doc_id).
+ *  - min_doc_id / max_doc_id: GLOBAL doc_id range used to order segments and
+ *    reject overlapping inputs.  Gaps are valid after compaction deletes.
  *  - doc_count: number of FTS LOCAL doc_ids in the source segment; the posting
  *    list domain is [0, doc_count).  For fresh (non-merged) segments this
  *    equals max_doc_id - min_doc_id + 1, and the local-to-global mapping is

tests/db/collection_test.cc

@@ -5899,6 +5899,69 @@ TEST_F(CollectionTest, Feature_NoVectorCollection_FtsLifecycle) {
   FileHelper::RemoveDirectory(col_path);
 }
 
+TEST_F(CollectionTest, Feature_FtsOptimizeAcceptsGlobalDocIdGaps) {
+  FileHelper::RemoveDirectory(col_path);
+
+  auto schema = std::make_shared<CollectionSchema>("fts_optimize_gaps");
+  schema->set_max_doc_count_per_segment(1000);
+  schema->add_field(std::make_shared<FieldSchema>(
+      "content", DataType::STRING, false, std::make_shared<FtsIndexParams>()));
+
+  auto create_res = Collection::CreateAndOpen(col_path, *schema,
+                                              CollectionOptions{false, true});
+  ASSERT_TRUE(create_res.has_value()) << create_res.error().message();
+  auto col = std::move(create_res.value());
+
+  for (uint64_t batch = 0; batch < 3; ++batch) {
+    std::vector<Doc> docs;
+    docs.reserve(1000);
+    for (uint64_t i = 0; i < 1000; ++i) {
+      uint64_t id = batch * 1000 + i;
+      Doc doc;
+      doc.set_pk("pk_" + std::to_string(id));
+      doc.set<std::string>("content", "hello boundary");
+      docs.emplace_back(std::move(doc));
+    }
+    ASSERT_TRUE(col->Insert(docs).has_value());
+  }
+
+  auto delete_ranges = [&](uint64_t offset, uint64_t count) {
+    std::vector<std::string> pks;
+    pks.reserve(count * 3);
+    for (uint64_t base : {0, 1000, 2000}) {
+      for (uint64_t i = 0; i < count; ++i) {
+        pks.emplace_back("pk_" + std::to_string(base + offset + i));
+      }
+    }
+    auto result = col->Delete(pks);
+    ASSERT_TRUE(result.has_value()) << result.error().message();
+  };
+
+  // The first rebuild removes the head of each source segment, producing
+  // persisted global ranges [400, 999], [1400, 1999], [2400, 2999].
+  delete_ranges(0, 400);
+  ASSERT_TRUE(col->Optimize().ok());
+  ASSERT_EQ(col->Stats().value().doc_count, 1800u);
+
+  // A second round raises the delete ratio above the rebuild threshold and
+  // merges segments whose global ranges contain legitimate delete gaps.
+  delete_ranges(400, 200);
+  ASSERT_TRUE(col->Optimize().ok());
+  ASSERT_EQ(col->Stats().value().doc_count, 1200u);
+
+  SearchQuery query;
+  query.target_.field_name_ = "content";
+  query.topk_ = 10;
+  FtsClause fts;
+  fts.query_string_ = "hello";
+  query.target_.clause_ = fts;
+  auto result = col->Query(query);
+  ASSERT_TRUE(result.has_value()) << result.error().message();
+  ASSERT_EQ(result.value().size(), 10u);
+
+  ASSERT_TRUE(col->Destroy().ok());
+}
+
 TEST_F(CollectionTest, Feature_NoVectorCollection_FtsReopenWithoutOptimize) {
   FileHelper::RemoveDirectory(col_path);
 

tests/db/index/column/fts_column/fts_rocksdb_reducer_test.cc

@@ -333,22 +333,35 @@ TEST_F(FtsRocksdbReducerTest, FeedFailsBeforeInit) {
                    .has_value());
 }
 
-TEST_F(FtsRocksdbReducerTest, FeedFailsWithNonConsecutiveDocIds) {
+TEST_F(FtsRocksdbReducerTest, FeedAcceptsGapBetweenGlobalDocIdRanges) {
   FtsRocksdbReducer reducer = MakeReducer();
 
   FtsSegmentStats stats0 = MakeSegmentStats(0, 2);
   EXPECT_TRUE(reducer.feed(stats0, &src0_db_, src0_postings_, src0_positions_)
                   .has_value());
 
-  // Gap: src1 starts at 4 instead of 3
+  // Deletes may leave a gap between persisted segments.  The reducer remaps
+  // segment-local doc_ids by feed-order scan position, not by global doc_id.
   FtsSegmentStats stats1 = MakeSegmentStats(4, 6);
+  EXPECT_TRUE(reducer.feed(stats1, &src1_db_, src1_postings_, src1_positions_)
+                  .has_value());
+}
+
+TEST_F(FtsRocksdbReducerTest, FeedFailsWithOverlappingGlobalDocIdRanges) {
+  FtsRocksdbReducer reducer = MakeReducer();
+
+  FtsSegmentStats stats0 = MakeSegmentStats(0, 2);
+  EXPECT_TRUE(reducer.feed(stats0, &src0_db_, src0_postings_, src0_positions_)
+                  .has_value());
+
+  FtsSegmentStats stats1 = MakeSegmentStats(2, 4);
   EXPECT_FALSE(reducer.feed(stats1, &src1_db_, src1_postings_, src1_positions_)
                    .has_value());
 }
 
 TEST_F(FtsRocksdbReducerTest, FeedAcceptsEmptySegmentAsNoop) {
   // Empty segments (doc_count == 0) silently contribute nothing — the
-  // surrounding non-empty segments still get their contiguity validated
+  // surrounding non-empty segments still get their ordering validated
   // against each other, as if the empty one wasn't there.
   auto indexer0 = MakeSrc0Indexer();
   InsertDocs(indexer0.get(), {{0, "hello world"}, {1, "foo"}, {2, "bar"}});
@@ -361,7 +374,7 @@ TEST_F(FtsRocksdbReducerTest, FeedAcceptsEmptySegmentAsNoop) {
   ASSERT_TRUE(reducer.feed(stats0, &src0_db_, src0_postings_, src0_positions_)
                   .has_value());
 
-  // Empty middle segment — accepted, doesn't break contiguity.
+  // Empty middle segment — accepted, doesn't affect ordering.
   FtsSegmentStats empty_stats;
   empty_stats.min_doc_id = 0;
   empty_stats.max_doc_id = 0;
@@ -370,8 +383,8 @@ TEST_F(FtsRocksdbReducerTest, FeedAcceptsEmptySegmentAsNoop) {
       reducer.feed(empty_stats, &src1_db_, src1_postings_, src1_positions_)
           .has_value());
 
-  // src1 must still start at stats0.max_doc_id + 1 = 3, not be shifted by
-  // the (skipped) empty segment.
+  // The next non-empty segment is checked against stats0, not against the
+  // skipped empty segment.
   FtsSegmentStats stats1 = MakeSegmentStats(3, 3);
   ASSERT_TRUE(reducer.feed(stats1, &src1_db_, src1_postings_, src1_positions_)
                   .has_value());