From f7888fce8387a8c81261eb1d3b1282eb20b410eb Mon Sep 17 00:00:00 2001 From: Darren Schroeder <343840+fdncred@users.noreply.github.com> Date: Fri, 13 Jun 2025 16:29:07 -0500 Subject: [PATCH] fix `stor` insert/delete collision (#15838) # Description Based on some testing in [Discord](https://discord.com/channels/601130461678272522/1349836000804995196/1353138803640111135) we were able to find that `insert` and `delete` happening at the same time caused problems in the `stor` command. So, I added `conn.is_busy()` with a sleep to try and avoid that problem. ![image](https://github.com/user-attachments/assets/e01bccab-0aaa-40ab-b0bf-25e3c72aa037) /cc @NotTheDr01ds @132ikl # User-Facing Changes # Tests + Formatting # After Submitting --- .../nu-command/src/database/values/sqlite.rs | 43 ++++++++++++---- crates/nu-command/src/stor/delete.rs | 4 +- crates/nu-command/src/stor/insert.rs | 51 ++++++++++--------- 3 files changed, 63 insertions(+), 35 deletions(-) diff --git a/crates/nu-command/src/database/values/sqlite.rs b/crates/nu-command/src/database/values/sqlite.rs index 1bfa870b8f..7fee136269 100644 --- a/crates/nu-command/src/database/values/sqlite.rs +++ b/crates/nu-command/src/database/values/sqlite.rs @@ -112,16 +112,31 @@ impl SQLiteDatabase { if self.path == PathBuf::from(MEMORY_DB) { open_connection_in_memory_custom() } else { - Connection::open(&self.path).map_err(|e| ShellError::GenericError { + let conn = Connection::open(&self.path).map_err(|e| ShellError::GenericError { error: "Failed to open SQLite database from open_connection".into(), msg: e.to_string(), span: None, help: None, inner: vec![], - }) + })?; + conn.busy_handler(Some(SQLiteDatabase::sleeper)) + .map_err(|e| ShellError::GenericError { + error: "Failed to set busy handler for SQLite database".into(), + msg: e.to_string(), + span: None, + help: None, + inner: vec![], + })?; + Ok(conn) } } + fn sleeper(attempts: i32) -> bool { + log::warn!("SQLITE_BUSY, retrying after 250ms (attempt {})", attempts); + std::thread::sleep(std::time::Duration::from_millis(250)); + true + } + pub fn get_tables(&self, conn: &Connection) -> Result, SqliteError> { let mut table_names = conn.prepare("SELECT name FROM sqlite_master WHERE type = 'table'")?; @@ -668,13 +683,23 @@ pub fn convert_sqlite_value_to_nu_value(value: ValueRef, span: Span) -> Value { pub fn open_connection_in_memory_custom() -> Result { let flags = OpenFlags::default(); - Connection::open_with_flags(MEMORY_DB, flags).map_err(|e| ShellError::GenericError { - error: "Failed to open SQLite custom connection in memory".into(), - msg: e.to_string(), - span: Some(Span::test_data()), - help: None, - inner: vec![], - }) + let conn = + Connection::open_with_flags(MEMORY_DB, flags).map_err(|e| ShellError::GenericError { + error: "Failed to open SQLite custom connection in memory".into(), + msg: e.to_string(), + span: Some(Span::test_data()), + help: None, + inner: vec![], + })?; + conn.busy_handler(Some(SQLiteDatabase::sleeper)) + .map_err(|e| ShellError::GenericError { + error: "Failed to set busy handler for SQLite custom connection in memory".into(), + msg: e.to_string(), + span: Some(Span::test_data()), + help: None, + inner: vec![], + })?; + Ok(conn) } pub fn open_connection_in_memory() -> Result { diff --git a/crates/nu-command/src/stor/delete.rs b/crates/nu-command/src/stor/delete.rs index 12443addd5..eb0170e2ef 100644 --- a/crates/nu-command/src/stor/delete.rs +++ b/crates/nu-command/src/stor/delete.rs @@ -109,7 +109,9 @@ impl Command for StorDelete { // dbg!(&sql_stmt); conn.execute(&sql_stmt, []) .map_err(|err| ShellError::GenericError { - error: "Failed to open SQLite connection in memory from delete".into(), + error: + "Failed to delete using the SQLite connection in memory from delete.rs." + .into(), msg: err.to_string(), span: Some(Span::test_data()), help: None, diff --git a/crates/nu-command/src/stor/insert.rs b/crates/nu-command/src/stor/insert.rs index faa5c37e54..b1fbe3afea 100644 --- a/crates/nu-command/src/stor/insert.rs +++ b/crates/nu-command/src/stor/insert.rs @@ -164,34 +164,35 @@ fn process( } let new_table_name = table_name.unwrap_or("table".into()); + let mut create_stmt = format!("INSERT INTO {} (", new_table_name); + let mut column_placeholders: Vec = Vec::new(); + + let cols = record.columns(); + cols.for_each(|col| { + column_placeholders.push(col.to_string()); + }); + + create_stmt.push_str(&column_placeholders.join(", ")); + + // Values are set as placeholders. + create_stmt.push_str(") VALUES ("); + let mut value_placeholders: Vec = Vec::new(); + for (index, _) in record.columns().enumerate() { + value_placeholders.push(format!("?{}", index + 1)); + } + create_stmt.push_str(&value_placeholders.join(", ")); + create_stmt.push(')'); + + // dbg!(&create_stmt); + + // Get the params from the passed values + let params = values_to_sql(record.values().cloned())?; + if let Ok(conn) = db.open_connection() { - let mut create_stmt = format!("INSERT INTO {} (", new_table_name); - let mut column_placeholders: Vec = Vec::new(); - - let cols = record.columns(); - cols.for_each(|col| { - column_placeholders.push(col.to_string()); - }); - - create_stmt.push_str(&column_placeholders.join(", ")); - - // Values are set as placeholders. - create_stmt.push_str(") VALUES ("); - let mut value_placeholders: Vec = Vec::new(); - for (index, _) in record.columns().enumerate() { - value_placeholders.push(format!("?{}", index + 1)); - } - create_stmt.push_str(&value_placeholders.join(", ")); - create_stmt.push(')'); - - // dbg!(&create_stmt); - - // Get the params from the passed values - let params = values_to_sql(record.values().cloned())?; - conn.execute(&create_stmt, params_from_iter(params)) .map_err(|err| ShellError::GenericError { - error: "Failed to open SQLite connection in memory from insert".into(), + error: "Failed to insert using the SQLite connection in memory from insert.rs." + .into(), msg: err.to_string(), span: Some(Span::test_data()), help: None,