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

Btree random insertion test #364

Closed
wants to merge 9 commits into from
Closed

Btree random insertion test #364

wants to merge 9 commits into from

Conversation

friendlymatthew
Copy link
Collaborator

No description provided.

Comment on lines +178 to +181
// this should be the leaf node
if int(parent.Size()) > t.PageFile.PageSize() {
panic("too fat!")
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When we reach the point where we finally serialize the leaf node post top-down traversal, that leaf size can exceed a page. I was going to split, but is creating an intermediate node with only one element (the mid key) be sufficient?

Copy link
Owner

Choose a reason for hiding this comment

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

Hmm I think splitting while traversing downwards won't work because of this problem. Adding an intermediate node isn't great because there is a degenerate sequence of inserts that will always trigger an intermediate node insertion leading to bad asymptotic performance.

Specifically, right now you're splitting the tree as you're traversing down to the leaf node but instead it might be easier to insert it first and then propagate the split back up the tree. This would fix the issue as you wouldn't need to create an intermediate node.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good

Copy link

github-actions bot commented Jul 1, 2024

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.

2 participants