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

Replace rbtree with b+tree #25

Merged
merged 9 commits into from
Aug 6, 2021
Merged

Replace rbtree with b+tree #25

merged 9 commits into from
Aug 6, 2021

Conversation

M1Les
Copy link
Contributor

@M1Les M1Les commented Aug 3, 2021

This contains two changes.

  1. Adding an alternative tree library (ordered-btree) to use for indexing inside of in-memory provider. We noticed that red-black-tree has a bug in it that causes it to go into an infinite loop when the tree is being rebalanced. This change makes it possible to choose between the two libraries when database is created.
  2. A small fix for non-unique indices. When an item is removed from the store, non-unique index it is part of, will be reset to undefined:
    image

ind.remove will remove all values from the index, even if there are multiple items in that index (in case of non-unique index).

In this fix we're splitting remove operation in two steps for non-unique indices:

  • Find the index value (array of items with the same index)
  • Using primary key, locate and remove the item from the array

There are possibly some performance implications to doing array.filter to remove the item, but could not come up with a better way of removing item without iterating over all array items -- yes, we can stop comparing pks after the first match, but we still need to go over the rest of items to finish copying the array.

Copy link
Contributor

@thomastay thomastay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left some feedback

@thomastay
Copy link
Contributor

Also, looks like the CI build is failing because of lint issues with prettier

@M1Les
Copy link
Contributor Author

M1Les commented Aug 5, 2021

Yeah, it was failing before because of the other PR, it was not able to reuse the same version tag. Still fixing it

Sudhar123
Sudhar123 previously approved these changes Aug 5, 2021
Copy link
Contributor

@Sudhar123 Sudhar123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a comment above the remove for non-unique/unique indices

Copy link
Contributor

@thomastay thomastay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments look good

@M1Les M1Les merged commit 88643bb into master Aug 6, 2021
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.

None yet

3 participants