Tighten docs and flag aliasing follow-up

Author: mlafeldt

crates/duckdb/src/core/data_chunk.rs

@@ -26,7 +26,22 @@ impl Drop for DataChunkHandle {
 }
 
 impl DataChunkHandle {
-    #[allow(dead_code)]
+    /// Wrap a `duckdb_data_chunk` pointer owned elsewhere (e.g. by a DuckDB
+    /// callback frame) without taking ownership.
+    ///
+    /// # Safety
+    ///
+    /// `ptr` must be a valid `duckdb_data_chunk` that remains allocated and
+    /// usable for the full lifetime of the returned handle. The caller must
+    /// ensure DuckDB does not destroy the chunk, free any column vectors
+    /// reachable from it, or concurrently mutate the same chunk/vector state
+    /// while this handle or any vectors derived from it are in use.
+    ///
+    /// TODO(#673 follow-up): this handle's `flat_vector` / `list_vector` /
+    /// `array_vector` / `struct_vector` accessors currently take `&self` and
+    /// return writable wrappers, so safe code can obtain two wrappers over
+    /// the same column and produce aliased `&mut [T]` slices.
+    #[allow(dead_code)] // used only when `vtab` / `vscalar` features are enabled
     pub(crate) unsafe fn new_unowned(ptr: duckdb_data_chunk) -> Self {
         Self { ptr, owned: false }
     }

crates/duckdb/src/core/vector.rs

@@ -17,8 +17,7 @@ use crate::ffi::{
 /// A flat vector borrowed from a [`DataChunkHandle`].
 ///
 /// The `'a` lifetime ties the vector to the chunk it was obtained from,
-/// preventing the chunk from being dropped while the vector is still alive
-/// (see <https://github.com/duckdb/duckdb-rs/issues/673>).
+/// preventing the chunk from being dropped while the vector is still alive.
 pub struct FlatVector<'a> {
     ptr: duckdb_vector,
     capacity: usize,
@@ -166,8 +165,7 @@ impl Inserter<&Vec<u8>> for FlatVector<'_> {
 /// A list vector borrowed from a [`DataChunkHandle`].
 ///
 /// The `'a` lifetime ties the vector to the chunk it was obtained from,
-/// preventing the chunk from being dropped while the vector is still alive
-/// (see <https://github.com/duckdb/duckdb-rs/issues/673>).
+/// preventing the chunk from being dropped while the vector is still alive.
 ///
 /// Regression test for the use-after-free in issue #673: a `ListVector`
 /// must not be allowed to outlive its parent `DataChunkHandle`.
@@ -279,11 +277,10 @@ impl<'a> ListVector<'a> {
     }
 }
 
-/// A array vector (fixed-size list) borrowed from a [`DataChunkHandle`].
+/// An array vector (fixed-size list) borrowed from a [`DataChunkHandle`].
 ///
 /// The `'a` lifetime ties the vector to the chunk it was obtained from,
-/// preventing the chunk from being dropped while the vector is still alive
-/// (see <https://github.com/duckdb/duckdb-rs/issues/673>).
+/// preventing the chunk from being dropped while the vector is still alive.
 pub struct ArrayVector<'a> {
     ptr: duckdb_vector,
     _phantom: PhantomData<&'a ()>,
@@ -338,8 +335,7 @@ impl<'a> ArrayVector<'a> {
 /// A struct vector borrowed from a [`DataChunkHandle`].
 ///
 /// The `'a` lifetime ties the vector to the chunk it was obtained from,
-/// preventing the chunk from being dropped while the vector is still alive
-/// (see <https://github.com/duckdb/duckdb-rs/issues/673>).
+/// preventing the chunk from being dropped while the vector is still alive.
 pub struct StructVector<'a> {
     ptr: duckdb_vector,
     _phantom: PhantomData<&'a ()>,

crates/duckdb/src/vscalar/mod.rs

@@ -27,14 +27,19 @@ pub trait VScalar: Sized {
     /// Shared across worker threads and invocations — must not be modified during execution.
     /// Must be `'static` as it is stored in DuckDB and may outlive the current stack frame.
     type State: Sized + Send + Sync + 'static;
-    /// The actual function
+    /// The actual function.
     ///
-    /// # Safety
-    ///
-    /// This function is unsafe because it:
+    /// DuckDB guarantees that `input` and `output` stay live for the
+    /// duration of this call. Implementations are expected to populate
+    /// `output` for rows `0..input.len()` and must not read or write beyond
+    /// that range.
     ///
-    /// - Dereferences multiple raw pointers (`func`).
+    /// # Safety
     ///
+    /// This method is `unsafe` because the `input` chunk and `output` vector
+    /// wrap raw DuckDB pointers (see [`DataChunkHandle::new_unowned`] and the
+    /// `WritableVector` impl on `duckdb_vector`). Callers must guarantee those
+    /// pointers are valid DuckDB-owned storage for the duration of the call.
     unsafe fn invoke(
         state: &Self::State,
         input: &mut DataChunkHandle,

crates/duckdb/src/vtab/arrow.rs

@@ -636,26 +636,36 @@ pub fn write_arrow_array_to_vector(
     Ok(())
 }
 
+// `duckdb_vector` is a `Copy` raw pointer typedef, so the `'_` lifetime on the
+// returned wrappers below is tied to `&mut self` (i.e. to the local variable
+// holding the pointer, not to the underlying C++ vector). This prevents reusing
+// the same `duckdb_vector` handle while a wrapper is in flight, but it does **not**
+// track the C++ vector's liveness. Callers must independently guarantee that the
+// underlying DuckDB vector outlives the returned wrapper.
+//
+// TODO(#673 follow-up): this `impl` is reachable from safe downstream code
+// because `WritableVector` is a public safe trait with a public impl for the
+// raw `duckdb_vector` pointer type. Marking the trait `unsafe` (or removing
+// the raw-pointer impl entirely) would move this obligation into the type
+// system instead of relying on caller discipline.
 impl WritableVector for duckdb_vector {
     fn array_vector(&mut self) -> ArrayVector<'_> {
-        // SAFETY: the returned vector borrows from `&mut self`, so it cannot
-        // outlive this `duckdb_vector` handle. The caller is responsible for
-        // ensuring the underlying C++ vector outlives this handle.
+        // SAFETY: see module note above `impl WritableVector for duckdb_vector`.
         unsafe { ArrayVector::from_raw(*self) }
     }
 
     fn flat_vector(&mut self) -> FlatVector<'_> {
-        // SAFETY: see `array_vector`.
+        // SAFETY: see module note above `impl WritableVector for duckdb_vector`.
         unsafe { FlatVector::from_raw(*self) }
     }
 
     fn list_vector(&mut self) -> ListVector<'_> {
-        // SAFETY: see `array_vector`.
+        // SAFETY: see module note above `impl WritableVector for duckdb_vector`.
         unsafe { ListVector::from_raw(*self) }
     }
 
     fn struct_vector(&mut self) -> StructVector<'_> {
-        // SAFETY: see `array_vector`.
+        // SAFETY: see module note above `impl WritableVector for duckdb_vector`.
         unsafe { StructVector::from_raw(*self) }
     }
 }