From 016d9d17abcb844a3c86d1400751f37cc392522d Mon Sep 17 00:00:00 2001 From: Jorge Hermo Date: Mon, 13 Jan 2025 00:05:14 +0100 Subject: [PATCH 1/5] refactor: json functions vdbe --- core/vdbe/mod.rs | 143 +++++++++++++++++++++++------------------------ 1 file changed, 69 insertions(+), 74 deletions(-) diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs index 64c41b3bd..208c002a6 100644 --- a/core/vdbe/mod.rs +++ b/core/vdbe/mod.rs @@ -1343,84 +1343,79 @@ impl Program { let arg_count = func.arg_count; match &func.func { #[cfg(feature = "json")] - crate::function::Func::Json(JsonFunc::Json) => { - let json_value = &state.registers[*start_reg]; - let json_str = get_json(json_value); - match json_str { - Ok(json) => state.registers[*dest] = json, - Err(e) => return Err(e), - } - } - #[cfg(feature = "json")] - crate::function::Func::Json(JsonFunc::JsonArray) => { - let reg_values = &state.registers[*start_reg..*start_reg + arg_count]; - - let json_array = json_array(reg_values); - - match json_array { - Ok(json) => state.registers[*dest] = json, - Err(e) => return Err(e), - } - } - #[cfg(feature = "json")] - crate::function::Func::Json(JsonFunc::JsonExtract) => { - let result = match arg_count { - 0 => json_extract(&OwnedValue::Null, &[]), - _ => { - let val = &state.registers[*start_reg]; - let reg_values = - &state.registers[*start_reg + 1..*start_reg + arg_count]; - - json_extract(val, reg_values) + crate::function::Func::Json(json_func) => match json_func { + JsonFunc::Json => { + let json_value = &state.registers[*start_reg]; + let json_str = get_json(json_value); + match json_str { + Ok(json) => state.registers[*dest] = json, + Err(e) => return Err(e), } - }; + } + JsonFunc::JsonArray => { + let reg_values = + &state.registers[*start_reg..*start_reg + arg_count]; - match result { - Ok(json) => state.registers[*dest] = json, - Err(e) => return Err(e), - } - } - #[cfg(feature = "json")] - crate::function::Func::Json( - func @ (JsonFunc::JsonArrowExtract | JsonFunc::JsonArrowShiftExtract), - ) => { - assert_eq!(arg_count, 2); - let json = &state.registers[*start_reg]; - let path = &state.registers[*start_reg + 1]; - let func = match func { - JsonFunc::JsonArrowExtract => json_arrow_extract, - JsonFunc::JsonArrowShiftExtract => json_arrow_shift_extract, - _ => unreachable!(), - }; - let json_str = func(json, path); - match json_str { - Ok(json) => state.registers[*dest] = json, - Err(e) => return Err(e), - } - } - #[cfg(feature = "json")] - crate::function::Func::Json( - func @ (JsonFunc::JsonArrayLength | JsonFunc::JsonType), - ) => { - let json_value = &state.registers[*start_reg]; - let path_value = if arg_count > 1 { - Some(&state.registers[*start_reg + 1]) - } else { - None - }; - let func_result = match func { - JsonFunc::JsonArrayLength => { - json_array_length(json_value, path_value) + let json_array = json_array(reg_values); + + match json_array { + Ok(json) => state.registers[*dest] = json, + Err(e) => return Err(e), } - JsonFunc::JsonType => json_type(json_value, path_value), - _ => unreachable!(), - }; - - match func_result { - Ok(result) => state.registers[*dest] = result, - Err(e) => return Err(e), } - } + JsonFunc::JsonExtract => { + let result = match arg_count { + 0 => json_extract(&OwnedValue::Null, &[]), + _ => { + let val = &state.registers[*start_reg]; + let reg_values = &state.registers + [*start_reg + 1..*start_reg + arg_count]; + + json_extract(val, reg_values) + } + }; + + match result { + Ok(json) => state.registers[*dest] = json, + Err(e) => return Err(e), + } + } + JsonFunc::JsonArrowExtract | JsonFunc::JsonArrowShiftExtract => { + assert_eq!(arg_count, 2); + let json = &state.registers[*start_reg]; + let path = &state.registers[*start_reg + 1]; + let json_func = match json_func { + JsonFunc::JsonArrowExtract => json_arrow_extract, + JsonFunc::JsonArrowShiftExtract => json_arrow_shift_extract, + _ => unreachable!(), + }; + let json_str = json_func(json, path); + match json_str { + Ok(json) => state.registers[*dest] = json, + Err(e) => return Err(e), + } + } + JsonFunc::JsonArrayLength | JsonFunc::JsonType => { + let json_value = &state.registers[*start_reg]; + let path_value = if arg_count > 1 { + Some(&state.registers[*start_reg + 1]) + } else { + None + }; + let func_result = match json_func { + JsonFunc::JsonArrayLength => { + json_array_length(json_value, path_value) + } + JsonFunc::JsonType => json_type(json_value, path_value), + _ => unreachable!(), + }; + + match func_result { + Ok(result) => state.registers[*dest] = result, + Err(e) => return Err(e), + } + } + }, crate::function::Func::Scalar(scalar_func) => match scalar_func { ScalarFunc::Cast => { assert!(arg_count == 2); From 04cd6555743d5ff53571bb963b871cee69460f81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EA=B9=80=EC=84=A0=EC=9A=B0?= Date: Tue, 14 Jan 2025 17:19:33 +0900 Subject: [PATCH 2/5] Change VALID_URL_PREFIX to use sqlite instead --- .../java/src/main/java/org/github/tursodatabase/JDBC.java | 2 +- .../java/src/test/java/org/github/tursodatabase/JDBCTest.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bindings/java/src/main/java/org/github/tursodatabase/JDBC.java b/bindings/java/src/main/java/org/github/tursodatabase/JDBC.java index 87200ff5c..a341f032a 100644 --- a/bindings/java/src/main/java/org/github/tursodatabase/JDBC.java +++ b/bindings/java/src/main/java/org/github/tursodatabase/JDBC.java @@ -7,7 +7,7 @@ import java.util.Properties; import java.util.logging.Logger; public class JDBC implements Driver { - private static final String VALID_URL_PREFIX = "jdbc:limbo:"; + private static final String VALID_URL_PREFIX = "jdbc:sqlite:"; static { try { diff --git a/bindings/java/src/test/java/org/github/tursodatabase/JDBCTest.java b/bindings/java/src/test/java/org/github/tursodatabase/JDBCTest.java index d0cdc4dc3..45452f810 100644 --- a/bindings/java/src/test/java/org/github/tursodatabase/JDBCTest.java +++ b/bindings/java/src/test/java/org/github/tursodatabase/JDBCTest.java @@ -20,13 +20,13 @@ class JDBCTest { @Test void non_null_connection_is_returned_when_valid_url_is_passed() throws Exception { String fileUrl = TestUtils.createTempFile(); - LimboConnection connection = JDBC.createConnection("jdbc:limbo:" + fileUrl, new Properties()); + LimboConnection connection = JDBC.createConnection("jdbc:sqlite:" + fileUrl, new Properties()); assertThat(connection).isNotNull(); } @Test void connection_can_be_retrieved_from_DriverManager() throws SQLException { - try (Connection connection = DriverManager.getConnection("jdbc:limbo:sample.db")) { + try (Connection connection = DriverManager.getConnection("jdbc:sqlite:sample.db")) { assertThat(connection).isNotNull(); } } From d223c72d032d6f208f403a2c68c8a9e261175323 Mon Sep 17 00:00:00 2001 From: Pekka Enberg Date: Tue, 14 Jan 2025 10:25:11 +0200 Subject: [PATCH 3/5] Revert "core: Previous commits didn't actually remove nix as dependency, so do that here" This reverts commit cca3846f950c262c6a92c28980df155d94ef658b, we need to bring it back unfortunately. --- Cargo.lock | 1 + core/Cargo.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index aa3f2d2e8..fd7bec345 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1217,6 +1217,7 @@ dependencies = [ "miette", "mimalloc", "mockall", + "nix 0.29.0", "pest", "pest_derive", "polling", diff --git a/core/Cargo.toml b/core/Cargo.toml index 0daa58c0d..c0c12a0a3 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -40,6 +40,7 @@ fallible-iterator = "0.3.0" hex = "0.4.3" libc = "0.2.155" log = "0.4.20" +nix = { version = "0.29.0", features = ["fs"] } sieve-cache = "0.1.4" sqlite3-parser = { path = "../vendored/sqlite3-parser" } thiserror = "1.0.61" From 5c9505e8f72854d359a5a3a89d6442c1ffd1a3e8 Mon Sep 17 00:00:00 2001 From: Pekka Enberg Date: Tue, 14 Jan 2025 10:25:23 +0200 Subject: [PATCH 4/5] Revert "core/io/io_uring: replace nix and libc calls with their rustix counterparts." This reverts commit b146f5d4cba52648942d5b4d03b66ae649a50371 because it causes tests to hang. --- core/io/io_uring.rs | 81 +++++++++++++++++++++++++-------------------- 1 file changed, 46 insertions(+), 35 deletions(-) diff --git a/core/io/io_uring.rs b/core/io/io_uring.rs index 14a6bd83c..4f6001cdf 100644 --- a/core/io/io_uring.rs +++ b/core/io/io_uring.rs @@ -1,13 +1,11 @@ use super::{common, Completion, File, OpenFlags, IO}; use crate::{LimboError, Result}; +use libc::{c_short, fcntl, flock, iovec, F_SETLK}; use log::{debug, trace}; -use rustix::fs::{self, FlockOperation, OFlags}; -use rustix::io_uring::iovec; +use nix::fcntl::{FcntlArg, OFlag}; use std::cell::RefCell; use std::collections::HashMap; use std::fmt; -use std::io::ErrorKind; -use std::os::fd::AsFd; use std::os::unix::io::AsRawFd; use std::rc::Rc; use thiserror::Error; @@ -138,12 +136,12 @@ impl IO for UringIO { .open(path)?; // Let's attempt to enable direct I/O. Not all filesystems support it // so ignore any errors. - let fd = file.as_fd(); + let fd = file.as_raw_fd(); if direct { - match fs::fcntl_setfl(fd, OFlags::DIRECT) { - Ok(_) => {} + match nix::fcntl::fcntl(fd, FcntlArg::F_SETFL(OFlag::O_DIRECT)) { + Ok(_) => {}, Err(error) => debug!("Error {error:?} returned when setting O_DIRECT flag to read file. The performance of the system may be affected"), - } + }; } let uring_file = Rc::new(UringFile { io: self.inner.clone(), @@ -201,39 +199,52 @@ pub struct UringFile { impl File for UringFile { fn lock_file(&self, exclusive: bool) -> Result<()> { - let fd = self.file.as_fd(); + let fd = self.file.as_raw_fd(); + let flock = flock { + l_type: if exclusive { + libc::F_WRLCK as c_short + } else { + libc::F_RDLCK as c_short + }, + l_whence: libc::SEEK_SET as c_short, + l_start: 0, + l_len: 0, // Lock entire file + l_pid: 0, + }; + // F_SETLK is a non-blocking lock. The lock will be released when the file is closed // or the process exits or after an explicit unlock. - fs::fcntl_lock( - fd, - if exclusive { - FlockOperation::LockExclusive + let lock_result = unsafe { fcntl(fd, F_SETLK, &flock) }; + if lock_result == -1 { + let err = std::io::Error::last_os_error(); + if err.kind() == std::io::ErrorKind::WouldBlock { + return Err(LimboError::LockingError( + "File is locked by another process".into(), + )); } else { - FlockOperation::LockShared - }, - ) - .map_err(|e| { - let io_error = std::io::Error::from(e); - let message = match io_error.kind() { - ErrorKind::WouldBlock => { - "Failed locking file. File is locked by another process".to_string() - } - _ => format!("Failed locking file, {}", io_error), - }; - LimboError::LockingError(message) - })?; - + return Err(LimboError::IOError(err)); + } + } Ok(()) } fn unlock_file(&self) -> Result<()> { - let fd = self.file.as_fd(); - fs::fcntl_lock(fd, FlockOperation::Unlock).map_err(|e| { - LimboError::LockingError(format!( + let fd = self.file.as_raw_fd(); + let flock = flock { + l_type: libc::F_UNLCK as c_short, + l_whence: libc::SEEK_SET as c_short, + l_start: 0, + l_len: 0, + l_pid: 0, + }; + + let unlock_result = unsafe { fcntl(fd, F_SETLK, &flock) }; + if unlock_result == -1 { + return Err(LimboError::LockingError(format!( "Failed to release file lock: {}", - std::io::Error::from(e) - )) - })?; + std::io::Error::last_os_error() + ))); + } Ok(()) } @@ -250,7 +261,7 @@ impl File for UringFile { let len = buf.len(); let buf = buf.as_mut_ptr(); let iovec = io.get_iovec(buf, len); - io_uring::opcode::Readv::new(fd, iovec as *const iovec as *const libc::iovec, 1) + io_uring::opcode::Readv::new(fd, iovec, 1) .offset(pos as u64) .build() .user_data(io.ring.get_key()) @@ -271,7 +282,7 @@ impl File for UringFile { let buf = buffer.borrow(); trace!("pwrite(pos = {}, length = {})", pos, buf.len()); let iovec = io.get_iovec(buf.as_ptr(), buf.len()); - io_uring::opcode::Writev::new(fd, iovec as *const iovec as *const libc::iovec, 1) + io_uring::opcode::Writev::new(fd, iovec, 1) .offset(pos as u64) .build() .user_data(io.ring.get_key()) From cd04bf796a5a947d3becb7a2dea6bf6e354aede7 Mon Sep 17 00:00:00 2001 From: Pekka Enberg Date: Tue, 14 Jan 2025 10:34:00 +0200 Subject: [PATCH 5/5] Update README.md --- README.md | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 91d7cdad1..4c5d86908 100644 --- a/README.md +++ b/README.md @@ -25,14 +25,12 @@ ## Features -* In-process OLTP database engine library -* Asynchronous I/O support on Linux with `io_uring` -* SQLite compatibility ([status](COMPAT.md)) - * SQL dialect support - * File format support - * SQLite C API -* JavaScript/WebAssembly bindings (_wip_) -* Support for Linux, macOS, and Windows +Limbo is an in-process OLTP database engine library that has: + +* **Asynchronous I/O** support on Linux with `io_uring` +* **SQLite compatibility** [[doc](COMPAT.md)] for SQL dialect, file formats, and the C API +* **Language bindings** for JavaScript/WebAssembly, Rust, Python, and Java +* **OS support** for Linux, macOS, and Windows ## Getting Started