Repository: nushell/nushell (language: rust)

Issue:
`stor update` fails with single-quote (apostrophe) in string (but `stor insert` works)
### Describe the bug

Attempting to use a string in the `stor update --update-record` with an embedded single quote fails.  However, inserting that same record into a row entry works fine.

### How to reproduce

```nushell
stor reset   # Optional
stor create --table-name test_table --columns { id: int, value: str }
# => ╭────────────┬────────────────╮
# => │ test_table │ [list 0 items] │
# => ╰────────────┴────────────────╯

# We can insert new row with a string with a single quote just fine
stor insert -t test_table --data-record {
  id: 1
  value: "Doesn't fail"
}
# => ╭────────────┬───────────────────────────╮
# => │            │ ╭───┬────┬──────────────╮ │
# => │ test_table │ │ # │ id │    value     │ │
# => │            │ ├───┼────┼──────────────┤ │
# => │            │ │ 0 │  1 │ Doesn't fail │ │
# => │            │ ╰───┴────┴──────────────╯ │
# => ╰────────────┴───────────────────────────╯

# We can update the row as long as it doesn't have a single quote
stor update -t test_table --where-clause 'id = 1' --update-record {
  id: 1
  value: "This also works"
}
# => ╭────────────┬──────────────────────────────╮
# => │            │ ╭───┬────┬─────────────────╮ │
# => │ test_table │ │ # │ id │      value      │ │
# => │            │ ├───┼────┼─────────────────┤ │
# => │            │ │ 0 │  1 │ This also works │ │
# => │            │ ╰───┴────┴─────────────────╯ │
# => ╰────────────┴──────────────────────────────╯

# But an update with a single-quote in the string will fail
stor update -t test_table --where-clause 'id = 1' --update-record {
  id: 1
  value: "But this doesn't work"
}
# => Error:
# =>   × Failed to open SQLite connection in memory from update
# =>    ╭─[source:1:1]
# =>  1 │ nu
# =>    · ▲
# =>    · ╰── near "t": syntax error in UPDATE test_table SET id = 1, value = 'But this doesn't work' WHERE id = 1 at offset 54
# =>    ╰────
```

Note also that the span is incorrect - It points to a span start of 0 apparently, meaning it loads `view files | get 0` (a.k.a. the `source` entry)

### Expected behavior

Record is updated properly.

### Configuration

| key                | value                                    |
| ------------------ | ---------------------------------------- |
| version            | 0.101.1                                  |
| major              | 0                                        |
| minor              | 101                                      |
| patch              | 1                                        |
| branch             | main                                     |
| commit_hash        | 299453ecb7bff2be8fbb3ca6402ce743a1060d03 |
| build_os           | linux-x86_64                             |
| build_target       | x86_64-unknown-linux-gnu                 |
| rust_version       | rustc 1.82.0 (f6e511eec 2024-10-15)      |
| rust_channel       | 1.82.0-x86_64-unknown-linux-gnu          |
| cargo_version      | cargo 1.82.0 (8f40fc59f 2024-08-21)      |
| build_time         | 2025-01-24 09:22:03 -05:00               |
| build_rust_channel | release                                  |
| allocator          | mimalloc                                 |
| features           | default, sqlite, trash                   |
| installed_plugins  | formats 0.101.0, gstat 0.101.1           |


Discussion / hints:
I haven't had a chance to compare the different in `stor insert` vs. `stor update` yet to see if there's an easy, obvious fix.  Just wanted to get it written up while I had it easily reproducable.
`insert` uses bound parameters:

https://github.com/nushell/nushell/blob/befeddad59e2dace0e687d7eed8ca961dc570412/crates/nu-command/src/stor/insert.rs#L189-L192

while `update` builds an SQL string itself:

https://github.com/nushell/nushell/blob/befeddad59e2dace0e687d7eed8ca961dc570412/crates/nu-command/src/stor/update.rs#L174-L182
Hopefully someone can make it work. When I wrote all this mess I used sql. Maybe this PR will give someone an idea of what to do https://github.com/nushell/nushell/pull/12820
@weirdan @fdncred Thanks - I think I'm starting to grok the code a bit.  I'll take a stab at it, but don't mind if someone else gets to it first :-).

