Update to work with duckdb 1.1.1

[akumor]

This PR contains changes I made to get the duckdb-athena-extension to work with version 1.1.1 of duckdb. It includes changes to make use of extension-ci-tools and libduckdb_sys from duckdb-rs .

[dacort]

🤯 This is awesome, thank you! I was just thinking the other day I wanted to do this. I'll try to take a look soon. :)

[dacort]

You don't have to worry too much about the GH actions - those were mostly for building all the different artifacts and uploading to my own custom s3 bucket for hosting. With the new community extensions, looks like that's not relevant now.

[akumor]

Thank you for considering my pull request! I know it's the holidays so no rush. Rust/C/C++ are not my strongest languages so happy to get any feedback and make any changes.

Regarding the github actions, I don't mind trying to fix them (I don't think it's too much effort). If you don't think it is worth it or that it should be a different PR, that's fine too.

[dacort]

I really appreciate the contribution. :) And holidays happen to be the perfect time hehe.

Rust/C/C++ are not my strong suit either, but so far all the changes look good.

I'm going to do some tests locally myself and if those pass, I say this PR can get merged and a subsequent one can be opened for the GH actions.

[dacort]

@akumor Have you gotten this to build locally? I'm getting an error when running make.

Consolidate compiler generated dependencies of target parquet_extension
[ 66%] Built target parquet_extension
[ 66%] Linking CXX static library libduckdb_static.a
[ 66%] Built target duckdb_static
Consolidate compiler generated dependencies of target athena_loadable_extension
make[3]: *** No rule to make target `extension/athena/libduckdb_athena.a', needed by `extension/athena/athena.duckdb_extension'.  Stop.
make[2]: *** [extension/athena/CMakeFiles/athena_loadable_extension.dir/all] Error 2
make[1]: *** [all] Error 2
make: *** [release] Error 2

There's a good chance my toolchains are not quite configured/versioned properly, so likely on my end, but wanted to double-check.

[akumor]

yes, I was able to get it built locally before creating this PR 🤔 I am running Fedora linux so I did not test on a mac and I obviously did not get the github workflows running 🤔

[dacort]

kk, likely user error. :) Will do some more digging.

[dacort]

Finally got it to build on mac - the change mentioned in the other comment helped, but I also had to revert some changes to the athena_init and athen_version functions. I'm checking now to see if it still builds in Linux - unsure why those wouldn't work in mac. :\

edit: it does build in Linux with the reverts from below.

diff --git a/extension_config.cmake b/extension_config.cmake
index 46ee268..100287e 100644
--- a/extension_config.cmake
+++ b/extension_config.cmake
@@ -4,7 +4,7 @@
 duckdb_extension_load(athena
     SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}
     LOAD_TESTS
-    LINKED_LIBS "../../cargo/build/x86_64-unknown-linux-gnu/release/libduckdb_athena.a"
+    LINKED_LIBS "${CMAKE_CURRENT_BINARY_DIR}/libduckdb_athena.a"
 )
 
 # Any extra extensions that should be built
diff --git a/src/athena_extension.cpp b/src/athena_extension.cpp
index d90375f..9414d3f 100644
--- a/src/athena_extension.cpp
+++ b/src/athena_extension.cpp
@@ -10,7 +10,7 @@ namespace duckdb
     void AthenaExtension::Load(DuckDB &db)
     {
         // Call the Rust function to initialize the extension.
-        athena_init_rust(&db);
+        athena_init(&db);
     }
 
     std::string AthenaExtension::Name()
diff --git a/src/include/rust.h b/src/include/rust.h
index bb8eeb4..69927d5 100644
--- a/src/include/rust.h
+++ b/src/include/rust.h
@@ -1,6 +1,6 @@
 
 extern "C"
 {
-    void athena_init_rust(void *db);
-    void athena_version_rust(void);
+    void athena_init(void *db);
+    void athena_version(void);
 } // extern "C"
diff --git a/src/lib.rs b/src/lib.rs
index e11c55e..f49929f 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -20,7 +20,7 @@ lazy_static::lazy_static! {
 /// # Safety
 /// .
 #[no_mangle]
-pub unsafe extern "C" fn athena_init_rust(db: *mut _duckdb_database) {
+pub unsafe extern "C" fn athena_init(db: *mut _duckdb_database) {
     init(db).expect("init failed");
 }
 
@@ -34,6 +34,6 @@ unsafe fn init(db: *mut _duckdb_database) -> Result<()> {
 
 /// Version hook for DuckDB, indicates which version of DuckDB this extension was compiled against
 #[no_mangle]
-pub extern "C" fn athena_version_rust() -> *const c_char {
+pub extern "C" fn athena_version() -> *const c_char {
     unsafe { duckdb_library_version() }
 }

[dacort]

OK, I think with these changes it should build across linux/mac - haven't tested windows yet, but that's ok.

Once those change are made, I can merge in and then remove the existing GH actions as I'm pretty sure everything in those is taken care of by the new extension template.

In the (near) future, it'd be good to either just change this to a c++ extension or use the upcoming C extension API if rust is still desired.

[akumor]

Once those change are made, I can merge in and then remove the existing GH actions as I'm pretty sure everything in those is taken care of by the new extension template.

I think I have addressed all of the comments on the PR now. Let me know if you would like to see any other changes.

In the (near) future, it'd be good to either just change this to a c++ extension or use the upcoming duckdb/duckdb#12682 if rust is still desired.

I agree! My personal bias would be to keep rust because of my own personal interests and because I think it might make it easier to contribute. Regardless of direction, I would be willing to take a stab at this unless you feel particularly inclined to take it on. I am particularly interested in adding support for different databases as described in this issue too.

[dacort]

I would be willing to take a stab at this

Feel free! You'll definitely get to it before I will - I don't know much about the upcoming extension API.

Will get this merged in.