Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Windows Support #3

Open
rupurt opened this issue Jul 14, 2023 · 13 comments
Open

Add Windows Support #3

rupurt opened this issue Jul 14, 2023 · 13 comments
Labels
enhancement New feature or request

Comments

@rupurt
Copy link
Owner

rupurt commented Jul 14, 2023

No description provided.

@rupurt rupurt changed the title Add Windows support Add Windows Support Jul 14, 2023
@rupurt rupurt added the enhancement New feature or request label Jul 14, 2023
@kmatt
Copy link

kmatt commented Jul 15, 2023

https://news.ycombinator.com/threads?id=kermatt#36729903

I don't have a Windows machine so wasn't testing against it.

I do, and happy to test.

@rupurt
Copy link
Owner Author

rupurt commented Jul 15, 2023

Thank you! I should be able to get pretty far with the Github actions Windows runner but would definitely appreciate someone testing it in real life on Windows.

The build process is a slight modification to the default DuckDB extension template which uses CMake.

@kmatt
Copy link

kmatt commented Jul 15, 2023

So far, using Nix to create local environment with dependencies on Debian 12, and cross-compiling with mingw32:

# Select POSIX from:
sudo update-alternatives --config i686-w64-mingw32-gcc
sudo update-alternatives --config i686-w64-mingw32-g++

A CMake toolchain file:

set(CMAKE_SYSTEM_NAME Windows)

set(CMAKE_C_COMPILER /usr/bin/i686-w64-mingw32-gcc)
set(CMAKE_CXX_COMPILER /usr/bin/i686-w64-mingw32-g++)

set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM NEVER)

set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY)
set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY)

Gets me halfway:

[ 55%] Building CXX object extension/odbc_scanner/CMakeFiles/odbc_scanner_extension.dir/src/odbc_scan.cpp.obj
In file included from /odbc-scanner-duckdb-extension/src/odbc_scan.cpp:1:
/odbc-scanner-duckdb-extension/src/include/odbc_scan.hpp: In static member function ‘static duckdb::unique_ptr<duckdb::GlobalTableFunctionState, std::default_delete<duckdb::GlobalTableFunctionState>, true> duckdb::OdbcScanFunction::OdbcScanInitGlobalState(duckdb::ClientContext&, duckdb::TableFunctionInitInput&)’:
/odbc-scanner-duckdb-extension/src/include/odbc_scan.hpp:339:12: error: could not convert ‘global_state’ from ‘unique_ptr<duckdb::OdbcScanGlobalState,default_delete<duckdb::OdbcScanGlobalState>,[...]>’ to ‘unique_ptr<duckdb::GlobalTableFunctionState,default_delete<duckdb::GlobalTableFunctionState>,[...]>’
  339 |     return global_state;
      |            ^~~~~~~~~~~~
      |            |
      |            unique_ptr<duckdb::OdbcScanGlobalState,default_delete<duckdb::OdbcScanGlobalState>,[...]>
gmake[3]: *** [extension/odbc_scanner/CMakeFiles/odbc_scanner_extension.dir/build.make:77: extension/odbc_scanner/CMakeFiles/odbc_scanner_extension.dir/src/odbc_scan.cpp.obj] Error 1

@kmatt
Copy link

kmatt commented Jul 15, 2023

diff --git a/src/include/odbc_scan.hpp b/src/include/odbc_scan.hpp
index a177d9b..2c4557d 100644
--- a/src/include/odbc_scan.hpp
+++ b/src/include/odbc_scan.hpp
@@ -335,7 +335,7 @@ public:
   static unique_ptr<GlobalTableFunctionState>
   OdbcScanInitGlobalState(ClientContext &context,
                           TableFunctionInitInput &input) {
-    auto global_state = make_uniq<OdbcScanGlobalState>();
+    auto global_state = make_uniq<GlobalTableFunctionState>();
     return global_state;
   }

gets me to [ 75%] Linking CXX shared library libduckdb.dll with multiple undefined references to look into next.

Assuming cross-compiling unixODBC is not a fool's errand

@rupurt
Copy link
Owner Author

rupurt commented Jul 15, 2023

Do the instructions for the nix shell work? https://github.com/rupurt/odbc-scanner-duckdb-extension#development

@rupurt
Copy link
Owner Author

rupurt commented Jul 16, 2023

So far, using Nix to create local environment with dependencies on Debian 12, and cross-compiling with mingw32:

# Select POSIX from:
sudo update-alternatives --config i686-w64-mingw32-gcc
sudo update-alternatives --config i686-w64-mingw32-g++

A CMake toolchain file:

set(CMAKE_SYSTEM_NAME Windows)

set(CMAKE_C_COMPILER /usr/bin/i686-w64-mingw32-gcc)
set(CMAKE_CXX_COMPILER /usr/bin/i686-w64-mingw32-g++)

