DuckDB lifetimes

benchmarks/duckdb-bench/src/lib.rs

@@ -14,14 +14,14 @@ use vortex_bench::Benchmark;
 use vortex_bench::Format;
 use vortex_bench::IdempotentPath;
 use vortex_bench::generate_duckdb_registration_sql;
-use vortex_duckdb::duckdb::Config;
-use vortex_duckdb::duckdb::Connection;
-use vortex_duckdb::duckdb::Database;
+use vortex_duckdb::duckdb::OwnedConfig;
+use vortex_duckdb::duckdb::OwnedConnection;
+use vortex_duckdb::duckdb::OwnedDatabase;
 
 /// DuckDB context for benchmarks.
 pub struct DuckClient {
-    pub db: Database,
-    pub connection: Connection,
+    pub db: OwnedDatabase,
+    pub connection: OwnedConnection,
     pub db_path: PathBuf,
     pub threads: Option<usize>,
 }
@@ -65,17 +65,17 @@ impl DuckClient {
     pub fn open_and_setup_database(
         path: Option<PathBuf>,
         threads: Option<usize>,
-    ) -> Result<(Database, Connection)> {
-        let mut config = Config::new().vortex_expect("failed to create duckdb config");
+    ) -> Result<(OwnedDatabase, OwnedConnection)> {
+        let mut config = OwnedConfig::new().vortex_expect("failed to create duckdb config");
 
         // Set DuckDB thread count if specified
         if let Some(thread_count) = threads {
             config.set("threads", &format!("{}", thread_count))?;
         }
 
         let db = match path {
-            Some(path) => Database::open_with_config(path, config),
-            None => Database::open_in_memory_with_config(config),
+            Some(path) => OwnedDatabase::open_with_config(path, config),
+            None => OwnedDatabase::open_in_memory_with_config(config),
         }?;
 
         let connection = db.connect()?;
@@ -100,17 +100,6 @@ impl DuckClient {
     }
 
     pub fn reopen(&mut self) -> Result<()> {
-        // take ownership of the connection & database
-        let mut connection = unsafe { Connection::borrow(self.connection.as_ptr()) };
-        std::mem::swap(&mut self.connection, &mut connection);
-        let mut db = unsafe { Database::borrow(self.db.as_ptr()) };
-        std::mem::swap(&mut self.db, &mut db);
-
-        // drop the connection, then the database (order might be important?)
-        // NB: self.db and self.connection will be dangling pointers, which we'll fix below
-        drop(connection);
-        drop(db);
-
         let (mut db, mut connection) =
             Self::open_and_setup_database(Some(self.db_path.clone()), self.threads)?;
 

vortex-duckdb/cpp/copy_function.cpp

@@ -45,16 +45,20 @@ unique_ptr<FunctionData> c_bind_one(ClientContext &context,
                                     const vector<LogicalType> &column_types) {
 
     auto c_column_names = vector<char *>();
+    c_column_names.reserve(column_names.size());
     for (const auto &col_id : column_names) {
         c_column_names.push_back(const_cast<char *>(col_id.c_str()));
     }
 
     auto c_column_types = vector<duckdb_logical_type>();
+    c_column_types.reserve(c_column_types.size());
     for (auto &col_type : column_types) {
         c_column_types.push_back(reinterpret_cast<duckdb_logical_type>(const_cast<LogicalType *>(&col_type)));
     }
 
     duckdb_vx_error error_out = nullptr;
+    // TODO(myrrc): do we pass ownership of c_column_names in bind?
+    // If yes, it's a UB as we'd double delete on function return
     auto ffi_bind_data = copy_vtab_one.bind(reinterpret_cast<duckdb_vx_copy_func_bind_input>(&info),
                                             c_column_names.data(),
                                             c_column_names.size(),

vortex-duckdb/cpp/file_system.cpp

@@ -33,7 +33,7 @@ duckdb_vx_fs_open(duckdb_client_context ctx, const char *path, duckdb_vx_error *
     }
 
     try {
-        auto *client_context = reinterpret_cast<ClientContext *>(ctx);
+        auto client_context = reinterpret_cast<ClientContext *>(ctx);
         auto &fs = FileSystem::GetFileSystem(*client_context);
         auto handle = fs.OpenFile(path, FileFlags::FILE_FLAGS_READ | FileFlags::FILE_FLAGS_PARALLEL_ACCESS);
         return reinterpret_cast<duckdb_vx_file_handle>(new FileHandleWrapper(std::move(handle)));
@@ -51,7 +51,7 @@ duckdb_vx_fs_create(duckdb_client_context ctx, const char *path, duckdb_vx_error
     }
 
     try {
-        auto *client_context = reinterpret_cast<ClientContext *>(ctx);
+        auto client_context = reinterpret_cast<ClientContext *>(ctx);
         auto &fs = FileSystem::GetFileSystem(*client_context);
         auto handle = fs.OpenFile(path,
                                   FileFlags::FILE_FLAGS_WRITE | FileFlags::FILE_FLAGS_FILE_CREATE |
@@ -142,7 +142,7 @@ extern "C" duckdb_state duckdb_vx_fs_list_files(duckdb_client_context ctx,
     }
 
     try {
-        auto *client_context = reinterpret_cast<ClientContext *>(ctx);
+        auto client_context = reinterpret_cast<ClientContext *>(ctx);
         auto &fs = FileSystem::GetFileSystem(*client_context);
 
         fs.ListFiles(directory,