Skip to content

Commit

Permalink
Fixed the error due occuring due to PageContent::page_type function…
Browse files Browse the repository at this point in the history
… trying to convert overflow pages to PageType variant

I have changed places which use `page_type` function to use `maybe_page_type`
  • Loading branch information
krishvishal committed Jan 5, 2025
1 parent a1bd833 commit 8a54812
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 26 deletions.
37 changes: 25 additions & 12 deletions core/storage/btree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,9 @@ impl BTreeCursor {
continue;
}
None => {
unreachable!("we shall not go back up! The only way is down the slope");
// We've hit either a leaf page or an overflow page
// Either way, we're done traversing
return Ok(CursorResult::Ok(()));
}
}
}
Expand Down Expand Up @@ -699,10 +701,15 @@ impl BTreeCursor {
self.pager.add_dirty(page.get().id);

let page = page.get().contents.as_mut().unwrap();
assert!(matches!(page.page_type(), PageType::TableLeaf));

let page_type = match page.maybe_page_type() {
Some(pt) => pt,
None => return Ok(CursorResult::Ok(())), // Handle overflow pages
};
assert!(matches!(page_type, PageType::TableLeaf));

// find cell
(self.find_cell(page, int_key), page.page_type())
(self.find_cell(page, int_key), page_type)
};

// TODO: if overwrite drop cell
Expand Down Expand Up @@ -1308,8 +1315,11 @@ impl BTreeCursor {
}

if self.is_overflow(contents) {
// Create new child page to hold current root contents
let child_page = self.allocate_page(contents.page_type(), 0);
// Remember original page type - the child will inherit this
let original_type = contents.page_type();

// Create new child page with root's original type
let child_page = self.allocate_page(original_type, 0);
let child_page_id = child_page.get().id;
let child_contents = child_page.get().contents.as_mut().unwrap();

Expand All @@ -1327,10 +1337,10 @@ impl BTreeCursor {
}
let right_pointer = contents.rightmost_pointer();

// Clear root and convert to interior node
// Clear root - IMPORTANT: Set type BEFORE clearing contents
contents.write_u8(PAGE_HEADER_OFFSET_PAGE_TYPE, PageType::TableInterior as u8);
contents.write_u16(PAGE_HEADER_OFFSET_CELL_COUNT, 0);
contents.write_u16(PAGE_HEADER_OFFSET_FIRST_FREEBLOCK, 0);
contents.write_u8(0, PageType::TableInterior as u8);

// Initialize child with root's old contents
for (idx, cell) in cells.iter().enumerate() {
Expand All @@ -1348,7 +1358,7 @@ impl BTreeCursor {

self.pager.add_dirty(child_page_id);

// Update page stack
// Update page stack to include both root and child
self.stack.clear();
self.stack.push(current_root.clone());
self.stack.push(child_page.clone());
Expand Down Expand Up @@ -1386,8 +1396,7 @@ impl BTreeCursor {

// setup overflow page
let contents = page.get().contents.as_mut().unwrap();
let buf = contents.as_ptr();
buf.fill(0);
contents.write_u32(0, 0); // Initialize next overflow page pointer to 0

page
}
Expand Down Expand Up @@ -1746,13 +1755,17 @@ impl BTreeCursor {
fn find_cell(&self, page: &PageContent, int_key: u64) -> usize {
let mut cell_idx = 0;
let cell_count = page.cell_count();
let page_type = match page.maybe_page_type() {
Some(pt) => pt,
None => return 0, // Return 0 for overflow pages
};
while cell_idx < cell_count {
match page
.cell_get(
cell_idx,
self.pager.clone(),
self.payload_overflow_threshold_max(page.page_type()),
self.payload_overflow_threshold_min(page.page_type()),
self.payload_overflow_threshold_max(page_type),
self.payload_overflow_threshold_min(page_type),
self.usable_space(),
)
.unwrap()
Expand Down
39 changes: 25 additions & 14 deletions core/storage/sqlite3_ondisk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ fn write_header_to_buf(buf: &mut [u8], header: &DatabaseHeader) {
}

#[repr(u8)]
#[derive(Debug, PartialEq, Clone)]
#[derive(Debug, PartialEq, Clone, Copy)]
pub enum PageType {
IndexInterior = 2,
TableInterior = 5,
Expand Down Expand Up @@ -421,9 +421,13 @@ impl PageContent {
}

pub fn maybe_page_type(&self) -> Option<PageType> {
match self.read_u8(0).try_into() {
Ok(v) => Some(v),
Err(_) => None, // this could be an overflow page
let page_type = self.read_u8(0);
match page_type {
2 => Some(PageType::IndexInterior),
5 => Some(PageType::TableInterior),
10 => Some(PageType::IndexLeaf),
13 => Some(PageType::TableLeaf),
_ => None, // This handles overflow pages and any other non-btree page types
}
}

Expand Down Expand Up @@ -534,11 +538,14 @@ impl PageContent {
}

pub fn rightmost_pointer(&self) -> Option<u32> {
match self.page_type() {
PageType::IndexInterior => Some(self.read_u32(8)),
PageType::TableInterior => Some(self.read_u32(8)),
PageType::IndexLeaf => None,
PageType::TableLeaf => None,
match self.maybe_page_type() {
Some(page_type) => match page_type {
PageType::IndexInterior => Some(self.read_u32(8)),
PageType::TableInterior => Some(self.read_u32(8)),
PageType::IndexLeaf => None,
PageType::TableLeaf => None,
},
None => None, // Handle overflow pages by returning None
}
}

Expand Down Expand Up @@ -650,11 +657,15 @@ impl PageContent {
}

pub fn is_leaf(&self) -> bool {
match self.page_type() {
PageType::IndexInterior => false,
PageType::TableInterior => false,
PageType::IndexLeaf => true,
PageType::TableLeaf => true,
// First check if this is a btree page by looking at the type
match self.maybe_page_type() {
Some(page_type) => match page_type {
PageType::IndexInterior => false,
PageType::TableInterior => false,
PageType::IndexLeaf => true,
PageType::TableLeaf => true,
},
None => false, // Overflow pages are not leaf pages
}
}

Expand Down

0 comments on commit 8a54812

Please sign in to comment.