duckdb / arrow

Extension for DuckDB for functions that require the Apache Arrow dependency
MIT License
34 stars 11 forks source link

Add Windows build #18

Closed jzavala-gonzalez closed 1 year ago

jzavala-gonzalez commented 1 year ago

Added the Windows.yml based on the one from the main branch from a couple weeks ago. Updated CMakeLists with Arrow’s windows instructions so it compiles. See https://github.com/apache/arrow/blob/main/docs/source/developers/cpp/windows.rst the sections on “Windows dependency resolution issues” and “statically linking to arrow on windows”. CI ran fine on my fork but let me know if i should check anything else

paulrutter commented 1 year ago

Related bug: https://github.com/duckdb/duckdb/issues/8758

It would be highly appreciated if this gets merged and published.

paulrutter commented 1 year ago

@jzavala-gonzalez any update on this one? Whats needed to move this forward?

jzavala-gonzalez commented 1 year ago

Hi Paul. I'm not sure but I think a maintainer needs to approve the CI to run so as to check the workflow committed in this PR actually works. Also Duckdb 0.9.0 released after this PR was drafted so I'm hoping that doesnt require more changes

jzavala-gonzalez commented 1 year ago

@paulrutter I added an artifact upload to the CI in my fork and managed to extract an arrow extension for Windows. See and download the artifact included in this workflow run. Although it looks like it compiled it for duckdb v0.8.2-dev3190 so it can't be used with v0.9.0 ? If I figure out how to tweak the workflow so that it builds a different version, I'll let you know

EDIT: Managed to compile one for v0.9.0 at this workflow run. Download the artifact, start duckdb allowing unsigned extensions, then just "LOAD 'arrow.duckdb_extension'" relative path to the file. I tried rebuilding for v0.8.1 but some test failed and i'll check back later

image

paulrutter commented 1 year ago

Thanks! Will check it out and let you know @jzavala-gonzalez

paulrutter commented 1 year ago

When using v0.8.2-dev3190, i got DuckDB and the extension to install, but got strange results from queries (BigInt instead of int when doing a count). But the extension itself seemed to work and import the data though. It probably needs a bit more testing from my side.

Mause commented 1 year ago

Somehow i'm not able to install DuckDB 0.9.0 on my machine, there seem to be no prebuilt binaries. Building from source also failed, maybe i need to update my toolchain.

npm ERR! node-pre-gyp info it worked if it ends with ok
npm ERR! node-pre-gyp info using node-pre-gyp@1.0.11
npm ERR! node-pre-gyp info using node@18.16.1 | win32 | x64
npm ERR! node-pre-gyp info check checked for "C:\git\blueconic\etc\nodejs\node_modules\duckdb\lib\binding\duckdb.node" (not found)
npm ERR! node-pre-gyp http GET https://duckdb-node.s3.amazonaws.com/duckdb-v0.9.0-node-v108-win32-x64.tar.gz
npm ERR! node-pre-gyp ERR! install response status 403 Forbidden on https://duckdb-node.s3.amazonaws.com/duckdb-v0.9.0-node-v108-win32-x64.tar.gz
npm ERR! node-pre-gyp WARN Pre-built binaries not installable for duckdb@0.9.0 and node@18.16.1 (node-v108 ABI, unknown) (falling back to source compile with node-gyp)
npm ERR! node-pre-gyp WARN Hit error response status 403 Forbidden on https://duckdb-node.s3.amazonaws.com/duckdb-v0.9.0-node-v108-win32-x64.tar.gz

When using v0.8.2-dev3190, i got DuckDB and the extension to install, but got strange results from queries (BigInt instead of int when doing a count). But the extension itself seemed to work and import the data though.

Can you please raise this in the DuckDB repo instead of this unrelated issue?

paulrutter commented 1 year ago

@Mause certainly, i will update my comment to reflect only on the extension.

paulrutter commented 1 year ago

Somehow i'm not able to install DuckDB 0.9.0 on my machine, there seem to be no prebuilt binaries. Building from source also failed, maybe i need to update my toolchain.

npm ERR! node-pre-gyp info it worked if it ends with ok
npm ERR! node-pre-gyp info using node-pre-gyp@1.0.11
npm ERR! node-pre-gyp info using node@18.16.1 | win32 | x64
npm ERR! node-pre-gyp info check checked for "C:\git\blueconic\etc\nodejs\node_modules\duckdb\lib\binding\duckdb.node" (not found)
npm ERR! node-pre-gyp http GET https://duckdb-node.s3.amazonaws.com/duckdb-v0.9.0-node-v108-win32-x64.tar.gz
npm ERR! node-pre-gyp ERR! install response status 403 Forbidden on https://duckdb-node.s3.amazonaws.com/duckdb-v0.9.0-node-v108-win32-x64.tar.gz
npm ERR! node-pre-gyp WARN Pre-built binaries not installable for duckdb@0.9.0 and node@18.16.1 (node-v108 ABI, unknown) (falling back to source compile with node-gyp)
npm ERR! node-pre-gyp WARN Hit error response status 403 Forbidden on https://duckdb-node.s3.amazonaws.com/duckdb-v0.9.0-node-v108-win32-x64.tar.gz

When using v0.8.2-dev3190, i got DuckDB and the extension to install, but got strange results from queries (BigInt instead of int when doing a count). But the extension itself seemed to work and import the data though.

Can you please raise this in the DuckDB repo instead of this unrelated issue?

Already reported in https://github.com/duckdb/duckdb/issues/9113

jzavala-gonzalez commented 1 year ago

When using v0.8.2-dev3190, i got DuckDB and the extension to install, but got strange results from queries (BigInt instead of int when doing a count).

If you're using Node JS, I think that's normal when converting back to Javascript objects.

But the extension itself seemed to work and import the data though.

That's great to hear! To test v0.9.0 I just used the prebuilt binaries for the CLI executables in the installation page (until the issue you linked is resolved)

paulrutter commented 1 year ago

I will give it another shot next week using 0.9.0 and let you know the results 👍

jzavala-gonzalez commented 1 year ago

@samansmink is there anything I can help with to check whether this PR is ok to merge? It's been a while since it was opened and I don't know if it might get outdated in case the duckdb repo changes something. Thank you for your time!

samansmink commented 1 year ago

Hi @jzavala-gonzalez, sorry for the wait, this one slipped my attention. Thanks for the PR i think it looks good, I will rework the CI of the extension soonish so that change may be short-lived, but that's fine!

jzavala-gonzalez commented 1 year ago

Thank you!!!