Merge 'Lower ownership requirement for Value' from Levy A.

If you have a `&str` you would need to allocate and copy the string just
to pass a reference to it again. Same goes if you have a slice of bytes.
In all (most?) situations, that is not what you want and sometimes
impossible to satisfy. Example:
```rs
impl From<&str> for Value<'_> {
    fn from(value: &str) -> Self {
        Self::Text(&value.to_owned())
    }
}
```
Here, there is no way to pass a reference to a `String` without making
the lifetime `'static`, since the string has to be dropped by the end of
the function or leaked. I would consider this a anti-pattern. There is
no reason to keep a shared reference to a owned value. (And can't think
of any situation where you would need such thing)
Now, this is possible:
```rs
impl<'a> From<&'a str> for Value<'a> {
    fn from(value: &'a str) -> Self {
        Self::Text(value)
    }
}
```

Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com>

Closes #838
This commit is contained in:
Pekka Enberg
2025-02-04 13:10:10 +02:00
5 changed files with 12 additions and 8 deletions

View File

@@ -192,7 +192,7 @@ fn to_js_value(value: limbo_core::Value) -> JsValue {
}
limbo_core::Value::Float(f) => JsValue::from(f),
limbo_core::Value::Text(t) => JsValue::from_str(t),
limbo_core::Value::Blob(b) => js_sys::Uint8Array::from(b.as_slice()).into(),
limbo_core::Value::Blob(b) => js_sys::Uint8Array::from(b).into(),
}
}

View File

@@ -15,8 +15,8 @@ pub enum Value<'a> {
Null,
Integer(i64),
Float(f64),
Text(&'a String),
Blob(&'a Vec<u8>),
Text(&'a str),
Blob(&'a [u8]),
}
impl Display for Value<'_> {

View File

@@ -58,8 +58,12 @@ pub(crate) fn do_flush(conn: &Rc<Connection>, tmp_db: &TempDatabase) -> anyhow::
Ok(())
}
pub(crate) fn compare_string(a: &String, b: &String) {
pub(crate) fn compare_string(a: impl AsRef<str>, b: impl AsRef<str>) {
let a = a.as_ref();
let b = b.as_ref();
assert_eq!(a.len(), b.len(), "Strings are not equal in size!");
let a = a.as_bytes();
let b = b.as_bytes();

View File

@@ -59,8 +59,8 @@ mod tests {
limbo_core::Value::Null => rusqlite::types::Value::Null,
limbo_core::Value::Integer(x) => rusqlite::types::Value::Integer(*x),
limbo_core::Value::Float(x) => rusqlite::types::Value::Real(*x),
limbo_core::Value::Text(x) => rusqlite::types::Value::Text((*x).clone()),
limbo_core::Value::Blob(x) => rusqlite::types::Value::Blob((*x).clone()),
limbo_core::Value::Text(x) => rusqlite::types::Value::Text(x.to_string()),
limbo_core::Value::Blob(x) => rusqlite::types::Value::Blob(x.to_vec()),
})
.collect()
}

View File

@@ -46,12 +46,12 @@ fn test_statement_bind() -> anyhow::Result<()> {
let mut stmt = conn.prepare("select ?, ?1, :named, ?3, ?4")?;
stmt.bind_at(1.try_into()?, Value::Text(&"hello".to_string()));
stmt.bind_at(1.try_into()?, Value::Text("hello"));
let i = stmt.parameters().index(":named").unwrap();
stmt.bind_at(i, Value::Integer(42));
stmt.bind_at(3.try_into()?, Value::Blob(&vec![0x1, 0x2, 0x3]));
stmt.bind_at(3.try_into()?, Value::Blob(&[0x1, 0x2, 0x3]));
stmt.bind_at(4.try_into()?, Value::Float(0.5));