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

Update RRB size table after draining in push_chunk #75

Merged
merged 1 commit into from
Mar 11, 2019

Conversation

krobelus
Copy link
Contributor

@krobelus krobelus commented Mar 8, 2019

When a chunk does not fully fit in a node, we drain as many elements from the
chunk into the node and later push the leftover chunk to a sibling.

This commit makes sure that the size field of the parent node is updated
correctly. For dense nodes, we can simply add the number of drained elements to
the size. If there is a size table, we add that number to the size entry
corresponding to the current element and all entries to the right.

Closes #74

@krobelus krobelus force-pushed the issue_74 branch 2 times, most recently from 903f2ba to a956d1a Compare March 8, 2019 18:45
@krobelus
Copy link
Contributor Author

krobelus commented Mar 8, 2019

not sure if it's ok to add #![feature(stmt_expr_attributes)] for the #[rustfmt::skip]

When a chunk does not fully fit in a node, we drain as many elements from the
chunk into the node and later push the leftover chunk to a sibling.

This commit makes sure that the size field of the parent node is updated
correctly. For dense nodes, we can simply add the number of drained elements to
the size. If there is a size table, we add that number to the size entry
corresponding to the current element and all entries to the right.

Closes bodil#74
@bodil
Copy link
Owner

bodil commented Mar 11, 2019

No, #![feature] is going to break the build on non-nightly. It's annoying that the CI hasn't run on this PR - I moved it to CircleCI recently, which I've only just discovered defaults to not building PRs unless you tell it to.

It looks very good otherwise, I'll merge and unbreak the build later.

@bodil bodil merged commit 417fa4b into bodil:master Mar 11, 2019
@bodil
Copy link
Owner

bodil commented Mar 11, 2019

Oh, and you seem to have fixed all the bugs uncovered by the big integration test I wrote before the weekend. I think I owe you a beverage of your choice or some such. 😁👍

@krobelus
Copy link
Contributor Author

Cool! I'm happy to help :)
I may have some more coming up soon, as I'm fuzzing Vector's public API atm

@krobelus krobelus deleted the issue_74 branch March 12, 2019 13:02
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