-
Notifications
You must be signed in to change notification settings - Fork 7
Unsized node bug fix #103
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
Unsized node bug fix #103
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,107 +1 @@ | ||
| // use futures::executor::block_on; | ||
| // use worktable::prelude::*; | ||
| // use worktable::worktable; | ||
| // | ||
| // #[tokio::main] | ||
| // async fn main() { | ||
| // // describe WorkTable | ||
| // worktable!( | ||
| // name: My, | ||
| // persist: true, | ||
| // columns: { | ||
| // id: u64 primary_key autoincrement, | ||
| // val: i64, | ||
| // test: i32, | ||
| // attr: String, | ||
| // attr2: i32, | ||
| // attr_float: f64, | ||
| // attr_string: String, | ||
| // | ||
| // }, | ||
| // indexes: { | ||
| // idx1: attr, | ||
| // idx2: attr2 unique, | ||
| // idx3: attr_string, | ||
| // }, | ||
| // queries: { | ||
| // update: { | ||
| // ValById(val) by id, | ||
| // AllAttrById(attr, attr2) by id, | ||
| // UpdateOptionalById(test) by id, | ||
| // }, | ||
| // delete: { | ||
| // ByAttr() by attr, | ||
| // ById() by id, | ||
| // } | ||
| // } | ||
| // ); | ||
| // | ||
| // // Init Worktable | ||
| // let config = PersistenceConfig::new("data", "data"); | ||
| // let my_table = MyWorkTable::new(config).await.unwrap(); | ||
| // | ||
| // // WT rows (has prefix My because of table name) | ||
| // let row = MyRow { | ||
| // val: 777, | ||
| // attr: "Attribute0".to_string(), | ||
| // attr2: 345, | ||
| // test: 1, | ||
| // id: 0, | ||
| // attr_float: 100.0, | ||
| // attr_string: "String_attr0".to_string(), | ||
| // }; | ||
| // | ||
| // for i in 2..1000000_i64 { | ||
| // let row = MyRow { | ||
| // val: 777, | ||
| // attr: format!("Attribute{}", i), | ||
| // attr2: 345 + i as i32, | ||
| // test: i as i32, | ||
| // id: i as u64, | ||
| // attr_float: 100.0 + i as f64, | ||
| // attr_string: format!("String_attr{}", i), | ||
| // }; | ||
| // | ||
| // my_table.insert(row).unwrap(); | ||
| // } | ||
| // | ||
| // // insert | ||
| // let pk: MyPrimaryKey = my_table.insert(row).expect("primary key"); | ||
| // | ||
| // // Select ALL records from WT | ||
| // let _select_all = my_table.select_all().execute(); | ||
| // //println!("Select All {:?}", select_all); | ||
| // | ||
| // // Select All records with attribute TEST | ||
| // let _select_all = my_table.select_all().execute(); | ||
| // //println!("Select All {:?}", select_all); | ||
| // | ||
| // // Select by Idx | ||
| // //let _select_by_attr = my_table | ||
| // // .select_by_attr("Attribute1".to_string()) | ||
| // // .execute() | ||
| // //r .unwrap(); | ||
| // | ||
| // //for row in select_by_attr { | ||
| // // println!("Select by idx, row {:?}", row); | ||
| // //} | ||
| // | ||
| // // Update Value query | ||
| // let update = my_table.update_val_by_id(ValByIdQuery { val: 1337 }, pk.clone()); | ||
| // let _ = block_on(update); | ||
| // | ||
| // let _select_all = my_table.select_all().execute(); | ||
| // //println!("Select after update val {:?}", select_all); | ||
| // | ||
| // let delete = my_table.delete(pk); | ||
| // let _ = block_on(delete); | ||
| // | ||
| // let _select_all = my_table.select_all().execute(); | ||
| // //println!("Select after delete {:?}", select_all); | ||
| // | ||
| // let info = my_table.system_info(); | ||
| // | ||
| // println!("{info}"); | ||
| // } | ||
|
|
||
| fn main() {} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,7 @@ where | |
| { | ||
| inner: Vec<T>, | ||
| length_capacity: usize, | ||
| length_without_deleted: usize, | ||
| removed_length: usize, | ||
| length: usize, | ||
| } | ||
|
|
||
|
|
@@ -45,9 +45,23 @@ where | |
| inner, | ||
| length, | ||
| length_capacity, | ||
| length_without_deleted: length, | ||
| removed_length: 0, | ||
| } | ||
| } | ||
|
|
||
| pub fn rebuild(&mut self) { | ||
| self.length = self | ||
| .inner | ||
| .last() | ||
| .expect("should not rebuild on empty node") | ||
| .aligned_size(); | ||
| self.length += UNSIZED_HEADER_LENGTH as usize; | ||
| for value in self.inner.iter() { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does the size contains the last item twice?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Node's id is stored separately. This unsized node logic is needed to mirror state of |
||
| self.length += value.aligned_size(); | ||
| self.length += UnsizedIndexPageUtility::<T>::slots_value_size(); | ||
| } | ||
| self.removed_length = 0; | ||
| } | ||
| } | ||
|
|
||
| impl<T> NodeLike<T> for UnsizedNode<T> | ||
|
|
@@ -59,7 +73,7 @@ where | |
| inner: Vec::new(), | ||
| length_capacity: capacity, | ||
| length: UNSIZED_HEADER_LENGTH as usize, | ||
| length_without_deleted: UNSIZED_HEADER_LENGTH as usize, | ||
| removed_length: 0, | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -68,18 +82,20 @@ where | |
| } | ||
|
|
||
| fn halve(&mut self) -> Self { | ||
| let middle_length = (self.length_without_deleted | ||
| let middle_length = (self.length | ||
| - self.removed_length | ||
| - (self.max().unwrap().aligned_size() + UNSIZED_HEADER_LENGTH as usize)) | ||
| / 2; | ||
| let current_node_id_size = self.max().unwrap().aligned_size(); | ||
| let mut middle_variance = f64::INFINITY; | ||
| let mut ind = false; | ||
| let mut i = 1; | ||
| let mut current_length = 0; | ||
| let mut middle_idx = 0; | ||
| let mut iter = self.inner.iter(); | ||
| while !ind { | ||
| let val = iter.next().expect("we should stop before node's end"); | ||
| let Some(val) = iter.next() else { | ||
| break; | ||
| }; | ||
| current_length += val.aligned_size(); | ||
| current_length += UnsizedIndexPageUtility::<T>::slots_value_size(); | ||
| let current_middle_variance = | ||
|
|
@@ -96,19 +112,8 @@ where | |
| } | ||
|
|
||
| let new_inner = self.inner.split_off(middle_idx); | ||
| let node_id_len = new_inner.last().unwrap().aligned_size(); | ||
| let split = Self { | ||
| inner: new_inner, | ||
| length_capacity: self.length_capacity, | ||
| length: self.length_without_deleted - (current_node_id_size + current_length) | ||
| + node_id_len, | ||
| length_without_deleted: self.length_without_deleted | ||
| - (current_node_id_size + current_length) | ||
| + node_id_len, | ||
| }; | ||
| self.length = | ||
| current_length + self.max().unwrap().aligned_size() + UNSIZED_HEADER_LENGTH as usize; | ||
| self.length_without_deleted = self.length; | ||
| let split = Self::from_inner(new_inner, self.length_capacity); | ||
| self.rebuild(); | ||
|
|
||
| split | ||
| } | ||
|
|
@@ -134,13 +139,10 @@ where | |
| // Node id is stored separately too, so we need to count node_id twice | ||
| self.length -= node_id_len; | ||
| self.length += value_size; | ||
| self.length_without_deleted -= node_id_len; | ||
| self.length_without_deleted += value_size; | ||
| } | ||
| self.length += value_size; | ||
| self.length += UnsizedIndexPageUtility::<T>::slots_value_size(); | ||
| self.length_without_deleted += value_size; | ||
| self.length_without_deleted += UnsizedIndexPageUtility::<T>::slots_value_size(); | ||
|
|
||
| (true, idx) | ||
| } | ||
| (false, idx) => (false, idx), | ||
|
|
@@ -172,16 +174,14 @@ where | |
| where | ||
| T: Borrow<Q>, | ||
| { | ||
| let node_id_len = self.max().map(|v| v.aligned_size()).unwrap_or(0); | ||
| // TODO: Refactor this when empty links logic will be added to the page | ||
| if let Some((val, i)) = NodeLike::delete(&mut self.inner, value) { | ||
| let new_node_id_len = self.max().map(|v| v.aligned_size()).unwrap_or(0); | ||
| if new_node_id_len != node_id_len { | ||
| self.length_without_deleted -= node_id_len; | ||
| self.length_without_deleted += new_node_id_len; | ||
| self.removed_length += | ||
| val.aligned_size() + UnsizedIndexPageUtility::<T>::slots_value_size(); | ||
|
|
||
| if self.removed_length > self.length_capacity / 2 { | ||
| self.rebuild() | ||
| } | ||
| self.length_without_deleted -= val.aligned_size(); | ||
| self.length_without_deleted -= UnsizedIndexPageUtility::<T>::slots_value_size(); | ||
| Some((val, i)) | ||
| } else { | ||
| None | ||
|
|
@@ -246,10 +246,10 @@ mod test { | |
| assert_eq!(node.length, node.length_capacity); | ||
| let split = node.halve(); | ||
| assert_eq!(node.length, 152); | ||
| assert_eq!(node.length_without_deleted, 152); | ||
| assert_eq!(node.removed_length, 0); | ||
| assert_eq!(node.inner.len(), 2); | ||
| assert_eq!(split.length, 136); | ||
| assert_eq!(split.length_without_deleted, 136); | ||
| assert_eq!(split.removed_length, 0); | ||
| assert_eq!(split.inner.len(), 1); | ||
| } | ||
|
|
||
|
|
@@ -258,10 +258,10 @@ mod test { | |
| let mut node = UnsizedNode::<String>::with_capacity(200); | ||
| node.insert(String::from_utf8(vec![b'1'; 16]).unwrap()); | ||
| assert_eq!(node.length, 120); | ||
| assert_eq!(node.length_without_deleted, 120); | ||
| assert_eq!(node.removed_length, 0); | ||
| node.delete(&String::from_utf8(vec![b'1'; 16]).unwrap()); | ||
| assert_eq!(node.length, 120); | ||
| assert_eq!(node.length_without_deleted, 64); | ||
| assert_eq!(node.removed_length, 32); | ||
| } | ||
|
|
||
| #[test] | ||
|
|
@@ -270,10 +270,10 @@ mod test { | |
| node.insert(String::from_utf8(vec![b'1'; 16]).unwrap()); | ||
| node.insert(String::from_utf8(vec![b'2'; 24]).unwrap()); | ||
| assert_eq!(node.length, 168); | ||
| assert_eq!(node.length_without_deleted, 168); | ||
| assert_eq!(node.removed_length, 0); | ||
| node.delete(&String::from_utf8(vec![b'2'; 24]).unwrap()); | ||
| assert_eq!(node.length, 168); | ||
| assert_eq!(node.length_without_deleted, 120); | ||
| assert_eq!(node.removed_length, 40); | ||
| } | ||
|
|
||
| #[test] | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are two operations instead of a single assignment?