From 51eb2af06ab316e9709c6372e309847fc097600f Mon Sep 17 00:00:00 2001 From: Diego Reis Date: Sat, 12 Apr 2025 22:24:22 -0300 Subject: [PATCH] core(refactor): Add CreateBTreeFlags Passing 1s and 0s with comments is not rustacean, and since we already follow the pattern of struct flags in other sections of the codebase it's better use it here too. --- core/storage/pager.rs | 38 +++++++++++++++++++++++++++++++------- core/translate/index.rs | 3 ++- core/translate/schema.rs | 5 +++-- core/vdbe/execute.rs | 2 +- core/vdbe/explain.rs | 4 ++-- core/vdbe/insn.rs | 8 ++++++-- 6 files changed, 45 insertions(+), 15 deletions(-) diff --git a/core/storage/pager.rs b/core/storage/pager.rs index 9d6d90c00..5d9554198 100644 --- a/core/storage/pager.rs +++ b/core/storage/pager.rs @@ -206,14 +206,11 @@ impl Pager { }) } - pub fn btree_create(&self, flags: usize) -> u32 { + pub fn btree_create(&self, flags: &CreateBTreeFlags) -> u32 { let page_type = match flags { - 1 => PageType::TableLeaf, - 2 => PageType::IndexLeaf, - _ => unreachable!( - "wrong create table flags, should be 1 for table and 2 for index, got {}", - flags, - ), + _ if flags.is_table() => PageType::TableLeaf, + _ if flags.is_index() => PageType::IndexLeaf, + _ => unreachable!("Invalid flags state"), }; let page = self.do_allocate_page(page_type, 0); let id = page.get().id; @@ -642,6 +639,33 @@ pub fn allocate_page(page_id: usize, buffer_pool: &Rc, offset: usize page } +#[derive(Debug)] +pub struct CreateBTreeFlags(pub u8); +impl CreateBTreeFlags { + pub const TABLE: u8 = 0b0001; + pub const INDEX: u8 = 0b0010; + + pub fn new_table() -> Self { + Self(CreateBTreeFlags::TABLE) + } + + pub fn new_index() -> Self { + Self(CreateBTreeFlags::INDEX) + } + + pub fn is_table(&self) -> bool { + (self.0 & CreateBTreeFlags::TABLE) != 0 + } + + pub fn is_index(&self) -> bool { + (self.0 & CreateBTreeFlags::INDEX) != 0 + } + + pub fn get_flags(&self) -> u8 { + self.0 + } +} + #[cfg(test)] mod tests { use std::sync::Arc; diff --git a/core/translate/index.rs b/core/translate/index.rs index 32d7cd2e9..20647d15c 100644 --- a/core/translate/index.rs +++ b/core/translate/index.rs @@ -2,6 +2,7 @@ use std::sync::Arc; use crate::{ schema::{BTreeTable, Column, Index, IndexColumn, PseudoTable, Schema}, + storage::pager::CreateBTreeFlags, types::Record, util::normalize_ident, vdbe::{ @@ -91,7 +92,7 @@ pub fn translate_create_index( program.emit_insn(Insn::CreateBtree { db: 0, root: root_page_reg, - flags: 2, // index leaf + flags: CreateBTreeFlags::new_index(), }); // open the sqlite schema table for writing and create a new entry for the index diff --git a/core/translate/schema.rs b/core/translate/schema.rs index 3d5aa79db..449d1e0e8 100644 --- a/core/translate/schema.rs +++ b/core/translate/schema.rs @@ -2,6 +2,7 @@ use std::fmt::Display; use crate::ast; use crate::schema::Schema; +use crate::storage::pager::CreateBTreeFlags; use crate::translate::ProgramBuilder; use crate::translate::ProgramBuilderOpts; use crate::translate::QueryMode; @@ -60,7 +61,7 @@ pub fn translate_create_table( program.emit_insn(Insn::CreateBtree { db: 0, root: table_root_reg, - flags: 1, // Table leaf page + flags: CreateBTreeFlags::new_table(), }); // Create an automatic index B-tree if needed @@ -92,7 +93,7 @@ pub fn translate_create_table( program.emit_insn(Insn::CreateBtree { db: 0, root: index_root_reg, - flags: 2, // Index leaf page + flags: CreateBTreeFlags::new_index(), }); } diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index 4d2a96d10..37c66b111 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -4171,7 +4171,7 @@ pub fn op_create_btree( // TODO: implement temp databases todo!("temp databases not implemented yet"); } - let root_page = pager.btree_create(*flags); + let root_page = pager.btree_create(flags); state.registers[*root] = Register::OwnedValue(OwnedValue::Integer(root_page as i64)); state.pc += 1; Ok(InsnFunctionStepResult::Step) diff --git a/core/vdbe/explain.rs b/core/vdbe/explain.rs index 3ce60f5db..79b0f3ded 100644 --- a/core/vdbe/explain.rs +++ b/core/vdbe/explain.rs @@ -1177,10 +1177,10 @@ pub fn insn_to_str( "CreateBtree", *db as i32, *root as i32, - *flags as i32, + flags.get_flags() as i32, OwnedValue::build_text(""), 0, - format!("r[{}]=root iDb={} flags={}", root, db, flags), + format!("r[{}]=root iDb={} flags={}", root, db, flags.get_flags()), ), Insn::Destroy { root, diff --git a/core/vdbe/insn.rs b/core/vdbe/insn.rs index 0047f9d11..acd8166c3 100644 --- a/core/vdbe/insn.rs +++ b/core/vdbe/insn.rs @@ -1,7 +1,11 @@ use std::{num::NonZero, rc::Rc}; use super::{execute, AggFunc, BranchOffset, CursorID, FuncCtx, InsnFunction, PageIdx}; -use crate::{schema::BTreeTable, storage::wal::CheckpointMode, types::Record}; +use crate::{ + schema::BTreeTable, + storage::{pager::CreateBTreeFlags, wal::CheckpointMode}, + types::Record, +}; use limbo_macros::Description; /// Flags provided to comparison instructions (e.g. Eq, Ne) which determine behavior related to NULL values. @@ -703,7 +707,7 @@ pub enum Insn { /// The root page of the new b-tree (P2). root: usize, /// Flags (P3). - flags: usize, + flags: CreateBTreeFlags, }, /// Deletes an entire database table or index whose root page in the database file is given by P1.