diff --git a/vortex-duckdb/build.rs b/vortex-duckdb/build.rs index af44aceedeb..abec7f73f43 100644 --- a/vortex-duckdb/build.rs +++ b/vortex-duckdb/build.rs @@ -492,7 +492,6 @@ fn main() { .file("cpp/expr.cpp") .file("cpp/file_system.cpp") .file("cpp/logical_type.cpp") - .file("cpp/object_cache.cpp") .file("cpp/replacement_scan.cpp") .file("cpp/scalar_function.cpp") .file("cpp/table_filter.cpp") diff --git a/vortex-duckdb/cpp/client_context.cpp b/vortex-duckdb/cpp/client_context.cpp index c683229bc9e..9e9f954f52a 100644 --- a/vortex-duckdb/cpp/client_context.cpp +++ b/vortex-duckdb/cpp/client_context.cpp @@ -5,7 +5,6 @@ #include #include -#include extern "C" duckdb_client_context duckdb_vx_connection_get_client_context(duckdb_connection conn) { try { @@ -16,16 +15,6 @@ extern "C" duckdb_client_context duckdb_vx_connection_get_client_context(duckdb_ } } -extern "C" duckdb_vx_object_cache duckdb_client_context_get_object_cache(duckdb_client_context ffi_context) { - try { - auto *context = reinterpret_cast(ffi_context); - // This is okay because this is a ref to the object cache, this lives as long as the database. - return reinterpret_cast(&duckdb::ObjectCache::GetObjectCache(*context)); - } catch (...) { - return nullptr; - } -} - extern "C" duckdb_value duckdb_client_context_try_get_current_setting(duckdb_client_context context, const char *key) { if (!context || !key) { diff --git a/vortex-duckdb/cpp/object_cache.cpp b/vortex-duckdb/cpp/object_cache.cpp deleted file mode 100644 index ffa315385eb..00000000000 --- a/vortex-duckdb/cpp/object_cache.cpp +++ /dev/null @@ -1,48 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -// SPDX-FileCopyrightText: Copyright the Vortex contributors - -#include "duckdb_vx.h" -#include "duckdb/storage/object_cache.hpp" - -#include - -namespace vortex { - -// Wrapper class to hold opaque pointers in DuckDB's object cache -class OpaqueWrapper : public duckdb::ObjectCacheEntry { -public: - duckdb::unique_ptr ptr; - - explicit OpaqueWrapper(void *p, duckdb_vx_deleter_fn del) : ptr(p, del) { - } - ~OpaqueWrapper() override = default; - - duckdb::string GetObjectType() override { - return "vortex_opaque_wrapper"; - } - - // Static method required by DuckDB's object cache - static duckdb::string ObjectType() { - return "vortex_opaque_wrapper"; - } -}; - -} // namespace vortex - -extern "C" void duckdb_vx_object_cache_put(duckdb_vx_object_cache cache, - const char *key, - void *value, - duckdb_vx_deleter_fn deleter) { - auto object_cache = reinterpret_cast(cache); - auto wrapper = duckdb::make_shared_ptr(value, deleter); - object_cache->Put(std::string(key), wrapper); -} - -extern "C" void *duckdb_vx_object_cache_get(duckdb_vx_object_cache cache, const char *key) { - auto object_cache = reinterpret_cast(cache); - auto entry = object_cache->Get(std::string(key)); - if (!entry) { - return nullptr; - } - return entry->ptr.get(); -} diff --git a/vortex-duckdb/src/duckdb/client_context.rs b/vortex-duckdb/src/duckdb/client_context.rs index 13b7dd6bdf8..4cb646b82c5 100644 --- a/vortex-duckdb/src/duckdb/client_context.rs +++ b/vortex-duckdb/src/duckdb/client_context.rs @@ -3,11 +3,7 @@ use std::ffi::CStr; -use vortex::error::vortex_panic; - use crate::cpp; -use crate::duckdb::ObjectCache; -use crate::duckdb::ObjectCacheRef; use crate::duckdb::Value; use crate::lifetime_wrapper; @@ -36,17 +32,6 @@ impl ClientContextRef { unsafe { &*(self as *const Self) } } - /// Get the object cache for this client context. - pub fn object_cache(&self) -> &ObjectCacheRef { - unsafe { - let cache = cpp::duckdb_client_context_get_object_cache(self.as_ptr()); - if cache.is_null() { - vortex_panic!("Failed to get object cache from client context"); - } - ObjectCache::borrow(cache) - } - } - /// Try to get the current value of a configuration setting. /// Returns None if the setting doesn't exist. pub fn try_get_current_setting(&self, key: &CStr) -> Option { diff --git a/vortex-duckdb/src/duckdb/connection.rs b/vortex-duckdb/src/duckdb/connection.rs index 7eb566565b5..dc7c4b5b741 100644 --- a/vortex-duckdb/src/duckdb/connection.rs +++ b/vortex-duckdb/src/duckdb/connection.rs @@ -279,39 +279,4 @@ mod tests { data: String, value: i32, } - - #[test] - fn test_object_cache_put_get() { - let conn = test_connection().unwrap(); - let client_context = conn.client_context().unwrap(); - let cache = client_context.object_cache(); - - // Test with a simple struct - let test_entry = TestCacheEntry { - data: "hello world".to_string(), - value: 42, - }; - - // Store the entry in the cache - cache.put("test_key", test_entry); - - // Retrieve it back - let retrieved = cache.get::("test_key"); - assert!(retrieved.is_some()); - - let retrieved_entry = retrieved.unwrap(); - assert_eq!(retrieved_entry.data, "hello world"); - assert_eq!(retrieved_entry.value, 42); - } - - #[test] - fn test_object_cache_get_nonexistent() { - let conn = test_connection().unwrap(); - let client_context = conn.client_context().unwrap(); - let cache = client_context.object_cache(); - - // Try to get a non-existent key - let result = cache.get::("nonexistent_key"); - assert!(result.is_none()); - } } diff --git a/vortex-duckdb/src/duckdb/mod.rs b/vortex-duckdb/src/duckdb/mod.rs index ec82613fd85..50f1039fcd6 100644 --- a/vortex-duckdb/src/duckdb/mod.rs +++ b/vortex-duckdb/src/duckdb/mod.rs @@ -13,7 +13,6 @@ mod expr; mod file_system; mod logical_type; mod macro_; -mod object_cache; mod query_result; mod scalar_function; mod selection_vector; @@ -37,7 +36,6 @@ pub use ddb_string::*; pub use expr::*; pub use file_system::*; pub use logical_type::*; -pub use object_cache::*; pub use query_result::*; pub use scalar_function::*; pub use selection_vector::*; diff --git a/vortex-duckdb/src/duckdb/object_cache.rs b/vortex-duckdb/src/duckdb/object_cache.rs deleted file mode 100644 index 89bf6a013e1..00000000000 --- a/vortex-duckdb/src/duckdb/object_cache.rs +++ /dev/null @@ -1,77 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -// SPDX-FileCopyrightText: Copyright the Vortex contributors - -use std::ffi::CString; -use std::os::raw::c_void; - -use vortex::error::VortexExpect; -use vortex::error::vortex_err; -use vortex::error::vortex_panic; - -use crate::cpp; -use crate::lifetime_wrapper; - -/// Custom deleter function for Box allocated in Rust -unsafe extern "C-unwind" fn rust_box_deleter(ptr: *mut c_void) { - if !ptr.is_null() { - unsafe { - drop(Box::from_raw(ptr as *mut T)); - } - } -} - -lifetime_wrapper!(ObjectCache, cpp::duckdb_vx_object_cache, |_| { - vortex_panic!("ObjectCache is owned by the DatabaseInstance, not by Rust") -}); - -impl ObjectCacheRef { - /// Erases the lifetime of this reference, returning a `&'static ObjectCacheRef`. - /// - /// # Safety - /// - /// The caller must ensure that the underlying `ObjectCache` outlives all uses of the - /// returned reference. In practice, the `ObjectCache` is owned by the `DatabaseInstance` - /// and lives as long as the database, so this is safe as long as the database is kept alive. - pub unsafe fn erase_lifetime(&self) -> &'static Self { - unsafe { &*(self as *const Self) } - } - - /// Store an entry in the object cache with the given key. - /// The entry will be converted to an opaque pointer and stored. - /// Uses a proper deleter to ensure memory is freed when the cache entry is removed. - pub fn put(&self, key: &str, entry: T) -> *mut T { - let key_cstr = CString::new(key) - .map_err(|e| vortex_err!("invalid key: {}", e)) - .vortex_expect("object cache key should be valid C string"); - let opaque_ptr = Box::into_raw(Box::new(entry)); - - unsafe { - cpp::duckdb_vx_object_cache_put( - self.as_ptr(), - key_cstr.as_ptr(), - opaque_ptr.cast(), - Some(rust_box_deleter::), - ); - } - opaque_ptr - } - - /// Retrieve an entry from the object cache with the given key. - /// Returns None if the key is not found. - pub fn get(&self, key: &str) -> Option<&T> { - let key_cstr = CString::new(key) - .map_err(|e| vortex_err!("invalid key: {}", e)) - .vortex_expect("object cache key should be valid C string"); - - unsafe { - let opaque_ptr = cpp::duckdb_vx_object_cache_get(self.as_ptr(), key_cstr.as_ptr()); - (!opaque_ptr.is_null()) - .then_some(opaque_ptr.cast::().as_ref()) - .flatten() - } - } -} - -// SAFETY: Send + Sync since the cache has a mutex wrapper on the C++ side. -unsafe impl Send for ObjectCacheRef {} -unsafe impl Sync for ObjectCacheRef {} diff --git a/vortex-duckdb/src/e2e_test/object_cache_test.rs b/vortex-duckdb/src/e2e_test/object_cache_test.rs deleted file mode 100644 index 712d2a41209..00000000000 --- a/vortex-duckdb/src/e2e_test/object_cache_test.rs +++ /dev/null @@ -1,154 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -// SPDX-FileCopyrightText: Copyright the Vortex contributors - -//! Test table function that demonstrates object cache usage - -use std::ffi::CString; - -use vortex::error::VortexResult; -use vortex::error::vortex_err; - -use crate::cpp::DUCKDB_TYPE; -use crate::duckdb::BindInputRef; -use crate::duckdb::BindResultRef; -use crate::duckdb::ClientContextRef; -use crate::duckdb::DataChunkRef; -use crate::duckdb::LogicalType; -use crate::duckdb::TableFunction; -use crate::duckdb::TableInitInput; - -#[derive(Debug, Clone)] -pub struct TestTableFunction; - -#[derive(Debug, Clone)] -pub struct TestBindData { - cache_key: String, -} - -#[derive(Debug)] -pub struct TestGlobalState; - -#[derive(Debug)] -pub struct TestLocalState; - -#[derive(Debug, PartialEq)] -struct CachedData { - message: String, - count: i32, -} - -impl TableFunction for TestTableFunction { - type BindData = TestBindData; - type GlobalState = TestGlobalState; - type LocalState = TestLocalState; - - fn bind( - client_context: &ClientContextRef, - _input: &BindInputRef, - result: &mut BindResultRef, - ) -> VortexResult { - let logical_type = LogicalType::new(DUCKDB_TYPE::DUCKDB_TYPE_BIGINT); - result.add_result_column("test_value", &logical_type); - - let cache = client_context.object_cache(); - let cached_data = CachedData { - message: "Hello from bind phase cache!".to_string(), - count: 123, - }; - - // Store data in cache during bind - cache.put("bind_phase_data", cached_data); - - Ok(TestBindData { - cache_key: "test_table_function_data".to_string(), - }) - } - - fn table_scan_progress( - _client_context: &ClientContextRef, - _bind_data: &Self::BindData, - _global_state: &Self::GlobalState, - ) -> f64 { - 100.0 - } - - fn scan( - _client_context: &ClientContextRef, - _bind_data: &Self::BindData, - _local_state: &mut Self::LocalState, - _global_state: &Self::GlobalState, - chunk: &mut DataChunkRef, - ) -> VortexResult<()> { - chunk.set_len(0); - - Ok(()) - } - - fn init_global(input: &TableInitInput) -> VortexResult { - if let Ok(ctx) = input.client_context() { - let cached_data = CachedData { - message: "Hello from table function cache!".to_string(), - count: 42, - }; - let cache = ctx.object_cache(); - - cache.put(&input.bind_data().cache_key, cached_data); - } - - Ok(TestGlobalState) - } - - fn init_local( - _init: &TableInitInput, - _global: &Self::GlobalState, - ) -> VortexResult { - Ok(TestLocalState) - } - - fn partition_data( - _bind_data: &Self::BindData, - _global_init_data: &Self::GlobalState, - _local_init_data: &mut Self::LocalState, - ) -> VortexResult { - Ok(0) - } -} - -use crate::duckdb::Database; - -#[test] -fn test_table_function_with_object_cache() -> VortexResult<()> { - let db = Database::open_in_memory()?; - let conn = db.connect()?; - - // Register our test table function - let name = CString::new("test_cache_func").map_err(|e| vortex_err!("CString error: {}", e))?; - db.register_table_function::(&name)?; - - // Call the table function - this should store data in the cache during init_global - let _result = conn.query("SELECT * FROM test_cache_func()")?; - - // Try to verify that we can access the cached data from outside the table function - // This part is optional since we're not sure if the object cache access is working yet - let ctx = conn.client_context(); - if let Ok(ctx) = ctx { - let cache = ctx.object_cache(); - // Check data from bind phase - let bind_cached_data = cache.get::("bind_phase_data"); - if let Some(data) = bind_cached_data { - assert_eq!(data.message, "Hello from bind phase cache!"); - assert_eq!(data.count, 123); - println!("Successfully retrieved bind phase cached data!"); - } - - // Check data from init_global phase - let cached_data = cache.get::("test_table_function_data"); - if let Some(data) = cached_data { - assert_eq!(data.message, "Hello from table function cache!"); - assert_eq!(data.count, 42); - println!("Successfully retrieved init_global phase cached data!"); - } - } - - Ok(()) -}