Repository: tursodatabase/limbo (rust)

Issue:
Support SQLITE Readonly mode
Seems simple enough. From what I'm seeing, we only need to set a check on the transaction opcode and not lock the file.

Reference: https://github.com/sqlite/sqlite/blob/c26cde3bfdfb7df5d8230d7426e8505f7edd166e/src/vdbe.c#L4051

I will get started on this.

Pull request description:
Read only mode
Closes #1413 . Basically, SQLite emits a check in a transaction to see if it is attempting to write. If the db is in read only mode, it throws an error, else the statement is executed. Mirroring how Rusqlite does it, I modified the `OpenFlags` to use bitflags to better configure how we open our VFS. This modification, will enable us to run tests against the same database in parallel.

Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 1a4c6069..3fb8760a 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1726,6 +1726,7 @@ dependencies = [
 name = "limbo_core"
 version = "0.0.19"
 dependencies = [
+ "bitflags 2.9.0",
  "built",
  "cfg_block",
  "chrono",
diff --git a/core/Cargo.toml b/core/Cargo.toml
index f23aeeeb..9fd71b05 100644
--- a/core/Cargo.toml
+++ b/core/Cargo.toml
@@ -73,6 +73,7 @@ parking_lot = "0.12.3"
 crossbeam-skiplist = "0.1.3"
 tracing = "0.1.41"
 ryu = "1.0.19"
+bitflags = "2.9.0"
 
 [build-dependencies]
 chrono = { version = "0.4.38", default-features = false }
diff --git a/core/error.rs b/core/error.rs
index e8eb83a5..1eca5030 100644
--- a/core/error.rs
+++ b/core/error.rs
@@ -51,6 +51,8 @@ pub enum LimboError {
     IntegerOverflow,
     #[error("Schema is locked for write")]
     SchemaLocked,
+    #[error("Database Connection is read-only")]
+    ReadOnly,
 }
 
 #[macro_export]
diff --git a/core/io/generic.rs b/core/io/generic.rs
index fd59ece8..aab5f268 100644
--- a/core/io/generic.rs
+++ b/core/io/generic.rs
@@ -20,11 +20,15 @@ unsafe impl Sync for GenericIO {}
 impl IO for GenericIO {
     fn open_file(&self, path: &str, flags: OpenFlags, _direct: bool) -> Result<Arc<dyn File>> {
         trace!("open_file(path = {})", path);
-        let file = std::fs::OpenOptions::new()
-            .read(true)
-            .write(true)
-            .create(matches!(flags, OpenFlags::Create))
-            .open(path)?;
+        let mut file = std::fs::File::options();
+        file.read(true);
+
+        if !flags.contains(OpenFlags::ReadOnly) {
+            file.write(true);
+            file.create(flags.contains(OpenFlags::Create));
+        }
+
+        let file = file.open(path)?;
         Ok(Arc::new(GenericFile {
             file: RefCell::new(file),
             memory_io: Arc::new(MemoryIO::new()),
diff --git a/core/io/io_uring.rs b/core/io/io_uring.rs
index b4b21aca..25d6aa33 100644
--- a/core/io/io_uring.rs
+++ b/core/io/io_uring.rs
@@ -139,11 +139,15 @@ impl WrappedIOUring {
 impl IO for UringIO {
     fn open_file(&self, path: &str, flags: OpenFlags, direct: bool) -> Result<Arc<dyn File>> {
         trace!("open_file(path = {})", path);
-        let file = std::fs::File::options()
-            .read(true)
-            .write(true)
-            .create(matches!(flags, OpenFlags::Create))
-            .open(path)?;
+        let mut file = std::fs::File::options();
+        file.read(true);
+
+        if !flags.contains(OpenFlags::ReadOnly) {
+            file.write(true);
+            file.create(flags.contains(OpenFlags::Create));
+        }
+
+        let file = file.open(path)?;
         // Let's attempt to enable direct I/O. Not all filesystems support it
         // so ignore any errors.
         let fd = file.as_fd();
@@ -158,7 +162,7 @@ impl IO for UringIO {
             file,
         });
         if std::env::var(common::ENV_DISABLE_FILE_LOCK).is_err() {
-            uring_file.lock_file(true)?;
+            uring_file.lock_file(!flags.contains(OpenFlags::ReadOnly))?;
         }
         Ok(uring_file)
     }
diff --git a/core/io/mod.rs b/core/io/mod.rs
index 6f161d11..6f75e9be 100644
--- a/core/io/mod.rs
+++ b/core/io/mod.rs
@@ -1,4 +1,5 @@
 use crate::Result;
+use bitflags::bitflags;
 use cfg_block::cfg_block;
 use std::fmt;
 use std::sync::Arc;
@@ -19,18 +20,20 @@ pub trait File: Send + Sync {
     fn size(&self) -> Result<u64>;
 }
 
-#[derive(Copy, Clone)]
-pub enum OpenFlags {
-    None,
-    Create,
+#[derive(Debug, Copy, Clone, PartialEq)]
+pub struct OpenFlags(i32);
+
+bitflags! {
+    impl OpenFlags: i32 {
+        const None = 0b00000000;
+        const Create = 0b0000001;
+        const ReadOnly = 0b0000010;
+    }
 }
 
-impl OpenFlags {
-    pub fn to_flags(&self) -> i32 {
-        match self {
-            Self::None => 0,
-            Self::Create => 1,
-        }
+impl Default for OpenFlags {
+    fn default() -> Self {
+        Self::Create
     }
 }
 
diff --git a/core/io/unix.rs b/core/io/unix.rs
index c232ed3a..721ba20f 100644
--- a/core/io/unix.rs
+++ b/core/io/unix.rs
@@ -3,6 +3,7 @@ use crate::io::common;
 use crate::Result;
 
 use super::{Completion, File, MemoryIO, OpenFlags, IO};
+use crate::io::clock::{Clock, Instant};
 use polling::{Event, Events, Poller};
 use rustix::{
     fd::{AsFd, AsRawFd},
@@ -18,7 +19,6 @@ use std::{
     sync::Arc,
 };
 use tracing::{debug, trace};
-use crate::io::clock::{Clock, Instant};
 
 struct OwnedCallbacks(UnsafeCell<Callbacks>);
 // We assume we locking on IO level is done by user.
@@ -197,12 +197,15 @@ impl Clock for UnixIO {
 impl IO for UnixIO {
     fn open_file(&self, path: &str, flags: OpenFlags, _direct: bool) -> Result<Arc<dyn File>> {
         trace!("open_file(path = {})", path);
-        let file = std::fs::File::options()
-            .read(true)
-            .custom_flags(OFlags::NONBLOCK.bits() as i32)
-            .write(true)
-            .create(matches!(flags, OpenFlags::Create))
-            .open(path)?;
+        let mut file = std::fs::File::options();
+        file.read(true).custom_flags(OFlags::NONBLOCK.bits() as i32);
+
+        if !flags.contains(OpenFlags::ReadOnly) {
+            file.write(true);
+            file.create(flags.contains(OpenFlags::Create));
+        }
+
+        let file = file.open(path)?;
 
         #[allow(clippy::arc_with_non_send_sync)]
         let unix_file = Arc::new(UnixFile {
@@ -211,7 +214,7 @@ impl IO for UnixIO {
             callbacks: BorrowedCallbacks(self.callbacks.as_mut().into()),
         });
         if std::env::var(common::ENV_DISABLE_FILE_LOCK).is_err() {
-            unix_file.lock_file(true)?;
+            unix_file.lock_file(!flags.contains(OpenFlags::ReadOnly))?;
         }
         Ok(unix_file)
     }
@@ -258,7 +261,7 @@ impl IO for UnixIO {
         getrandom::getrandom(&mut buf).unwrap();
         i64::from_ne_bytes(buf)
     }
-    
+
     fn get_memory_io(&self) -> Arc<MemoryIO> {
         Arc::new(MemoryIO::new())
     }
diff --git a/core/io/vfs.rs b/core/io/vfs.rs
index 95b4055d..d02f7d34 100644
--- a/core/io/vfs.rs
+++ b/core/io/vfs.rs
@@ -24,7 +24,7 @@ impl IO for VfsMod {
         })?;
         let ctx = self.ctx as *mut c_void;
         let vfs = unsafe { &*self.ctx };
-        let file = unsafe { (vfs.open)(ctx, c_path.as_ptr(), flags.to_flags(), direct) };
+        let file = unsafe { (vfs.open)(ctx, c_path.as_ptr(), flags.0, direct) };
         if file.is_null() {
             return Err(LimboError::ExtensionError("File not found".to_string()));
         }
diff --git a/core/io/windows.rs b/core/io/windows.rs
index 6c46d197..a329abc1 100644
--- a/core/io/windows.rs
+++ b/core/io/windows.rs
@@ -19,11 +19,15 @@ unsafe impl Sync for WindowsIO {}
 impl IO for WindowsIO {
     fn open_file(&self, path: &str, flags: OpenFlags, direct: bool) -> Result<Arc<dyn File>> {
         trace!("open_file(path = {})", path);
-        let file = std::fs::File::options()
-            .read(true)
-            .write(true)
-            .create(matches!(flags, OpenFlags::Create))
-            .open(path)?;
+        let mut file = std::fs::File::options();
+        file.read(true);
+
+        if !flags.contains(OpenFlags::ReadOnly) {
+            file.write(true);
+            file.create(flags.contains(OpenFlags::Create));
+        }
+
+        let file = file.open(path)?;
         Ok(Arc::new(WindowsFile {
             file: RefCell::new(file),
         }))
diff --git a/core/lib.rs b/core/lib.rs
index ddf741ff..2f7ab057 100644
--- a/core/lib.rs
+++ b/core/lib.rs
@@ -101,6 +101,7 @@ pub struct Database {
     // create DB connections.
     shared_page_cache: Arc<RwLock<DumbLruPageCache>>,
     shared_wal: Arc<UnsafeCell<WalFileShared>>,
+    open_flags: OpenFlags,
 }
 
 unsafe impl Send for Database {}
@@ -109,10 +110,20 @@ unsafe impl Sync for Database {}
 impl Database {
     #[cfg(feature = "fs")]
     pub fn open_file(io: Arc<dyn IO>, path: &str, enable_mvcc: bool) -> Result<Arc<Database>> {
-        let file = io.open_file(path, OpenFlags::Create, true)?;
+        Self::open_file_with_flags(io, path, OpenFlags::default(), enable_mvcc)
+    }
+
+    #[cfg(feature = "fs")]
+    pub fn open_file_with_flags(
+        io: Arc<dyn IO>,
+        path: &str,
+        flags: OpenFlags,
+        enable_mvcc: bool,
+    ) -> Result<Arc<Database>> {
+        let file = io.open_file(path, flags, true)?;
         maybe_init_database_file(&file, &io)?;
         let db_file = Arc::new(DatabaseFile::new(file));
-        Self::open(io, path, db_file, enable_mvcc)
+        Self::open_with_flags(io, path, db_file, flags, enable_mvcc)
     }
 
     #[allow(clippy::arc_with_non_send_sync)]
@@ -121,6 +132,17 @@ impl Database {
         path: &str,
         db_file: Arc<dyn DatabaseStorage>,
         enable_mvcc: bool,
+    ) -> Result<Arc<Database>> {
+        Self::open_with_flags(io, path, db_file, OpenFlags::default(), enable_mvcc)
+    }
+
+    #[allow(clippy::arc_with_non_send_sync)]
+    pub fn open_with_flags(
+        io: Arc<dyn IO>,
+        path: &str,
+        db_file: Arc<dyn DatabaseStorage>,
+        flags: OpenFlags,
+        enable_mvcc: bool,
     ) -> Result<Arc<Database>> {
         let db_header = Pager::begin_open(db_file.clone())?;
         // ensure db header is there
@@ -155,6 +177,7 @@ impl Database {
             db_file,
             io: io.clone(),
             page_size,
+            open_flags: flags,
         };
         let db = Arc::new(db);
         {
diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs
index 948f0a5b..30f9e678 100644
--- a/core/vdbe/execute.rs
+++ b/core/vdbe/execute.rs
@@ -1564,20 +1564,17 @@ pub fn op_transaction(
     let Insn::Transaction { write } = insn else {
         unreachable!("unexpected Insn {:?}", insn)
     };
+    let connection = program.connection.upgrade().unwrap();
+    if *write && connection._db.open_flags.contains(OpenFlags::ReadOnly) {
+        return Err(LimboError::ReadOnly);
+    }
     if let Some(mv_store) = &mv_store {
         if state.mv_tx_id.is_none() {
             let tx_id = mv_store.begin_tx();
-            program
-                .connection
-                .upgrade()
-                .unwrap()
-                .mv_transactions
-                .borrow_mut()
-                .push(tx_id);
+            connection.mv_transactions.borrow_mut().push(tx_id);
             state.mv_tx_id = Some(tx_id);
         }
     } else {
-        let connection = program.connection.upgrade().unwrap();
         let current_state = connection.transaction_state.get();
         let (new_transaction_state, updated) = match (current_state, write) {
             (TransactionState::Write, true) => (TransactionState::Write, false),


Test patch:
diff --git a/tests/integration/common.rs b/tests/integration/common.rs
index 2c668a12..1ef890db 100644
--- a/tests/integration/common.rs
+++ b/tests/integration/common.rs
@@ -1,6 +1,7 @@
 use limbo_core::{CheckpointStatus, Connection, Database, IO};
 use rand::{rng, RngCore};
-use std::path::PathBuf;
+use rusqlite::params;
+use std::path::{Path, PathBuf};
 use std::rc::Rc;
 use std::sync::Arc;
 use tempfile::TempDir;
@@ -28,6 +29,14 @@ impl TempDatabase {
         Self { path, io }
     }
 
+    pub fn new_with_existent(db_path: &Path) -> Self {
+        let io: Arc<dyn IO + Send> = Arc::new(limbo_core::PlatformIO::new().unwrap());
+        Self {
+            path: db_path.to_path_buf(),
+            io,
+        }
+    }
+
     pub fn new_with_rusqlite(table_sql: &str) -> Self {
         let mut path = TempDir::new().unwrap().into_path();
         path.push("test.db");
@@ -44,8 +53,21 @@ impl TempDatabase {
     }
 
     pub fn connect_limbo(&self) -> Rc<limbo_core::Connection> {
+        Self::connect_limbo_with_flags(&self, limbo_core::OpenFlags::default())
+    }
+
+    pub fn connect_limbo_with_flags(
+        &self,
+        flags: limbo_core::OpenFlags,
+    ) -> Rc<limbo_core::Connection> {
         log::debug!("conneting to limbo");
-        let db = Database::open_file(self.io.clone(), self.path.to_str().unwrap(), false).unwrap();
+        let db = Database::open_file_with_flags(
+            self.io.clone(),
+            self.path.to_str().unwrap(),
+            flags,
+            false,
+        )
+        .unwrap();
 
         let conn = db.connect().unwrap();
         log::debug!("connected to limbo");
@@ -104,9 +126,97 @@ pub fn maybe_setup_tracing() {
         .with(EnvFilter::from_default_env())
         .try_init();
 }
+
+pub(crate) fn sqlite_exec_rows(
+    conn: &rusqlite::Connection,
+    query: &str,
+) -> Vec<Vec<rusqlite::types::Value>> {
+    let mut stmt = conn.prepare(&query).unwrap();
+    let mut rows = stmt.query(params![]).unwrap();
+    let mut results = Vec::new();
+    while let Some(row) = rows.next().unwrap() {
+        let mut result = Vec::new();
+        for i in 0.. {
+            let column: rusqlite::types::Value = match row.get(i) {
+                Ok(column) => column,
+                Err(rusqlite::Error::InvalidColumnIndex(_)) => break,
+                Err(err) => panic!("unexpected rusqlite error: {}", err),
+            };
+            result.push(column);
+        }
+        results.push(result)
+    }
+
+    results
+}
+
+pub(crate) fn limbo_exec_rows(
+    db: &TempDatabase,
+    conn: &Rc<limbo_core::Connection>,
+    query: &str,
+) -> Vec<Vec<rusqlite::types::Value>> {
+    let mut stmt = conn.prepare(query).unwrap();
+    let mut rows = Vec::new();
+    'outer: loop {
+        let row = loop {
+            let result = stmt.step().unwrap();
+            match result {
+                limbo_core::StepResult::Row => {
+                    let row = stmt.row().unwrap();
+                    break row;
+                }
+                limbo_core::StepResult::IO => {
+                    db.io.run_once().unwrap();
+                    continue;
+                }
+                limbo_core::StepResult::Done => break 'outer,
+                r => panic!("unexpected result {:?}: expecting single row", r),
+            }
+        };
+        let row = row
+            .get_values()
+            .map(|x| match x {
+                limbo_core::OwnedValue::Null => rusqlite::types::Value::Null,
+                limbo_core::OwnedValue::Integer(x) => rusqlite::types::Value::Integer(*x),
+                limbo_core::OwnedValue::Float(x) => rusqlite::types::Value::Real(*x),
+                limbo_core::OwnedValue::Text(x) => {
+                    rusqlite::types::Value::Text(x.as_str().to_string())
+                }
+                limbo_core::OwnedValue::Blob(x) => rusqlite::types::Value::Blob(x.to_vec()),
+            })
+            .collect();
+        rows.push(row);
+    }
+    rows
+}
+
+pub(crate) fn limbo_exec_rows_error(
+    db: &TempDatabase,
+    conn: &Rc<limbo_core::Connection>,
+    query: &str,
+) -> limbo_core::Result<()> {
+    let mut stmt = conn.prepare(query)?;
+    loop {
+        let result = stmt.step()?;
+        match result {
+            limbo_core::StepResult::IO => {
+                db.io.run_once()?;
+                continue;
+            }
+            limbo_core::StepResult::Done => return Ok(()),
+            r => panic!("unexpected result {:?}: expecting single row", r),
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
-    use super::TempDatabase;
+    use std::vec;
+
+    use tempfile::TempDir;
+
+    use super::{limbo_exec_rows, limbo_exec_rows_error, TempDatabase};
+    use rusqlite::types::Value;
 
     #[test]
     fn test_statement_columns() -> anyhow::Result<()> {
@@ -145,4 +255,29 @@ mod tests {
 
         Ok(())
     }
+
+    #[test]
+    fn test_limbo_open_read_only() -> anyhow::Result<()> {
+        let path = TempDir::new().unwrap().into_path().join("temp_read_only");
+        let db = TempDatabase::new_with_existent(&path);
+        {
+            let conn = db.connect_limbo();
+            let ret = limbo_exec_rows(&db, &conn, "CREATE table t(a)");
+            assert!(ret.is_empty(), "{:?}", ret);
+            limbo_exec_rows(&db, &conn, "INSERT INTO t values (1)");
+            conn.close().unwrap()
+        }
+
+        {
+            let conn = db.connect_limbo_with_flags(
+                limbo_core::OpenFlags::default() | limbo_core::OpenFlags::ReadOnly,
+            );
+            let ret = limbo_exec_rows(&db, &conn, "SELECT * from t");
+            assert_eq!(ret, vec![vec![Value::Integer(1)]]);
+
+            let err = limbo_exec_rows_error(&db, &conn, "INSERT INTO t values (1)").unwrap_err();
+            assert!(matches!(err, limbo_core::LimboError::ReadOnly), "{:?}", err);
+        }
+        Ok(())
+    }
 }
diff --git a/tests/integration/fuzz/mod.rs b/tests/integration/fuzz/mod.rs
index fb38455a..82d1c11a 100644
--- a/tests/integration/fuzz/mod.rs
+++ b/tests/integration/fuzz/mod.rs
@@ -2,14 +2,14 @@ pub mod grammar_generator;
 
 #[cfg(test)]
 mod tests {
-    use std::{collections::HashSet, rc::Rc};
+    use std::collections::HashSet;
 
     use rand::{seq::IndexedRandom, Rng, SeedableRng};
     use rand_chacha::ChaCha8Rng;
     use rusqlite::params;
 
     use crate::{
-        common::TempDatabase,
+        common::{limbo_exec_rows, sqlite_exec_rows, TempDatabase},
         fuzz::grammar_generator::{const_str, rand_int, rand_str, GrammarGenerator},
     };
 
@@ -24,69 +24,6 @@ mod tests {
         (rng, seed)
     }
 
-    fn sqlite_exec_rows(
-        conn: &rusqlite::Connection,
-        query: &str,
-    ) -> Vec<Vec<rusqlite::types::Value>> {
-        let mut stmt = conn.prepare(&query).unwrap();
-        let mut rows = stmt.query(params![]).unwrap();
-        let mut results = Vec::new();
-        while let Some(row) = rows.next().unwrap() {
-            let mut result = Vec::new();
-            for i in 0.. {
-                let column: rusqlite::types::Value = match row.get(i) {
-                    Ok(column) => column,
-                    Err(rusqlite::Error::InvalidColumnIndex(_)) => break,
-                    Err(err) => panic!("unexpected rusqlite error: {}", err),
-                };
-                result.push(column);
-            }
-            results.push(result)
-        }
-
-        results
-    }
-
-    fn limbo_exec_rows(
-        db: &TempDatabase,
-        conn: &Rc<limbo_core::Connection>,
-        query: &str,
-    ) -> Vec<Vec<rusqlite::types::Value>> {
-        let mut stmt = conn.prepare(query).unwrap();
-        let mut rows = Vec::new();
-        'outer: loop {
-            let row = loop {
-                let result = stmt.step().unwrap();
-                match result {
-                    limbo_core::StepResult::Row => {
-                        let row = stmt.row().unwrap();
-                        break row;
-                    }
-                    limbo_core::StepResult::IO => {
-                        db.io.run_once().unwrap();
-                        continue;
-                    }
-                    limbo_core::StepResult::Done => break 'outer,
-                    r => panic!("unexpected result {:?}: expecting single row", r),
-                }
-            };
-            let row = row
-                .get_values()
-                .map(|x| match x {
-                    limbo_core::OwnedValue::Null => rusqlite::types::Value::Null,
-                    limbo_core::OwnedValue::Integer(x) => rusqlite::types::Value::Integer(*x),
-                    limbo_core::OwnedValue::Float(x) => rusqlite::types::Value::Real(*x),
-                    limbo_core::OwnedValue::Text(x) => {
-                        rusqlite::types::Value::Text(x.as_str().to_string())
-                    }
-                    limbo_core::OwnedValue::Blob(x) => rusqlite::types::Value::Blob(x.to_vec()),
-                })
-                .collect();
-            rows.push(row);
-        }
-        rows
-    }
-
     #[test]
     pub fn arithmetic_expression_fuzz_ex1() {
         let db = TempDatabase::new_empty();