set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM NEVER)

set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY)
set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY)

Gets me halfway:

[ 55%] Building CXX object extension/odbc_scanner/CMakeFiles/odbc_scanner_extension.dir/src/odbc_scan.cpp.obj
In file included from /odbc-scanner-duckdb-extension/src/odbc_scan.cpp:1:
/odbc-scanner-duckdb-extension/src/include/odbc_scan.hpp: In static member function ‘static duckdb::unique_ptr<duckdb::GlobalTableFunctionState, std::default_delete<duckdb::GlobalTableFunctionState>, true> duckdb::OdbcScanFunction::OdbcScanInitGlobalState(duckdb::ClientContext&, duckdb::TableFunctionInitInput&)’:
/odbc-scanner-duckdb-extension/src/include/odbc_scan.hpp:339:12: error: could not convert ‘global_state’ from ‘unique_ptr<duckdb::OdbcScanGlobalState,default_delete<duckdb::OdbcScanGlobalState>,[...]>’ to ‘unique_ptr<duckdb::GlobalTableFunctionState,default_delete<duckdb::GlobalTableFunctionState>,[...]>’
  339 |     return global_state;
      |            ^~~~~~~~~~~~
      |            |
      |            unique_ptr<duckdb::OdbcScanGlobalState,default_delete<duckdb::OdbcScanGlobalState>,[...]>
gmake[3]: *** [extension/odbc_scanner/CMakeFiles/odbc_scanner_extension.dir/build.make:77: extension/odbc_scanner/CMakeFiles/odbc_scanner_extension.dir/src/odbc_scan.cpp.obj] Error 1

I tried debugging this by compiling with gcc & g++ instead of clang. I ran into the same error on return of global_state and fixed it. Changes are on main if you want to give it another go.

@kmatt
Copy link

kmatt commented Jul 18, 2023

Do the instructions for the nix shell work? https://github.com/rupurt/odbc-scanner-duckdb-extension#development

They do (as far as I can tell, Nix is new for me), and with the changes in 9ad0f91 it builds, failing at link time which is not a surprise as linking probably needs to occur against Windows ODBC lib not unixODBC. Not sure yet how to tackle that as I'm cross-compiling on Debian.

@rupurt
Copy link
Owner Author

rupurt commented Jul 18, 2023

Sweet. Great to hear. I'll try to take a stab myself this week.

@hcc-donder
Copy link

Has there been any progress on this issue? This would be most welcome as it would allow access to many other databases (MS SQL Server for one).

@rupurt
Copy link
Owner Author

rupurt commented Feb 1, 2024

Sorry, I haven't had time to work on this extension in a while.

As I learned more about ODBC and the DuckDB API I've decided to totally re-architect this extension. I'm building a performance oriented ODBC client in zig. I will then link to that library for this extension to do the heavy lifting. I need performance oriented bulk ODBC -> Arrow format for other projects and bindings to DataFrame libraries like Polars.

@dmonder
Copy link

dmonder commented Feb 1, 2024

No apology necessary. I am just interested in this and noticed there was nothing since last July. I am definitely supportive of what you just described and I am happy to test it when it is ready to do. My main database is in SQL Server but I am trying to build a smaller, performance driven platform for data analytics and DuckDB is proving to be well suited for this. I will keep an eye on this for your future updates.

@rupurt
Copy link
Owner Author

rupurt commented Feb 24, 2024

I've got the Zig ODBC library to a decent spot where I'm making regular progress https://github.com/rupurt/zodbc. I have the build working on Linux x86_64 & Mac aarch64 + x86_64 through rosetta. It should be trivial to compile for Windows as it ships out of the box with ODBC headers and Zig cross compilation is fantastic.

I think it'll take me ~1 more month to get a robust initial version. Adherence to the ODBC concurrency model for maximum performance is why I decided to write it in the first place. That work is in ConnectionPool and I feel like I've got a decent worker abstraction where each worker executes in a separate kernel thread. Totally open to any improvements or suggestions.

To get it integrated with DuckDB my plan is to ship a python zodbc package on pip with support for the pyarrow.RecordBatchReader interface. DuckDB has great support Arrow so that should get a bunch of y'all unblocked. The next phase of the plan is to restart this extension from scratch and build the extension with Zig so that we have a tight integration between the DuckDB catalog and ODBC. I haven't been able to find any guides or examples on how to do that so if y'all have any links you can drop me for building DuckDB extensions with Zig that would be a huge help 🙏

@rupurt
Copy link
Owner Author

rupurt commented Mar 2, 2024

I've got Zig building DuckDB extensions working https://github.com/rupurt/duckdb-extension-template-zig. Starting work on the Zig ODBC DuckDB extension this weekend. Stay tuned!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants