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

bugfix: 'node_16<T>::grow()' partial_key index offset inconsistent #20

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bowling233
Copy link

This issue was discovered when using the iterator to traverse and output text.

The functions in node_48.hpp will all add an offset of 128 when indexing using partial_key, for example:

node_48<T>::find_child(char partial_key){
    uint8_t index = indexes_[128 + partial_key];
    //...
}

However, node_16<T>::grow() ignores this point and directly uses the partial_key stored in keys_ as the index, causing an offset in indexing when the node grows:

template <class T> inner_node<T> *node_16<T>::grow() {
  //...
  for (int i = 0; i < n_children_; ++i) {
    new_node->indexes_[(uint8_t) this->keys_[i]] = i;
  }  //...
}

For example, consider a node_16 instance:

node_16:
keys_: [a, b, ...]
ASCII: [97, 98, ...]
node_16<T>::grow()
indexes_: [..., 0, ..., EMPTY, ...]
        wrong [97]  correct[97+128]

When querying a in node_48, find_child() will index indexes_[128 + 97], but the value there is EMPTY. Moreover, this error will also affect the correctness of functions like node_48<T>::next_partial_key, causing iterators to obtain incorrect values of keys_, and so on.

Screenshot 2024-03-14 at 10 29 11 PM

The iterator in the figure obtained an incorrect value of -55 through node_48<T>::next_partial_key. This node was grown from a node_16, where the original key stored at that position was I (73). node_48<T>::next_partial_key(-128) start from -128+128 with an offset of 73 to get I, but partial_key remains -128+73=-55.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant