DuckDB lifetimes

benchmarks/duckdb-bench/src/lib.rs

@@ -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/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,

vortex-duckdb/src/convert/dtype.rs

@@ -60,17 +60,18 @@ use vortex::extension::datetime::Timestamp;
 
 use crate::cpp::DUCKDB_TYPE;
 use crate::duckdb::LogicalType;
+use crate::duckdb::LogicalTypeRef;
 
 pub trait FromLogicalType {
     fn from_logical_type(
-        logical_type: LogicalType,
+        logical_type: &LogicalTypeRef,
         nullability: Nullability,
     ) -> VortexResult<DType>;
 }
 
 impl FromLogicalType for DType {
     fn from_logical_type(
-        logical_type: LogicalType,
+        logical_type: &LogicalTypeRef,
         nullability: Nullability,
     ) -> VortexResult<DType> {
         Ok(match logical_type.as_type_id() {
@@ -125,29 +126,35 @@ impl FromLogicalType for DType {
             DUCKDB_TYPE::DUCKDB_TYPE_TIMESTAMP_NS => {
                 DType::Extension(Timestamp::new(TimeUnit::Nanoseconds, nullability).erased())
             }
-            DUCKDB_TYPE::DUCKDB_TYPE_ARRAY => DType::FixedSizeList(
-                Arc::new(DType::from_logical_type(
-                    logical_type.array_child_type(),
-                    Nullability::Nullable,
-                )?),
-                logical_type.array_type_array_size(),
-                nullability,
-            ),
-            DUCKDB_TYPE::DUCKDB_TYPE_LIST => DType::List(
-                Arc::new(DType::from_logical_type(
-                    logical_type.list_child_type(),
-                    Nullability::Nullable,
-                )?),
-                nullability,
-            ),
+            DUCKDB_TYPE::DUCKDB_TYPE_ARRAY => {
+                let child_type = logical_type.array_child_type();
+                DType::FixedSizeList(
+                    Arc::new(DType::from_logical_type(
+                        &child_type,
+                        Nullability::Nullable,
+                    )?),
+                    logical_type.array_type_array_size(),
+                    nullability,
+                )
+            }
+            DUCKDB_TYPE::DUCKDB_TYPE_LIST => {
+                let child_type = logical_type.list_child_type();
+                DType::List(
+                    Arc::new(DType::from_logical_type(
+                        &child_type,
+                        Nullability::Nullable,
+                    )?),
+                    nullability,
+                )
+            }
             DUCKDB_TYPE::DUCKDB_TYPE_STRUCT => DType::Struct(
                 (0..logical_type.struct_type_child_count())
                     .map(|i| {
                         let child_name = logical_type.struct_child_name(i);
                         let child_type = logical_type.struct_child_type(i);
                         Ok((
                             child_name,
-                            DType::from_logical_type(child_type, Nullability::Nullable)?,
+                            DType::from_logical_type(&child_type, Nullability::Nullable)?,
                         ))
                     })
                     .collect::<VortexResult<_>>()?,
@@ -168,9 +175,9 @@ impl FromLogicalType for DType {
     }
 }
 
-pub fn from_duckdb_table<I, S>(iter: I) -> VortexResult<StructFields>
+pub fn from_duckdb_table<'a, I, S>(iter: I) -> VortexResult<StructFields>
 where
-    I: Iterator<Item = (S, LogicalType, Nullability)>,
+    I: Iterator<Item = (S, &'a LogicalTypeRef, Nullability)>,
     S: AsRef<str>,
 {
     iter.map(|(name, type_, nullability)| {

vortex-duckdb/src/convert/expr.rs

@@ -35,7 +35,7 @@ use crate::duckdb;
 
 const DUCKDB_FUNCTION_NAME_CONTAINS: &str = "contains";
 
-fn like_pattern_str(value: &duckdb::Expression) -> VortexResult<Option<String>> {
+fn like_pattern_str(value: &duckdb::ExpressionRef) -> VortexResult<Option<String>> {
     match value.as_class().vortex_expect("unknown class") {
         duckdb::ExpressionClass::BoundConstant(constant) => {
             Ok(Some(format!("%{}%", constant.value.as_string().as_str())))
@@ -45,7 +45,9 @@ fn like_pattern_str(value: &duckdb::Expression) -> VortexResult<Option<String>>
 }
 
 #[allow(clippy::cognitive_complexity)]
-pub fn try_from_bound_expression(value: &duckdb::Expression) -> VortexResult<Option<Expression>> {
+pub fn try_from_bound_expression(
+    value: &duckdb::ExpressionRef,
+) -> VortexResult<Option<Expression>> {
     let Some(value) = value.as_class() else {
         tracing::debug!("no expression class id {:?}", value.as_class_id());
         return Ok(None);
@@ -56,23 +58,23 @@ pub fn try_from_bound_expression(value: &duckdb::Expression) -> VortexResult<Opt
         duckdb::ExpressionClass::BoundComparison(compare) => {
             let operator: Operator = compare.op.try_into()?;
 
-            let Some(left) = try_from_bound_expression(&compare.left)? else {
+            let Some(left) = try_from_bound_expression(compare.left)? else {
                 return Ok(None);
             };
-            let Some(right) = try_from_bound_expression(&compare.right)? else {
+            let Some(right) = try_from_bound_expression(compare.right)? else {
                 return Ok(None);
             };
 
             Binary.new_expr(operator, [left, right])
         }
         duckdb::ExpressionClass::BoundBetween(between) => {
-            let Some(array) = try_from_bound_expression(&between.input)? else {
+            let Some(array) = try_from_bound_expression(between.input)? else {
                 return Ok(None);
             };
-            let Some(lower) = try_from_bound_expression(&between.lower)? else {
+            let Some(lower) = try_from_bound_expression(between.lower)? else {
                 return Ok(None);
             };
-            let Some(upper) = try_from_bound_expression(&between.upper)? else {
+            let Some(upper) = try_from_bound_expression(between.upper)? else {
                 return Ok(None);
             };
             Between.new_expr(
@@ -97,7 +99,7 @@ pub fn try_from_bound_expression(value: &duckdb::Expression) -> VortexResult<Opt
             | DUCKDB_VX_EXPR_TYPE::DUCKDB_VX_EXPR_TYPE_OPERATOR_IS_NOT_NULL => {
                 let children = operator.children().collect_vec();
                 assert_eq!(children.len(), 1);
-                let Some(child) = try_from_bound_expression(&children[0])? else {
+                let Some(child) = try_from_bound_expression(children[0])? else {
                     return Ok(None);
                 };
                 match operator.op {
@@ -113,7 +115,7 @@ pub fn try_from_bound_expression(value: &duckdb::Expression) -> VortexResult<Opt
                 // First child is element, rest form the list.
                 let children = operator.children().collect_vec();
                 assert!(children.len() >= 2);
-                let Some(element) = try_from_bound_expression(&children[0])? else {
+                let Some(element) = try_from_bound_expression(children[0])? else {
                     return Ok(None);
                 };
 
@@ -153,10 +155,10 @@ pub fn try_from_bound_expression(value: &duckdb::Expression) -> VortexResult<Opt
             DUCKDB_FUNCTION_NAME_CONTAINS => {
                 let children = func.children().collect_vec();
                 assert_eq!(children.len(), 2);
-                let Some(value) = try_from_bound_expression(&children[0])? else {
+                let Some(value) = try_from_bound_expression(children[0])? else {
                     return Ok(None);
                 };
-                let Some(pattern_lit) = like_pattern_str(&children[1])? else {
+                let Some(pattern_lit) = like_pattern_str(children[1])? else {
                     vortex_bail!("expected pattern to be bound string")
                 };
                 let pattern = lit(pattern_lit);
@@ -170,7 +172,7 @@ pub fn try_from_bound_expression(value: &duckdb::Expression) -> VortexResult<Opt
         duckdb::ExpressionClass::BoundConjunction(conj) => {
             let Some(children) = conj
                 .children()
-                .map(|c| try_from_bound_expression(&c))
+                .map(try_from_bound_expression)
                 .collect::<VortexResult<Option<Vec<_>>>>()?
             else {
                 return Ok(None);

vortex-duckdb/src/convert/scalar.rs

@@ -66,7 +66,8 @@ impl ToDuckDBScalar for Scalar {
     /// Struct and List scalars are not yet implemented and cause a panic.
     fn try_to_duckdb_scalar(&self) -> VortexResult<Value> {
         if self.is_null() {
-            return Ok(Value::null(&LogicalType::try_from(self.dtype())?));
+            let lt = LogicalType::try_from(self.dtype())?;
+            return Ok(Value::null(&lt));
         }
 
         match self.dtype() {
@@ -116,7 +117,8 @@ impl ToDuckDBScalar for DecimalScalar<'_> {
             .ok_or_else(|| vortex_err!("decimal scalar without decimal dtype"))?;
 
         let Some(decimal_value) = self.decimal_value() else {
-            return Ok(Value::null(&LogicalType::try_from(self.dtype())?));
+            let lt = LogicalType::try_from(self.dtype())?;
+            return Ok(Value::null(&lt));
         };
 
         let huge_value = match decimal_value {
@@ -231,14 +233,14 @@ impl TryFrom<Value> for Scalar {
     type Error = VortexError;
 
     fn try_from(value: Value) -> Result<Self, Self::Error> {
-        Scalar::try_from(value.as_ref())
+        Scalar::try_from(&*value)
     }
 }
 
-impl<'a> TryFrom<ValueRef<'a>> for Scalar {
+impl<'a> TryFrom<&'a ValueRef> for Scalar {
     type Error = VortexError;
 
-    fn try_from(value: ValueRef<'a>) -> Result<Self, Self::Error> {
+    fn try_from(value: &'a ValueRef) -> Result<Self, Self::Error> {
         use crate::duckdb::ExtractedValue;
         let dtype = DType::from_logical_type(value.logical_type(), Nullable)?;
         match value.extract() {

vortex-duckdb/src/convert/table_filter.rs

@@ -23,11 +23,11 @@ use vortex::expr::or_collect;
 use vortex::scalar::Scalar;
 
 use crate::cpp::DUCKDB_VX_EXPR_TYPE;
-use crate::duckdb::TableFilter;
 use crate::duckdb::TableFilterClass;
+use crate::duckdb::TableFilterRef;
 
 pub fn try_from_table_filter(
-    value: &TableFilter,
+    value: &TableFilterRef,
     col: &Expression,
     scope_dtype: &DType,
 ) -> VortexResult<Option<Expression>> {
@@ -40,7 +40,7 @@ pub fn try_from_table_filter(
         TableFilterClass::ConjunctionAnd(conj_and) => {
             let Some(children) = conj_and
                 .children()
-                .map(|child| try_from_table_filter(&child, col, scope_dtype))
+                .map(|child| try_from_table_filter(child, col, scope_dtype))
                 .try_collect::<_, Option<Vec<_>>, _>()?
             else {
                 return Ok(None);
@@ -52,7 +52,7 @@ pub fn try_from_table_filter(
         TableFilterClass::ConjunctionOr(disjuction_or) => {
             let Some(children) = disjuction_or
                 .children()
-                .map(|child| try_from_table_filter(&child, col, scope_dtype))
+                .map(|child| try_from_table_filter(child, col, scope_dtype))
                 .try_collect::<_, Option<Vec<_>>, _>()?
             else {
                 return Ok(None);
@@ -63,11 +63,11 @@ pub fn try_from_table_filter(
         TableFilterClass::IsNull => is_null(col.clone()),
         TableFilterClass::IsNotNull => not(is_null(col.clone())),
         TableFilterClass::StructExtract(name, child_filter) => {
-            return try_from_table_filter(&child_filter, &get_item(name, col.clone()), scope_dtype);
+            return try_from_table_filter(child_filter, &get_item(name, col.clone()), scope_dtype);
         }
         TableFilterClass::Optional(child) => {
             // Optional expressions are optional not yet supported.
-            return try_from_table_filter(&child, col, scope_dtype).or_else(|_err| {
+            return try_from_table_filter(child, col, scope_dtype).or_else(|_err| {
                 // Failed to convert the optional expression, but it's optional, so who cares?
                 Ok(None)
             });
@@ -107,7 +107,7 @@ pub fn try_from_table_filter(
                 op,
                 move || {
                     let value = data.latest()?;
-                    let scalar = Scalar::try_from(value.as_ref())
+                    let scalar = Scalar::try_from(&*value)
                         .vortex_expect("failed to convert dynamic filter value to scalar");
                     scalar.into_value()
                 },
@@ -116,7 +116,7 @@ pub fn try_from_table_filter(
                 col.clone(),
             )
         }
-        TableFilterClass::Expression(expr) => {
+        TableFilterClass::ExpressionRef(expr) => {
             // TODO(ngates): figure out which column ID DuckDB is using for the expression.
             vortex_bail!("expression table filter is not supported: {}", expr);
         }