branch-4.1: [fix](be) Key the index lookup map by each surviving index in remove_index #66316 - #66393
Open
github-actions[bot] wants to merge 1 commit into
Open
branch-4.1: [fix](be) Key the index lookup map by each surviving index in remove_index #66316#66393github-actions[bot] wants to merge 1 commit into
github-actions[bot] wants to merge 1 commit into
Conversation
…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.
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
|
run buildall |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cherry-picked from #66316