Fix patch:
diff --git a/crates/nu-command/src/stor/insert.rs b/crates/nu-command/src/stor/insert.rs
index 94b5bdf940130..4a2ebf492893b 100644
--- a/crates/nu-command/src/stor/insert.rs
+++ b/crates/nu-command/src/stor/insert.rs
@@ -161,27 +161,23 @@ fn process(
     let new_table_name = table_name.unwrap_or("table".into());
 
     if let Ok(conn) = db.open_connection() {
-        let mut create_stmt = format!("INSERT INTO {} ( ", new_table_name);
+        let mut create_stmt = format!("INSERT INTO {} (", new_table_name);
+        let mut column_placeholders: Vec<String> = Vec::new();
+
         let cols = record.columns();
         cols.for_each(|col| {
-            create_stmt.push_str(&format!("{}, ", col));
+            column_placeholders.push(col.to_string());
         });
-        if create_stmt.ends_with(", ") {
-            create_stmt.pop();
-            create_stmt.pop();
-        }
+
+        create_stmt.push_str(&column_placeholders.join(", "));
 
         // Values are set as placeholders.
-        create_stmt.push_str(") VALUES ( ");
+        create_stmt.push_str(") VALUES (");
+        let mut value_placeholders: Vec<String> = Vec::new();
         for (index, _) in record.columns().enumerate() {
-            create_stmt.push_str(&format!("?{}, ", index + 1));
+            value_placeholders.push(format!("?{}", index + 1));
         }
-
-        if create_stmt.ends_with(", ") {
-            create_stmt.pop();
-            create_stmt.pop();
-        }
-
+        create_stmt.push_str(&value_placeholders.join(", "));
         create_stmt.push(')');
 
         // dbg!(&create_stmt);
diff --git a/crates/nu-command/src/stor/update.rs b/crates/nu-command/src/stor/update.rs
index a158e0f7d063d..8a77f41d51071 100644
--- a/crates/nu-command/src/stor/update.rs
+++ b/crates/nu-command/src/stor/update.rs
@@ -1,6 +1,7 @@
-use crate::database::{SQLiteDatabase, MEMORY_DB};
+use crate::database::{values_to_sql, SQLiteDatabase, MEMORY_DB};
 use nu_engine::command_prelude::*;
 use nu_protocol::Signals;
+use rusqlite::params_from_iter;
 
 #[derive(Clone)]
 pub struct StorUpdate;
@@ -163,36 +164,12 @@ fn process(
         let mut update_stmt = format!("UPDATE {} ", new_table_name);
 
         update_stmt.push_str("SET ");
-        let vals = record.iter();
-        vals.for_each(|(key, val)| match val {
-            Value::Int { val, .. } => {
-                update_stmt.push_str(&format!("{} = {}, ", key, val));
-            }
-            Value::Float { val, .. } => {
-                update_stmt.push_str(&format!("{} = {}, ", key, val));
-            }
-            Value::String { val, .. } => {
-                update_stmt.push_str(&format!("{} = '{}', ", key, val));
-            }
-            Value::Date { val, .. } => {
-                update_stmt.push_str(&format!("{} = '{}', ", key, val));
-            }
-            Value::Bool { val, .. } => {
-                update_stmt.push_str(&format!("{} = {}, ", key, val));
-            }
-            _ => {
-                // return Err(ShellError::UnsupportedInput {
-                //     msg: format!("{} is not a valid datepart, expected one of year, month, day, hour, minute, second, millisecond, microsecond, nanosecond", part.item),
-                //     input: "value originates from here".to_string(),
-                //     msg_span: span,
-                //     input_span: val.span(),
-                // });
-            }
-        });
-        if update_stmt.ends_with(", ") {
-            update_stmt.pop();
-            update_stmt.pop();
+        let mut placeholders: Vec<String> = Vec::new();
+
+        for (index, (key, _)) in record.iter().enumerate() {
+            placeholders.push(format!("{} = ?{}", key, index + 1));
         }
+        update_stmt.push_str(&placeholders.join(", "));
 
         // Yup, this is a bit janky, but I'm not sure a better way to do this without having
         // --and and --or flags as well as supporting ==, !=, <>, is null, is not null, etc.
@@ -202,7 +179,10 @@ fn process(
         }
         // dbg!(&update_stmt);
 
-        conn.execute(&update_stmt, [])
+        // Get the params from the passed values
+        let params = values_to_sql(record.values().cloned())?;
+
+        conn.execute(&update_stmt, params_from_iter(params))
             .map_err(|err| ShellError::GenericError {
                 error: "Failed to open SQLite connection in memory from update".into(),
                 msg: err.to_string(),


Test patch:
diff --git a/crates/nu-command/tests/commands/mod.rs b/crates/nu-command/tests/commands/mod.rs
index 73d5d4577a368..3b47b0eb90432 100644
--- a/crates/nu-command/tests/commands/mod.rs
+++ b/crates/nu-command/tests/commands/mod.rs
@@ -105,6 +105,7 @@ mod sort_by;
 mod source_env;
 mod split_column;
 mod split_row;
+mod stor;
 mod str_;
 mod table;
 mod take;
diff --git a/crates/nu-command/tests/commands/stor.rs b/crates/nu-command/tests/commands/stor.rs
new file mode 100644
index 0000000000000..4ed0c5dae0d75
--- /dev/null
+++ b/crates/nu-command/tests/commands/stor.rs
@@ -0,0 +1,37 @@
+use nu_test_support::{nu, pipeline};
+
+#[test]
+fn stor_insert() {
+    let actual = nu!(pipeline(
+        r#"
+            stor create --table-name test_table --columns { id: int, value: str };
+            stor insert -t test_table --data-record {
+                id: 1
+                value: "'Initial value'"
+            };
+            stor open | query db 'select value from test_table' | get 0.value
+        "#
+    ));
+
+    assert_eq!(actual.out, "'Initial value'");
+}
+
+#[test]
+fn stor_update_with_quote() {
+    let actual = nu!(pipeline(
+        r#"
+            stor create --table-name test_table --columns { id: int, value: str };
+            stor insert -t test_table --data-record {
+                id: 1
+                value: "'Initial value'"
+            };
+            stor update -t test_table --where-clause 'id = 1' --update-record {
+                id: 1
+                value: "This didn't work, but should now."
+            };
+            stor open | query db 'select value from test_table' | get 0.value
+        "#
+    ));
+
+    assert_eq!(actual.out, "This didn't work, but should now.");
+}