From 266a7e1c66ab5b017619ecb64c2eb475dd0991d2 Mon Sep 17 00:00:00 2001 From: pedrocarlo Date: Thu, 31 Jul 2025 12:48:40 -0300 Subject: [PATCH] do not error in `op_transaction` if page 1 was not allocated --- core/error.rs | 4 ++++ core/lib.rs | 7 +++---- core/storage/pager.rs | 10 +++------- core/vdbe/execute.rs | 29 +++++++++++++++++++---------- 4 files changed, 29 insertions(+), 21 deletions(-) diff --git a/core/error.rs b/core/error.rs index a7d4f7960..add128ce7 100644 --- a/core/error.rs +++ b/core/error.rs @@ -65,6 +65,10 @@ pub enum LimboError { Conflict(String), #[error("Database schema changed")] SchemaUpdated, + #[error( + "Database is empty, header does not exist - page 1 should've been allocated before this" + )] + Page1NotAlloc, #[error("Transaction terminated")] TxTerminated, #[error("Write-write conflict")] diff --git a/core/lib.rs b/core/lib.rs index caa9aeb3d..a61944c8d 100644 --- a/core/lib.rs +++ b/core/lib.rs @@ -42,7 +42,6 @@ mod numeric; #[global_allocator] static GLOBAL: mimalloc::MiMalloc = mimalloc::MiMalloc; -use crate::storage::header_accessor::HeaderRef; use crate::translate::optimizer::optimize_plan; use crate::translate::pragma::TURSO_CDC_DEFAULT_TABLE_NAME; #[cfg(all(feature = "fs", feature = "conn_raw_api"))] @@ -311,9 +310,9 @@ impl Database { let pager = conn.pager.borrow().clone(); db.with_schema_mut(|schema| { - let header_ref = pager.io.block(|| HeaderRef::from_pager(&pager))?; - let header = header_ref.borrow(); - let header_schema_cookie = header.schema_cookie.get(); + let header_schema_cookie = pager + .io + .block(|| pager.with_header(|header| header.schema_cookie.get()))?; schema.schema_version = header_schema_cookie; let result = schema .make_from_btree(None, pager.clone(), &syms) diff --git a/core/storage/pager.rs b/core/storage/pager.rs index 759ab2950..ba614c0d4 100644 --- a/core/storage/pager.rs +++ b/core/storage/pager.rs @@ -28,14 +28,12 @@ use super::wal::CheckpointMode; #[cfg(not(feature = "omit_autovacuum"))] use {crate::io::Buffer as IoBuffer, ptrmap::*}; -struct HeaderRef(PageRef); +pub struct HeaderRef(PageRef); impl HeaderRef { pub fn from_pager(pager: &Pager) -> Result> { if !pager.db_state.is_initialized() { - return Err(LimboError::InternalError( - "Database is empty, header does not exist - page 1 should've been allocated before this".to_string() - )); + return Err(LimboError::Page1NotAlloc); } let (page, _c) = pager.read_page(DatabaseHeader::PAGE_ID)?; @@ -65,9 +63,7 @@ pub struct HeaderRefMut(PageRef); impl HeaderRefMut { pub fn from_pager(pager: &Pager) -> Result> { if !pager.db_state.is_initialized() { - return Err(LimboError::InternalError( - "Database is empty, header does not exist - page 1 should've been allocated before this".to_string(), - )); + return Err(LimboError::Page1NotAlloc); } let (page, _c) = pager.read_page(DatabaseHeader::PAGE_ID)?; diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index a96b5c5b2..59c9f0b31 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -4,7 +4,6 @@ use crate::numeric::{NullableInteger, Numeric}; use crate::schema::Table; use crate::storage::btree::{integrity_check, IntegrityCheckError, IntegrityCheckState}; use crate::storage::database::DatabaseFile; -use crate::storage::header_accessor::HeaderRef; use crate::storage::page_cache::DumbLruPageCache; use crate::storage::pager::{AtomicDbState, CreateBTreeFlags, DbState}; use crate::storage::sqlite3_ondisk::read_varint; @@ -2009,9 +2008,9 @@ pub fn op_transaction( if state.mv_tx_id.is_none() { // We allocate the first page lazily in the first transaction. return_if_io!(pager.maybe_allocate_page1()); - let header_ref = pager.io.block(|| HeaderRef::from_pager(&pager))?; - let header = header_ref.borrow(); - let header_schema_cookie = header.schema_cookie.get(); + let header_schema_cookie = pager + .io + .block(|| pager.with_header(|header| header.schema_cookie.get()))?; if header_schema_cookie != *schema_cookie { return Err(LimboError::SchemaUpdated); } @@ -2086,12 +2085,22 @@ pub fn op_transaction( } // Can only read header if page 1 has been allocated already - // begin_write_tx and begin_read_tx guarantee that happens - let header_ref = pager.io.block(|| HeaderRef::from_pager(&pager))?; - let header = header_ref.borrow(); - let header_schema_cookie = header.schema_cookie.get(); - if header_schema_cookie != *schema_cookie { - return Err(LimboError::SchemaUpdated); + // begin_write_tx that happens, but not begin_read_tx + let res = pager + .io + .block(|| pager.with_header(|header| header.schema_cookie.get())); + match res { + Ok(header_schema_cookie) => { + dbg!(header_schema_cookie, *schema_cookie); + if header_schema_cookie != *schema_cookie { + return Err(LimboError::SchemaUpdated); + } + } + // This means we are starting a read_tx and we do not have a page 1 yet, so we just continue execution + Err(LimboError::Page1NotAlloc) => {} + Err(err) => { + return Err(err); + } } if updated {