From 3ad7d194cbf8b4f159cbe0cce6dd99fd3d6f1a22 Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Thu, 3 Apr 2025 08:38:48 -0400 Subject: [PATCH 1/8] Prevent panic on loading non-existent vtab module --- core/util.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/core/util.rs b/core/util.rs index f17699233..b0acb54f2 100644 --- a/core/util.rs +++ b/core/util.rs @@ -60,8 +60,14 @@ pub fn parse_schema_rows( let sql: &str = row.get::<&str>(4)?; if root_page == 0 && sql.to_lowercase().contains("create virtual") { let name: &str = row.get::<&str>(1)?; - let vtab = syms.vtabs.get(name).unwrap().clone(); - schema.add_virtual_table(vtab); + let Some(vtab) = syms.vtabs.get(name) else { + return Err(LimboError::InvalidArgument(format!( + "Virtual table Module for {} not found in symbol table, + please load extension first", + name + ))); + }; + schema.add_virtual_table(vtab.clone()); } else { let table = schema::BTreeTable::from_sql(sql, root_page as usize)?; schema.add_btree_table(Rc::new(table)); From 4b9b6c969b2095f6aa0a7fcd2b31a4baf8b5cdd6 Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Thu, 3 Apr 2025 09:17:06 -0400 Subject: [PATCH 2/8] Parse schema rows after extensions are loaded --- core/ext/dynamic.rs | 14 ++++++++++++-- core/lib.rs | 37 ++++++++++++++++++++++++++++++------- core/util.rs | 2 +- core/vdbe/execute.rs | 6 +++--- core/vdbe/mod.rs | 6 +++--- 5 files changed, 49 insertions(+), 16 deletions(-) diff --git a/core/ext/dynamic.rs b/core/ext/dynamic.rs index df342caca..ec297bf36 100644 --- a/core/ext/dynamic.rs +++ b/core/ext/dynamic.rs @@ -6,6 +6,7 @@ use libloading::{Library, Symbol}; use limbo_ext::{ExtensionApi, ExtensionApiRef, ExtensionEntryPoint, ResultCode, VfsImpl}; use std::{ ffi::{c_char, CString}, + rc::Rc, sync::{Arc, Mutex, OnceLock}, }; @@ -29,7 +30,10 @@ unsafe impl Send for VfsMod {} unsafe impl Sync for VfsMod {} impl Connection { - pub fn load_extension>(&self, path: P) -> crate::Result<()> { + pub fn load_extension>( + self: &Rc, + path: P, + ) -> crate::Result<()> { use limbo_ext::ExtensionApiRef; let api = Box::new(self.build_limbo_ext()); @@ -44,7 +48,13 @@ impl Connection { let result_code = unsafe { entry(api_ptr) }; if result_code.is_ok() { let extensions = get_extension_libraries(); - extensions.lock().unwrap().push((Arc::new(lib), api_ref)); + extensions + .lock() + .map_err(|_| { + LimboError::ExtensionError("Error unlocking extension libraries".to_string()) + })? + .push((Arc::new(lib), api_ref)); + self.parse_schema_rows()?; Ok(()) } else { if !api_ptr.is_null() { diff --git a/core/lib.rs b/core/lib.rs index e364b226d..7176086f0 100644 --- a/core/lib.rs +++ b/core/lib.rs @@ -70,7 +70,7 @@ use vdbe::{builder::QueryMode, VTabOpaqueCursor}; pub type Result = std::result::Result; pub static DATABASE_VERSION: OnceLock = OnceLock::new(); -#[derive(Clone, PartialEq, Eq)] +#[derive(Clone, Copy, PartialEq, Eq)] enum TransactionState { Write, Read, @@ -158,7 +158,13 @@ impl Database { .try_write() .expect("lock on schema should succeed first try"); let syms = conn.syms.borrow(); - parse_schema_rows(rows, &mut schema, io, syms.deref(), None)?; + if let Err(LimboError::ExtensionError(e)) = + parse_schema_rows(rows, &mut schema, io, syms.deref(), None) + { + // this means that a vtab exists and we no longer have the module loaded. we print + // a warning to the user to load the module + eprintln!("Warning: {}", e); + } } Ok(db) } @@ -186,9 +192,9 @@ impl Database { schema: self.schema.clone(), header: self.header.clone(), last_insert_rowid: Cell::new(0), - auto_commit: RefCell::new(true), + auto_commit: Cell::new(true), mv_transactions: RefCell::new(Vec::new()), - transaction_state: RefCell::new(TransactionState::None), + transaction_state: Cell::new(TransactionState::None), last_change: Cell::new(0), syms: RefCell::new(SymbolTable::new()), total_changes: Cell::new(0), @@ -278,9 +284,9 @@ pub struct Connection { pager: Rc, schema: Arc>, header: Arc>, - auto_commit: RefCell, + auto_commit: Cell, mv_transactions: RefCell>, - transaction_state: RefCell, + transaction_state: Cell, last_insert_rowid: Cell, last_change: Cell, total_changes: Cell, @@ -517,7 +523,24 @@ impl Connection { } pub fn get_auto_commit(&self) -> bool { - *self.auto_commit.borrow() + self.auto_commit.get() + } + + pub fn parse_schema_rows(self: &Rc) -> Result<()> { + let rows = self.query("SELECT * FROM sqlite_schema")?; + let mut schema = self + .schema + .try_write() + .expect("lock on schema should succeed first try"); + let syms = self.syms.borrow(); + if let Err(LimboError::ExtensionError(e)) = + parse_schema_rows(rows, &mut schema, self.pager.io.clone(), syms.deref(), None) + { + // this means that a vtab exists and we no longer have the module loaded. we print + // a warning to the user to load the module + eprintln!("Warning: {}", e); + } + Ok(()) } } diff --git a/core/util.rs b/core/util.rs index b0acb54f2..55a50b7ae 100644 --- a/core/util.rs +++ b/core/util.rs @@ -61,7 +61,7 @@ pub fn parse_schema_rows( if root_page == 0 && sql.to_lowercase().contains("create virtual") { let name: &str = row.get::<&str>(1)?; let Some(vtab) = syms.vtabs.get(name) else { - return Err(LimboError::InvalidArgument(format!( + return Err(LimboError::ExtensionError(format!( "Virtual table Module for {} not found in symbol table, please load extension first", name diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index 09c283ecd..89a2d8d93 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -1542,8 +1542,8 @@ pub fn op_transaction( } } else { let connection = program.connection.upgrade().unwrap(); - let current_state = connection.transaction_state.borrow().clone(); - let (new_transaction_state, updated) = match (¤t_state, write) { + let current_state = connection.transaction_state.get(); + let (new_transaction_state, updated) = match (current_state, write) { (TransactionState::Write, true) => (TransactionState::Write, false), (TransactionState::Write, false) => (TransactionState::Write, false), (TransactionState::Read, true) => (TransactionState::Write, true), @@ -1597,7 +1597,7 @@ pub fn op_auto_commit( }; } - if *auto_commit != *conn.auto_commit.borrow() { + if *auto_commit != conn.auto_commit.get() { if *rollback { todo!("Rollback is not implemented"); } else { diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs index 8794b208a..550f21164 100644 --- a/core/vdbe/mod.rs +++ b/core/vdbe/mod.rs @@ -386,7 +386,7 @@ impl Program { ) -> Result { if let Some(mv_store) = mv_store { let conn = self.connection.upgrade().unwrap(); - let auto_commit = *conn.auto_commit.borrow(); + let auto_commit = conn.auto_commit.get(); if auto_commit { let mut mv_transactions = conn.mv_transactions.borrow_mut(); for tx_id in mv_transactions.iter() { @@ -400,7 +400,7 @@ impl Program { .connection .upgrade() .expect("only weak ref to connection?"); - let auto_commit = *connection.auto_commit.borrow(); + let auto_commit = connection.auto_commit.get(); tracing::trace!("Halt auto_commit {}", auto_commit); assert!( program_state.halt_state.is_none() @@ -409,7 +409,7 @@ impl Program { if program_state.halt_state.is_some() { self.step_end_write_txn(&pager, &mut program_state.halt_state, connection.deref()) } else if auto_commit { - let current_state = connection.transaction_state.borrow().clone(); + let current_state = connection.transaction_state.get(); match current_state { TransactionState::Write => self.step_end_write_txn( &pager, From c15035caf84c23ad9e61cdfbab7cafb6b1726b7d Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Thu, 3 Apr 2025 14:03:54 -0400 Subject: [PATCH 3/8] Add module and vtab to schema after table is reopened with proper ext --- core/lib.rs | 20 +++--- core/translate/emitter.rs | 1 + core/util.rs | 132 +++++++++++++++++++++++++++++++++++--- core/vdbe/execute.rs | 13 +++- 4 files changed, 147 insertions(+), 19 deletions(-) diff --git a/core/lib.rs b/core/lib.rs index 7176086f0..48fb2bd7c 100644 --- a/core/lib.rs +++ b/core/lib.rs @@ -157,9 +157,9 @@ impl Database { let mut schema = schema .try_write() .expect("lock on schema should succeed first try"); - let syms = conn.syms.borrow(); + let syms = conn.syms.borrow_mut(); if let Err(LimboError::ExtensionError(e)) = - parse_schema_rows(rows, &mut schema, io, syms.deref(), None) + parse_schema_rows(rows, &mut schema, io, syms, None) { // this means that a vtab exists and we no longer have the module loaded. we print // a warning to the user to load the module @@ -532,13 +532,15 @@ impl Connection { .schema .try_write() .expect("lock on schema should succeed first try"); - let syms = self.syms.borrow(); - if let Err(LimboError::ExtensionError(e)) = - parse_schema_rows(rows, &mut schema, self.pager.io.clone(), syms.deref(), None) { - // this means that a vtab exists and we no longer have the module loaded. we print - // a warning to the user to load the module - eprintln!("Warning: {}", e); + let syms = self.syms.borrow_mut(); + if let Err(LimboError::ExtensionError(e)) = + parse_schema_rows(rows, &mut schema, self.pager.io.clone(), syms, None) + { + // this means that a vtab exists and we no longer have the module loaded. we print + // a warning to the user to load the module + eprintln!("Warning: {}", e); + } } Ok(()) } @@ -653,7 +655,7 @@ impl VirtualTable { module_name )))?; if let VTabKind::VirtualTable = kind { - if module.module_kind != VTabKind::VirtualTable { + if module.module_kind == VTabKind::TableValuedFunction { return Err(LimboError::ExtensionError(format!( "{} is not a virtual table module", module_name diff --git a/core/translate/emitter.rs b/core/translate/emitter.rs index 23b937019..514bc21ab 100644 --- a/core/translate/emitter.rs +++ b/core/translate/emitter.rs @@ -135,6 +135,7 @@ fn prologue<'a>( Ok((t_ctx, init_label, start_offset)) } +#[derive(Clone, Copy, Debug)] pub enum TransactionMode { None, Read, diff --git a/core/util.rs b/core/util.rs index 55a50b7ae..f7c41348c 100644 --- a/core/util.rs +++ b/core/util.rs @@ -1,5 +1,5 @@ use limbo_sqlite3_parser::ast::{self, CreateTableBody, Expr, FunctionTail, Literal}; -use std::{rc::Rc, sync::Arc}; +use std::{cell::RefMut, rc::Rc, sync::Arc}; use crate::{ schema::{self, Column, Schema, Type}, @@ -40,7 +40,7 @@ pub fn parse_schema_rows( rows: Option, schema: &mut Schema, io: Arc, - syms: &SymbolTable, + mut syms: RefMut, mv_tx_id: Option, ) -> Result<()> { if let Some(mut rows) = rows { @@ -60,12 +60,35 @@ pub fn parse_schema_rows( let sql: &str = row.get::<&str>(4)?; if root_page == 0 && sql.to_lowercase().contains("create virtual") { let name: &str = row.get::<&str>(1)?; - let Some(vtab) = syms.vtabs.get(name) else { - return Err(LimboError::ExtensionError(format!( - "Virtual table Module for {} not found in symbol table, - please load extension first", - name - ))); + // a virtual table is found in the sqlite_schema, but it's no + // longer in the symbol table. We need to recreate it. + let vtab = if let Some(vtab) = syms.vtabs.get(name) { + vtab.clone() + } else { + // "create virtual table using mod" + let mod_name = module_name_from_sql(sql)?; + if let Some(vmod) = syms.vtab_modules.get(mod_name) { + if let limbo_ext::VTabKind::VirtualTable = vmod.module_kind + { + let vtab = crate::VirtualTable::from_args( + Some(name), + mod_name, + module_args_from_sql(sql)?, + &syms, + vmod.module_kind, + None, + )?; + syms.vtabs.insert(name.to_string(), vtab.clone()); + vtab + } else { + return Err(LimboError::Corrupt("Table valued function: {name} registered as virtual table in schema".to_string())); + } + } else { + return Err(LimboError::ExtensionError(format!( + "Virtual table module '{}' not found\nPlease load extension", + &mod_name + ))); + } }; schema.add_virtual_table(vtab.clone()); } else { @@ -138,6 +161,99 @@ pub fn check_ident_equivalency(ident1: &str, ident2: &str) -> bool { strip_quotes(ident1).eq_ignore_ascii_case(strip_quotes(ident2)) } +fn module_name_from_sql(sql: &str) -> Result<&str> { + if let Some(start) = sql.find("USING") { + let start = start + 6; + // stop at the first space, semicolon, or parenthesis + let end = sql[start..] + .find(|c: char| c.is_whitespace() || c == ';' || c == '(') + .unwrap_or(sql.len() - start) + + start; + Ok(sql[start..end].trim()) + } else { + Err(LimboError::InvalidArgument( + "Expected 'USING' in module name".to_string(), + )) + } +} + +// CREATE VIRTUAL TABLE table_name USING module_name(arg1, arg2, ...); +// CREATE VIRTUAL TABLE table_name USING module_name; +fn module_args_from_sql(sql: &str) -> Result> { + if !sql.contains('(') { + return Ok(vec![]); + } + let start = sql.find('(').ok_or_else(|| { + LimboError::InvalidArgument("Expected '(' in module argument list".to_string()) + })? + 1; + let end = sql.rfind(')').ok_or_else(|| { + LimboError::InvalidArgument("Expected ')' in module argument list".to_string()) + })?; + + let mut args = Vec::new(); + let mut current_arg = String::new(); + let mut chars = sql[start..end].chars().peekable(); + let mut in_quotes = false; + + while let Some(c) = chars.next() { + match c { + '\'' => { + if in_quotes { + if chars.peek() == Some(&'\'') { + // Escaped quote + current_arg.push('\''); + chars.next(); + } else { + in_quotes = false; + args.push(limbo_ext::Value::from_text(current_arg.trim().to_string())); + current_arg.clear(); + // Skip until comma or end + while let Some(&nc) = chars.peek() { + if nc == ',' { + chars.next(); // Consume comma + break; + } else if nc.is_whitespace() { + chars.next(); + } else { + return Err(LimboError::InvalidArgument( + "Unexpected characters after quoted argument".to_string(), + )); + } + } + } + } else { + in_quotes = true; + } + } + ',' => { + if !in_quotes { + if !current_arg.trim().is_empty() { + args.push(limbo_ext::Value::from_text(current_arg.trim().to_string())); + current_arg.clear(); + } + } else { + current_arg.push(c); + } + } + _ => { + current_arg.push(c); + } + } + } + + if !current_arg.trim().is_empty() && !in_quotes { + args.push(limbo_ext::Value::from_text(current_arg.trim().to_string())); + } + + if in_quotes { + return Err(LimboError::InvalidArgument( + "Unterminated string literal in module arguments".to_string(), + )); + } + + Ok(args) +} + pub fn check_literal_equivalency(lhs: &Literal, rhs: &Literal) -> bool { match (lhs, rhs) { (Literal::Numeric(n1), Literal::Numeric(n2)) => cmp_numeric_strings(n1, n2), diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index 89a2d8d93..65bf5238e 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -917,12 +917,21 @@ pub fn op_vcreate( "Failed to upgrade Connection".to_string(), )); }; + let mod_type = conn + .syms + .borrow() + .vtab_modules + .get(&module_name) + .ok_or_else(|| { + crate::LimboError::ExtensionError(format!("Module {} not found", module_name)) + })? + .module_kind; let table = crate::VirtualTable::from_args( Some(&table_name), &module_name, args, &conn.syms.borrow(), - limbo_ext::VTabKind::VirtualTable, + mod_type, None, )?; { @@ -4231,7 +4240,7 @@ pub fn op_parse_schema( Some(stmt), &mut schema, conn.pager.io.clone(), - &conn.syms.borrow(), + conn.syms.borrow_mut(), state.mv_tx_id, )?; state.pc += 1; From a0f71e27beece3aea2fdf24087ab14253006d29d Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Thu, 3 Apr 2025 14:04:28 -0400 Subject: [PATCH 4/8] Fix cli tests --- testing/cli_tests/extensions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testing/cli_tests/extensions.py b/testing/cli_tests/extensions.py index bab8cb74f..ac870ee4d 100755 --- a/testing/cli_tests/extensions.py +++ b/testing/cli_tests/extensions.py @@ -345,10 +345,10 @@ def test_kv(): limbo = TestLimboShell() limbo.run_test_fn( "create virtual table t using kv_store;", - lambda res: "Virtual table module not found: kv_store" in res, + lambda res: "Module kv_store not found" in res, ) limbo.execute_dot(f".load {ext_path}") - limbo.debug_print( + limbo.execute_dot( "create virtual table t using kv_store;", ) limbo.run_test_fn(".schema", lambda res: "CREATE VIRTUAL TABLE t" in res) From 6b5ec1f07b30a3d90af8f64daf4031fdd04139ea Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Thu, 3 Apr 2025 20:55:59 -0400 Subject: [PATCH 5/8] Remove mut borrow from sym table in parse schema fn --- core/ext/dynamic.rs | 4 +++- core/lib.rs | 8 ++++---- core/util.rs | 14 ++++++-------- core/vdbe/execute.rs | 16 +++++++++------- 4 files changed, 22 insertions(+), 20 deletions(-) diff --git a/core/ext/dynamic.rs b/core/ext/dynamic.rs index ec297bf36..60e4050d7 100644 --- a/core/ext/dynamic.rs +++ b/core/ext/dynamic.rs @@ -54,7 +54,9 @@ impl Connection { LimboError::ExtensionError("Error unlocking extension libraries".to_string()) })? .push((Arc::new(lib), api_ref)); - self.parse_schema_rows()?; + { + self.parse_schema_rows()?; + } Ok(()) } else { if !api_ptr.is_null() { diff --git a/core/lib.rs b/core/lib.rs index 48fb2bd7c..cd728fa64 100644 --- a/core/lib.rs +++ b/core/lib.rs @@ -157,9 +157,9 @@ impl Database { let mut schema = schema .try_write() .expect("lock on schema should succeed first try"); - let syms = conn.syms.borrow_mut(); + let syms = conn.syms.borrow(); if let Err(LimboError::ExtensionError(e)) = - parse_schema_rows(rows, &mut schema, io, syms, None) + parse_schema_rows(rows, &mut schema, io, &syms, None) { // this means that a vtab exists and we no longer have the module loaded. we print // a warning to the user to load the module @@ -533,9 +533,9 @@ impl Connection { .try_write() .expect("lock on schema should succeed first try"); { - let syms = self.syms.borrow_mut(); + let syms = self.syms.borrow(); if let Err(LimboError::ExtensionError(e)) = - parse_schema_rows(rows, &mut schema, self.pager.io.clone(), syms, None) + parse_schema_rows(rows, &mut schema, self.pager.io.clone(), &syms, None) { // this means that a vtab exists and we no longer have the module loaded. we print // a warning to the user to load the module diff --git a/core/util.rs b/core/util.rs index f7c41348c..dab083c16 100644 --- a/core/util.rs +++ b/core/util.rs @@ -1,5 +1,5 @@ use limbo_sqlite3_parser::ast::{self, CreateTableBody, Expr, FunctionTail, Literal}; -use std::{cell::RefMut, rc::Rc, sync::Arc}; +use std::{rc::Rc, sync::Arc}; use crate::{ schema::{self, Column, Schema, Type}, @@ -40,7 +40,7 @@ pub fn parse_schema_rows( rows: Option, schema: &mut Schema, io: Arc, - mut syms: RefMut, + syms: &SymbolTable, mv_tx_id: Option, ) -> Result<()> { if let Some(mut rows) = rows { @@ -70,16 +70,14 @@ pub fn parse_schema_rows( if let Some(vmod) = syms.vtab_modules.get(mod_name) { if let limbo_ext::VTabKind::VirtualTable = vmod.module_kind { - let vtab = crate::VirtualTable::from_args( + crate::VirtualTable::from_args( Some(name), mod_name, module_args_from_sql(sql)?, - &syms, + syms, vmod.module_kind, None, - )?; - syms.vtabs.insert(name.to_string(), vtab.clone()); - vtab + )? } else { return Err(LimboError::Corrupt("Table valued function: {name} registered as virtual table in schema".to_string())); } @@ -90,7 +88,7 @@ pub fn parse_schema_rows( ))); } }; - schema.add_virtual_table(vtab.clone()); + schema.add_virtual_table(vtab); } else { let table = schema::BTreeTable::from_sql(sql, root_page as usize)?; schema.add_btree_table(Rc::new(table)); diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index 65bf5238e..654e9a2c5 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -4236,13 +4236,15 @@ pub fn op_parse_schema( ))?; let mut schema = conn.schema.write(); // TODO: This function below is synchronous, make it async - parse_schema_rows( - Some(stmt), - &mut schema, - conn.pager.io.clone(), - conn.syms.borrow_mut(), - state.mv_tx_id, - )?; + { + parse_schema_rows( + Some(stmt), + &mut schema, + conn.pager.io.clone(), + &conn.syms.borrow(), + state.mv_tx_id, + )?; + } state.pc += 1; Ok(InsnFunctionStepResult::Step) } From 3a7f1e4056a967f8380158973153d1134bd66716 Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Thu, 3 Apr 2025 20:57:59 -0400 Subject: [PATCH 6/8] Add comments explaining flow of reloading vtabs from schema tbl --- core/util.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/util.rs b/core/util.rs index dab083c16..ca23f0e83 100644 --- a/core/util.rs +++ b/core/util.rs @@ -61,11 +61,11 @@ pub fn parse_schema_rows( if root_page == 0 && sql.to_lowercase().contains("create virtual") { let name: &str = row.get::<&str>(1)?; // a virtual table is found in the sqlite_schema, but it's no - // longer in the symbol table. We need to recreate it. + // longer in the in-memory schema. We need to recreate it if + // the module is loaded in the symbol table. let vtab = if let Some(vtab) = syms.vtabs.get(name) { vtab.clone() } else { - // "create virtual table using mod" let mod_name = module_name_from_sql(sql)?; if let Some(vmod) = syms.vtab_modules.get(mod_name) { if let limbo_ext::VTabKind::VirtualTable = vmod.module_kind @@ -82,6 +82,7 @@ pub fn parse_schema_rows( return Err(LimboError::Corrupt("Table valued function: {name} registered as virtual table in schema".to_string())); } } else { + // the extension isn't loaded, so we emit a warning. return Err(LimboError::ExtensionError(format!( "Virtual table module '{}' not found\nPlease load extension", &mod_name From 41ac91f14f4c528e24bcafe95f0d3409519ad75f Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Fri, 4 Apr 2025 09:55:43 -0400 Subject: [PATCH 7/8] Add tests for parsing vtab creation sql in ParseSchema --- core/util.rs | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/core/util.rs b/core/util.rs index ca23f0e83..3d12a2c6e 100644 --- a/core/util.rs +++ b/core/util.rs @@ -1753,4 +1753,88 @@ pub mod tests { Ok((OwnedValueType::Float, "1.23e4")) ); } + + #[test] + fn test_module_name_basic() { + let sql = "CREATE VIRTUAL TABLE x USING y;"; + assert_eq!(module_name_from_sql(sql).unwrap(), "y"); + } + + #[test] + fn test_module_name_with_args() { + let sql = "CREATE VIRTUAL TABLE x USING modname('a', 'b');"; + assert_eq!(module_name_from_sql(sql).unwrap(), "modname"); + } + + #[test] + fn test_module_name_missing_using() { + let sql = "CREATE VIRTUAL TABLE x (a, b);"; + assert!(module_name_from_sql(sql).is_err()); + } + + #[test] + fn test_module_name_no_semicolon() { + let sql = "CREATE VIRTUAL TABLE x USING limbo(a, b)"; + assert_eq!(module_name_from_sql(sql).unwrap(), "limbo"); + } + + #[test] + fn test_module_name_no_semicolon_or_args() { + let sql = "CREATE VIRTUAL TABLE x USING limbo"; + assert_eq!(module_name_from_sql(sql).unwrap(), "limbo"); + } + + #[test] + fn test_module_args_none() { + let sql = "CREATE VIRTUAL TABLE x USING modname;"; + let args = module_args_from_sql(sql).unwrap(); + assert_eq!(args.len(), 0); + } + + #[test] + fn test_module_args_basic() { + let sql = "CREATE VIRTUAL TABLE x USING modname('arg1', 'arg2');"; + let args = module_args_from_sql(sql).unwrap(); + assert_eq!(args.len(), 2); + assert_eq!("arg1", args[0].to_text().unwrap()); + assert_eq!("arg2", args[1].to_text().unwrap()); + for arg in args { + unsafe { arg.__free_internal_type() } + } + } + + #[test] + fn test_module_args_with_escaped_quote() { + let sql = "CREATE VIRTUAL TABLE x USING modname('a''b', 'c');"; + let args = module_args_from_sql(sql).unwrap(); + assert_eq!(args.len(), 2); + assert_eq!(args[0].to_text().unwrap(), "a'b"); + assert_eq!(args[1].to_text().unwrap(), "c"); + for arg in args { + unsafe { arg.__free_internal_type() } + } + } + + #[test] + fn test_module_args_unterminated_string() { + let sql = "CREATE VIRTUAL TABLE x USING modname('arg1, 'arg2');"; + assert!(module_args_from_sql(sql).is_err()); + } + + #[test] + fn test_module_args_extra_garbage_after_quote() { + let sql = "CREATE VIRTUAL TABLE x USING modname('arg1'x);"; + assert!(module_args_from_sql(sql).is_err()); + } + + #[test] + fn test_module_args_trailing_comma() { + let sql = "CREATE VIRTUAL TABLE x USING modname('arg1',);"; + let args = module_args_from_sql(sql).unwrap(); + assert_eq!(args.len(), 1); + assert_eq!("arg1", args[0].to_text().unwrap()); + for arg in args { + unsafe { arg.__free_internal_type() } + } + } } From 9b1e60a29c4882ab2551acaedac7b079849ce5a9 Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Tue, 8 Apr 2025 20:09:12 -0400 Subject: [PATCH 8/8] Fix typo in ext library lock err message --- core/ext/dynamic.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/ext/dynamic.rs b/core/ext/dynamic.rs index 60e4050d7..17138f268 100644 --- a/core/ext/dynamic.rs +++ b/core/ext/dynamic.rs @@ -51,7 +51,7 @@ impl Connection { extensions .lock() .map_err(|_| { - LimboError::ExtensionError("Error unlocking extension libraries".to_string()) + LimboError::ExtensionError("Error locking extension libraries".to_string()) })? .push((Arc::new(lib), api_ref)); {