Merge remote-tracking branch 'origin/main' into scalar-with-state

Author: mlafeldt

README.md

@@ -127,31 +127,57 @@ declarations for DuckDB's C API. By default, `libduckdb-sys` attempts to find a
 
 You can adjust this behavior in a number of ways:
 
-- If you use the `bundled` feature, `libduckdb-sys` will use the
-  [cc](https://crates.io/crates/cc) crate to compile DuckDB from source and
-  link against that. This source is embedded in the `libduckdb-sys` crate and
-  as we are still in development, we will update it regularly. After we are more stable,
-  we will use the stable released version from [duckdb](https://github.com/duckdb/duckdb/releases).
-  This is probably the simplest solution to any build problems. You can enable this by adding the following in your `Cargo.toml` file:
-
-  ```bash
-  cargo add duckdb --features bundled
-  ```
-
-  `Cargo.toml` will be updated.
-
-  ```toml
-  [dependencies]
-  # Assume that version DuckDB version 0.9.2 is used.
-  duckdb = { version = "0.9.2", features = ["bundled"] }
-  ```
-
-* When linking against a DuckDB library already on the system (so _not_ using any of the `bundled` features), you can set the `DUCKDB_LIB_DIR` environment variable to point to a directory containing the library. You can also set the `DUCKDB_INCLUDE_DIR` variable to point to the directory containing `duckdb.h`.
-* Installing the duckdb development packages will usually be all that is required, but
-  the build helpers for [pkg-config](https://github.com/alexcrichton/pkg-config-rs)
-  and [vcpkg](https://github.com/mcgoo/vcpkg-rs) have some additional configuration
-  options. The default when using vcpkg is to dynamically link,
-  which must be enabled by setting `VCPKGRS_DYNAMIC=1` environment variable before build.
+1. If you use the `bundled` feature, `libduckdb-sys` will use the
+   [cc](https://crates.io/crates/cc) crate to compile DuckDB from source and
+   link against that. This source is embedded in the `libduckdb-sys` crate and
+   as we are still in development, we will update it regularly. After we are more stable,
+   we will use the stable released version from [duckdb](https://github.com/duckdb/duckdb/releases).
+   This is probably the simplest solution to any build problems. You can enable this by adding the following in your `Cargo.toml` file:
+
+   ```bash
+   cargo add duckdb --features bundled
+   ```
+
+   `Cargo.toml` will be updated.
+
+   ```toml
+   [dependencies]
+   duckdb = { version = "1.3.2", features = ["bundled"] }
+   ```
+
+2. When linking against a DuckDB library already on the system (so _not_ using any of the `bundled` features), you can set the `DUCKDB_LIB_DIR` environment variable to point to a directory containing the library. You can also set the `DUCKDB_INCLUDE_DIR` variable to point to the directory containing `duckdb.h`.
+
+   Linux example:
+
+   ```shell
+   wget https://github.com/duckdb/duckdb/releases/download/v1.3.2/libduckdb-linux-arm64.zip
+   unzip libduckdb-linux-arm64.zip -d libduckdb
+
+   export DUCKDB_LIB_DIR=$PWD/libduckdb
+   export DUCKDB_INCLUDE_DIR=$DUCKDB_LIB_DIR
+   export LD_LIBRARY_PATH=$DUCKDB_LIB_DIR
+
+   cargo build --examples
+   ```
+
+   macOS example:
+
+   ```shell
+   wget https://github.com/duckdb/duckdb/releases/download/v1.3.2/libduckdb-osx-universal.zip
+   unzip libduckdb-osx-universal.zip -d libduckdb
+
+   export DUCKDB_LIB_DIR=$PWD/libduckdb
+   export DUCKDB_INCLUDE_DIR=$DUCKDB_LIB_DIR
+   export DYLD_FALLBACK_LIBRARY_PATH=$DUCKDB_LIB_DIR
+
+   cargo build --examples
+   ```
+
+3. Installing the duckdb development packages will usually be all that is required, but
+   the build helpers for [pkg-config](https://github.com/alexcrichton/pkg-config-rs)
+   and [vcpkg](https://github.com/mcgoo/vcpkg-rs) have some additional configuration
+   options. The default when using vcpkg is to dynamically link,
+   which must be enabled by setting `VCPKGRS_DYNAMIC=1` environment variable before build.
 
 ### Binding generation
 

crates/duckdb/src/appender/mod.rs

@@ -161,9 +161,7 @@ impl Appender<'_> {
 impl Drop for Appender<'_> {
     fn drop(&mut self) {
         if !self.app.is_null() {
-            let _ = self.flush(); // can't safely handle failures here
             unsafe {
-                ffi::duckdb_appender_close(self.app);
                 ffi::duckdb_appender_destroy(&mut self.app);
             }
         }

crates/duckdb/src/error.rs

@@ -47,6 +47,10 @@ pub enum Error {
     /// for [`query_row`](crate::Connection::query_row)) did not return any.
     QueryReturnedNoRows,
 
+    /// Error when a query that was expected to return only one row (e.g.,
+    /// for [`query_one`](crate::Connection::query_one)) did return more than one.
+    QueryReturnedMoreThanOneRow,
+
     /// Error when the value of a particular column is requested, but the index
     /// is out of range for the statement.
     InvalidColumnIndex(usize),
@@ -96,6 +100,7 @@ impl PartialEq for Error {
             (Self::InvalidPath(p1), Self::InvalidPath(p2)) => p1 == p2,
             (Self::ExecuteReturnedResults, Self::ExecuteReturnedResults) => true,
             (Self::QueryReturnedNoRows, Self::QueryReturnedNoRows) => true,
+            (Self::QueryReturnedMoreThanOneRow, Self::QueryReturnedMoreThanOneRow) => true,
             (Self::InvalidColumnIndex(i1), Self::InvalidColumnIndex(i2)) => i1 == i2,
             (Self::InvalidColumnName(n1), Self::InvalidColumnName(n2)) => n1 == n2,
             (Self::InvalidColumnType(i1, n1, t1), Self::InvalidColumnType(i2, n2, t2)) => {
@@ -170,6 +175,7 @@ impl fmt::Display for Error {
                 write!(f, "Execute returned results - did you mean to call query?")
             }
             Self::QueryReturnedNoRows => write!(f, "Query returned no rows"),
+            Self::QueryReturnedMoreThanOneRow => write!(f, "Query returned more than one row"),
             Self::InvalidColumnIndex(i) => write!(f, "Invalid column index: {i}"),
             Self::InvalidColumnName(ref name) => write!(f, "Invalid column name: {name}"),
             Self::InvalidColumnType(i, ref name, ref t) => {
@@ -201,6 +207,7 @@ impl error::Error for Error {
             | Self::InvalidParameterName(_)
             | Self::ExecuteReturnedResults
             | Self::QueryReturnedNoRows
+            | Self::QueryReturnedMoreThanOneRow
             | Self::InvalidColumnIndex(_)
             | Self::InvalidColumnName(_)
             | Self::InvalidColumnType(..)

crates/duckdb/src/lib.rs

@@ -1238,6 +1238,32 @@ mod test {
             }
             Ok(())
         }
+
+        #[test]
+        fn test_rows_and_then_with_custom_error() -> Result<()> {
+            let db = checked_memory_handle();
+            db.execute_batch("CREATE TABLE test (value INTEGER)")?;
+            db.execute_batch("INSERT INTO test VALUES (1), (3), (5)")?;
+
+            let mut stmt = db.prepare("SELECT value FROM test ORDER BY value")?;
+            let rows = stmt.query([])?;
+
+            // Use and_then to apply custom validation with custom error type
+            let results: Vec<i32> = rows
+                .and_then(|row| -> CustomResult<i32> {
+                    let val: i32 = row.get(0)?; // duckdb::Error automatically converted via From trait
+                    if val > 10 {
+                        Err(CustomError::SomeError) // Custom application-specific error
+                    } else {
+                        Ok(val)
+                    }
+                })
+                .collect::<CustomResult<Vec<_>>>()
+                .unwrap();
+
+            assert_eq!(results, vec![1, 3, 5]);
+            Ok(())
+        }
     }
 
     #[test]

crates/duckdb/src/row.rs

@@ -58,6 +58,11 @@ impl<'stmt> Rows<'stmt> {
 
     /// Map over this `Rows`, converting it to a [`Map`], which
     /// implements `FallibleIterator`.
+    ///
+    /// **Note:** This method requires the closure to return `duckdb::Result<B>`.
+    /// If you need to use custom error types, consider using [`and_then`](Self::and_then)
+    /// instead, which allows any error type that implements `From<duckdb::Error>`.
+    ///
     /// ```rust,no_run
     /// use fallible_iterator::FallibleIterator;
     /// # use duckdb::{Result, Statement};

crates/duckdb/src/statement.rs

@@ -349,6 +349,33 @@ impl Statement<'_> {
         self.query(params)?.get_expected_row().and_then(f)
     }
 
+    /// Convenience method to execute a query that is expected to return exactly
+    /// one row.
+    ///
+    /// Returns `Err(QueryReturnedMoreThanOneRow)` if the query returns more than one row.
+    ///
+    /// Returns `Err(QueryReturnedNoRows)` if no results are returned. If the
+    /// query truly is optional, you can call
+    /// [`.optional()`](crate::OptionalExt::optional) on the result of
+    /// this to get a `Result<Option<T>>` (requires that the trait
+    /// `duckdb::OptionalExt` is imported).
+    ///
+    /// # Failure
+    ///
+    /// Will return `Err` if the underlying DuckDB call fails.
+    pub fn query_one<T, P, F>(&mut self, params: P, f: F) -> Result<T>
+    where
+        P: Params,
+        F: FnOnce(&Row<'_>) -> Result<T>,
+    {
+        let mut rows = self.query(params)?;
+        let row = rows.get_expected_row().and_then(f)?;
+        if rows.next()?.is_some() {
+            return Err(Error::QueryReturnedMoreThanOneRow);
+        }
+        Ok(row)
+    }
+
     /// Return the row count
     #[inline]
     pub fn row_count(&self) -> usize {
@@ -635,9 +662,8 @@ mod test {
 
         let mut stmt = db.prepare("SELECT id FROM test where name = ?")?;
         {
-            let mut rows = stmt.query([&"one"])?;
-            let id: Result<i32> = rows.next()?.unwrap().get(0);
-            assert_eq!(Ok(1), id);
+            let id: i32 = stmt.query_one([&"one"], |r| r.get(0))?;
+            assert_eq!(id, 1);
         }
         Ok(())
     }
@@ -825,6 +851,71 @@ mod test {
         Ok(())
     }
 
+    #[test]
+    fn test_query_one() -> Result<()> {
+        let db = Connection::open_in_memory()?;
+        let sql = "BEGIN;
+                   CREATE TABLE foo(x INTEGER, y INTEGER);
+                   INSERT INTO foo VALUES(1, 3);
+                   INSERT INTO foo VALUES(2, 4);
+                   END;";
+        db.execute_batch(sql)?;
+
+        // Exactly one row
+        let y: i32 = db
+            .prepare("SELECT y FROM foo WHERE x = ?")?
+            .query_one([1], |r| r.get(0))?;
+        assert_eq!(y, 3);
+
+        // No rows
+        let res: Result<i32> = db
+            .prepare("SELECT y FROM foo WHERE x = ?")?
+            .query_one([99], |r| r.get(0));
+        assert_eq!(res.unwrap_err(), Error::QueryReturnedNoRows);
+
+        // Multiple rows
+        let res: Result<i32> = db.prepare("SELECT y FROM foo")?.query_one([], |r| r.get(0));
+        assert_eq!(res.unwrap_err(), Error::QueryReturnedMoreThanOneRow);
+
+        Ok(())
+    }
+
+    #[test]
+    fn test_query_one_optional() -> Result<()> {
+        use crate::OptionalExt;
+
+        let db = Connection::open_in_memory()?;
+        let sql = "BEGIN;
+                   CREATE TABLE foo(x INTEGER, y INTEGER);
+                   INSERT INTO foo VALUES(1, 3);
+                   INSERT INTO foo VALUES(2, 4);
+                   END;";
+        db.execute_batch(sql)?;
+
+        // Exactly one row
+        let y: Option<i32> = db
+            .prepare("SELECT y FROM foo WHERE x = ?")?
+            .query_one([1], |r| r.get(0))
+            .optional()?;
+        assert_eq!(y, Some(3));
+
+        // No rows
+        let y: Option<i32> = db
+            .prepare("SELECT y FROM foo WHERE x = ?")?
+            .query_one([99], |r| r.get(0))
+            .optional()?;
+        assert_eq!(y, None);
+
+        // Multiple rows - should still return error (not converted by optional)
+        let res = db
+            .prepare("SELECT y FROM foo")?
+            .query_one([], |r| r.get::<_, i32>(0))
+            .optional();
+        assert_eq!(res.unwrap_err(), Error::QueryReturnedMoreThanOneRow);
+
+        Ok(())
+    }
+
     #[test]
     fn test_query_by_column_name() -> Result<()> {
         let db = Connection::open_in_memory()?;

crates/duckdb/src/types/mod.rs

@@ -312,7 +312,6 @@ mod test {
 
         let mut stmt = db.prepare("SELECT b, t, i, f, n FROM foo")?;
         let mut rows = stmt.query([])?;
-
         let row = rows.next()?.unwrap();
 
         // check the correct types come back as expected
@@ -371,7 +370,6 @@ mod test {
 
         let mut stmt = db.prepare("SELECT b, t, i, f, n FROM foo")?;
         let mut rows = stmt.query([])?;
-
         let row = rows.next()?.unwrap();
         // NOTE: this is different from SQLite
         // assert_eq!(Value::Blob(vec![1, 2]), row.get::<_, Value>(0)?);

crates/duckdb/src/types/to_sql.rs

@@ -316,10 +316,7 @@ mod test {
 
         db.execute("INSERT INTO foo (id) VALUES (gen_random_uuid())", [])?;
 
-        let mut stmt = db.prepare("SELECT id FROM foo")?;
-        let mut rows = stmt.query([])?;
-        let row = rows.next()?.unwrap();
-        let found_id: String = row.get_unwrap(0);
+        let found_id: String = db.prepare("SELECT id FROM foo")?.query_one([], |r| r.get(0))?;
         assert_eq!(found_id.len(), 36);
         Ok(())
     }
@@ -337,11 +334,9 @@ mod test {
         let id_vec = id.as_bytes().to_vec();
         db.execute("INSERT INTO foo (id, label) VALUES (?, ?)", params![id_vec, "target"])?;
 
-        let mut stmt = db.prepare("SELECT id, label FROM foo WHERE id = ?")?;
-        let mut rows = stmt.query(params![id_vec])?;
-        let row = rows.next()?.unwrap();
-        let found_id: Uuid = row.get_unwrap(0);
-        let found_label: String = row.get_unwrap(1);
+        let (found_id, found_label): (Uuid, String) = db
+            .prepare("SELECT id, label FROM foo WHERE id = ?")?
+            .query_one(params![id_vec], |r| Ok((r.get_unwrap(0), r.get_unwrap(1))))?;
         assert_eq!(found_id, id);
         assert_eq!(found_label, "target");
         Ok(())
@@ -359,11 +354,9 @@ mod test {
         let id = Uuid::new_v4();
         db.execute("INSERT INTO foo (id, label) VALUES (?, ?)", params![id, "target"])?;
 
-        let mut stmt = db.prepare("SELECT id, label FROM foo WHERE id = ?")?;
-        let mut rows = stmt.query(params![id])?;
-        let row = rows.next()?.unwrap();
-        let found_id: Uuid = row.get_unwrap(0);
-        let found_label: String = row.get_unwrap(1);
+        let (found_id, found_label): (Uuid, String) = db
+            .prepare("SELECT id, label FROM foo WHERE id = ?")?
+            .query_one(params![id], |r| Ok((r.get_unwrap(0), r.get_unwrap(1))))?;
         assert_eq!(found_id, id);
         assert_eq!(found_label, "target");
         Ok(())

crates/duckdb/src/vtab/value.rs

@@ -1,5 +1,5 @@
-use crate::ffi::{duckdb_destroy_value, duckdb_get_int64, duckdb_get_varchar, duckdb_value};
-use std::{ffi::CString, fmt};
+use crate::ffi::{duckdb_destroy_value, duckdb_free, duckdb_get_int64, duckdb_get_varchar, duckdb_value};
+use std::{ffi::CStr, fmt, os::raw::c_void};
 
 /// The Value object holds a single arbitrary value of any type that can be
 /// stored in the database.
@@ -34,7 +34,27 @@ impl Value {
 
 impl fmt::Display for Value {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let c_string = unsafe { CString::from_raw(duckdb_get_varchar(self.ptr)) };
-        write!(f, "{}", c_string.to_str().unwrap())
+        unsafe {
+            let varchar = duckdb_get_varchar(self.ptr);
+            let c_str = CStr::from_ptr(varchar);
+            let res = write!(f, "{}", c_str.to_string_lossy());
+            duckdb_free(varchar as *mut c_void);
+            res
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::ffi::duckdb_create_varchar;
+    use std::ffi::CString;
+
+    #[test]
+    fn test_value_to_string() {
+        let c_str = CString::new("some value").unwrap();
+        let duckdb_val = unsafe { duckdb_create_varchar(c_str.as_ptr()) };
+        let val = Value::from(duckdb_val);
+        assert_eq!(val.to_string(), "some value");
     }
 }