This is an automated email from the ASF dual-hosted git repository.
airborne12 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 19c32c56a2f [fix](be) Key the index lookup map by each surviving index
in remove_index (#66316)
19c32c56a2f is described below
commit 19c32c56a2f586394b199772a51dff8c2e105f00
Author: Jack <[email protected]>
AuthorDate: Mon Aug 3 20:24:10 2026 +0800
[fix](be) Key the index lookup map by each surviving index in remove_index
(#66316)
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
`TabletSchema::remove_index` rebuilds the `(index_type, col_unique_id,
index_suffix) -> position` lookup map after dropping an entry. The loop
walks each surviving index, but builds every map key from
`_indexes.back()` instead of the entry being registered:
```cpp
for (size_t new_pos = 0; new_pos < _indexes.size(); ++new_pos) {
const auto& index = _indexes[new_pos];
...
IndexKey key = std::make_tuple(_indexes.back()->index_type(), col_uid,
_indexes.back()->get_index_suffix());
_col_id_suffix_to_index[key].push_back(new_pos);
}
```
So every survivor is filed under the LAST survivor's index type and
suffix. When the survivors are homogeneous -- e.g. several INVERTED
indexes with no suffix, which is what the existing coverage uses --
every
key is identical and the result is accidentally correct. When they
differ,
it is not: after dropping one of two INVERTED indexes on a table that
also
carries an NGRAM_BF index, the surviving INVERTED index is filed under
`(NGRAM_BF, col, "")`, and `inverted_indexs()` -- which looks up
`IndexType::INVERTED` -- no longer finds it. A surviving index then
becomes
invisible to callers that resolve indexes through this map, including
the
segment writer and index compaction, with no error reported. The same
applies to indexes that differ only in suffix (variant sub-column
indexes).
The fix keys each entry by its own `index_type()` /
`get_index_suffix()`.
This also makes `remove_index` consistent with the other three sites
that
populate the same map (`append_index`, `init_from_pb`, and the
column-append path): those iterate right after
`_indexes.emplace_back(...)`,
where `_indexes.back()` IS the entry being registered, so they are
correct
as written and are left unchanged.
The map is a runtime cache rebuilt from scratch by `init_from_pb`, so a
schema that round-trips through protobuf is unaffected; only an
in-memory
schema that keeps being used after `remove_index` sees the stale
mapping.
---
be/src/storage/tablet/tablet_schema.cpp | 4 +-
be/test/storage/tablet/tablet_schema_test.cpp | 56 +++++++++++++++++++++++++++
2 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/be/src/storage/tablet/tablet_schema.cpp
b/be/src/storage/tablet/tablet_schema.cpp
index 44928c0b169..13ea91d032d 100644
--- a/be/src/storage/tablet/tablet_schema.cpp
+++ b/be/src/storage/tablet/tablet_schema.cpp
@@ -979,8 +979,8 @@ void TabletSchema::remove_index(int64_t index_id) {
auto& pattern_to_index_map =
_index_by_unique_id_with_pattern[col_uid];
pattern_to_index_map[field_pattern].emplace_back(index);
} else {
- IndexKey key = std::make_tuple(_indexes.back()->index_type(),
col_uid,
-
_indexes.back()->get_index_suffix());
+ IndexKey key =
+ std::make_tuple(index->index_type(), col_uid,
index->get_index_suffix());
_col_id_suffix_to_index[key].push_back(new_pos);
}
}
diff --git a/be/test/storage/tablet/tablet_schema_test.cpp
b/be/test/storage/tablet/tablet_schema_test.cpp
index 6b29eea1d55..30828d8a3a3 100644
--- a/be/test/storage/tablet/tablet_schema_test.cpp
+++ b/be/test/storage/tablet/tablet_schema_test.cpp
@@ -368,6 +368,62 @@ TEST_F(TabletSchemaTest,
test_tablet_column_protobuf_roundtrip) {
deserialized.variant_enable_typed_paths_to_sparse());
}
+// remove_index() rebuilds the (index_type, col_uid, suffix) -> position lookup
+// map after dropping an entry. Every surviving index must be filed under ITS
OWN
+// key. The existing coverage below uses three INVERTED indexes with no suffix,
+// where every key is identical, so it cannot tell a correct rebuild from one
+// that keys every entry off the LAST surviving index.
+//
+// This pins the heterogeneous case: after the drop the survivors are an
+// INVERTED index followed by an NGRAM_BF index. If the rebuild takes the key
+// from _indexes.back(), the INVERTED index is filed under (NGRAM_BF, ...) and
+// inverted_indexs() -- which looks up IndexType::INVERTED -- stops seeing it,
+// making a surviving index invisible to the segment writer and to compaction.
+TEST_F(TabletSchemaTest,
test_remove_index_keeps_heterogeneous_survivors_findable) {
+ TabletSchema schema;
+
+ TabletColumn text_col;
+ text_col.set_unique_id(9001);
+ text_col.set_name("text_col");
+ text_col.set_type(FieldType::OLAP_FIELD_TYPE_STRING);
+ schema.append_column(text_col);
+
+ TabletColumn code_col;
+ code_col.set_unique_id(9002);
+ code_col.set_name("code_col");
+ code_col.set_type(FieldType::OLAP_FIELD_TYPE_STRING);
+ schema.append_column(code_col);
+
+ auto add_index = [&](int64_t index_id, IndexType type, int32_t col_uid) {
+ TabletIndex index;
+ TabletIndexPB index_pb;
+ index_pb.set_index_id(index_id);
+ index_pb.set_index_name("hetero_idx_" + std::to_string(index_id));
+ index_pb.set_index_type(type);
+ index_pb.add_col_unique_id(col_uid);
+ index.init_from_pb(index_pb);
+ schema.append_index(std::move(index));
+ };
+
+ add_index(500, IndexType::INVERTED, 9001); // survives
+ add_index(501, IndexType::INVERTED, 9001); // dropped below
+ add_index(502, IndexType::NGRAM_BF, 9002); // survives, and becomes back()
+
+ ASSERT_EQ(2, schema.inverted_indexs(9001, "").size());
+
+ schema.remove_index(501);
+
+ // The surviving INVERTED index must still be reachable by its own key.
+ auto survivors = schema.inverted_indexs(9001, "");
+ ASSERT_EQ(1, survivors.size())
+ << "surviving INVERTED index became invisible after remove_index;
the lookup map was "
+ "rebuilt using the last survivor's (index_type, suffix) instead
of each entry's own";
+ EXPECT_EQ(500, survivors[0]->index_id());
+
+ // The NGRAM_BF survivor must not be reported as an inverted index on its
column.
+ EXPECT_TRUE(schema.inverted_indexs(9002, "").empty());
+}
+
TEST_F(TabletSchemaTest, test_tablet_schema_remove_and_clear_index) {
TabletSchema schema;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
