core/btree: improve documentation #539
Conversation
core/storage/btree.rs
Outdated
| return; | ||
| } | ||
|
|
||
| // if we clear space that is at the start of the cell content area, | ||
| // we need to update the cell content area pointer forward to account for the removed space | ||
| // FIXME: is offset ever < cell_content_area? |
There was a problem hiding this comment.
Mental notes while developing. I don't remember what idea I had in mind xd
There was a problem hiding this comment.
sorry i shouldve been clearer. this FIXME was my addition -- should we consider it an invariant violation if a cell offset is < cell_content_area? if i understand correctly the cell content area grows leftwards from the end of the page and it should always point to the leftmost cell start.
There was a problem hiding this comment.
rewrote the FIXME to make it clearer what I mean. I don't want to change runtime code in this PR.
core/storage/btree.rs
Outdated
| // extend boundary of content area | ||
| page.write_u16(BTREE_HEADER_OFFSET_FREEBLOCK, page.first_freeblock()); | ||
| page.write_u16(BTREE_HEADER_OFFSET_CELL_CONTENT, offset + len); | ||
| page.write_u16(PAGE_HEADER_OFFSET_FREEBLOCK, page.first_freeblock()); // why is this here? |
There was a problem hiding this comment.
Added a better FIXME for this. I don't want to change runtime code in this PR
| let min_local = self.min_local(page_type); | ||
| let mut space_left = min_local + (record_buf.len() - min_local) % (self.usable_space() - 4); | ||
| let payload_overflow_threshold_min = self.payload_overflow_threshold_min(page_type); | ||
| let mut space_left = payload_overflow_threshold_min |
There was a problem hiding this comment.
I don't understand this calculation and it should be commented
There was a problem hiding this comment.
I don't remember right now. Might need to look at it again. There are things that are basically 1:1 copies of what sqlite does that's why names are like that
There was a problem hiding this comment.
I can try to find the places in sqlite code that these are direct c-to-rust translations of and attach them as comments
There was a problem hiding this comment.
also we can keep calling these max_local and min_local if it helps in referencing back to sqlite later. i just found those names confusing and tried to come up w something else, although it's difficult to translate these into concise variable names:
max_local = maximum payload size before the content must spill to overflow pages
min_local = minimum payload size when the content is ALLOWED to spill to overflow pages
There was a problem hiding this comment.
Added a reference to sqlite source in c417fe7
| let mut space_left = min_local + (payload_size - min_local) % (usable_size - 4); | ||
| if space_left > max_local { | ||
| space_left = min_local; | ||
| let mut space_left = payload_overflow_threshold_min |
There was a problem hiding this comment.
I don't understand this calculation and it should be commented
There was a problem hiding this comment.
Added a reference to sqlite source in c417fe7
This PR should have no functional changes, just variable renaming and comments
Using
///comment format for better IDE support