feat(go/adbc/drivermgr): Implement SetSubstraitPlan in CGO wrapper and add DuckDB wrapper tests

[joellubi]

No description provided.

[zeroshade]

i'm honestly surprised this works without needing the suffix. That's really cool to know and can likely help us simplify some other CI

[lidavidm]

I think you should prefer just "duckdb", and the C++ driver manger will look for "libduckdb.so" and "duckdb.dll"

[zeroshade]

any reason to not just insert all three rows in one query?

INSERT INTO test_table (id, name) VALUES (1, 'test'), (2, 'test'), (3, 'tested') or does DuckDB not support that?

[joellubi]

I updated this to use a single statement as you suggested. I think I was just rushing lol.

[zeroshade]

sigh i should really improve this in the substrait library lol

[zeroshade]

Thanks so much for this, this is great both for adding the CI using duckdb and for SetSubtraitPlan being used.

It looks like the issue is purely that conda isn't finding libduckdb for windows which is odd....

[lidavidm]

It looks like the issue is purely that conda isn't finding libduckdb for windows which is odd....

because on windows, it's probably duckdb.dll; Windows doesn't use the lib prefix. Use just "duckdb" and on Windows it'll add .dll for you and otherwise it'll prepend lib and append .so/.dylib

[joellubi]

It looks like the issue is purely that conda isn't finding libduckdb for windows which is odd....

because on windows, it's probably duckdb.dll; Windows doesn't use the lib prefix. Use just "duckdb" and on Windows it'll add .dll for you and otherwise it'll prepend lib and append .so/.dylib

I tried just duckdb without lib and it works, which is nice. I think there's another issue though which is that there doesn't seem to even be a windows version hosted on conda-forge...

That said, I had been testing this with a nightly release of duckdb because some substrait issues were recently fixed but hadn't yet been included in a stable release. Testing against nightly might help surface issues/drift early. I suppose I could just download those in the github workflow and branch on the os / arch. Kind of complicated unfortunately. What do you think?

[lidavidm]

If someone knows the right things to do here (I outlined what I think is necessary but don't really have the time: need to add the mingw gcc on Windows + configure the Go builds here to use a different compiler than CC/CXX) conda-forge/arrow-adbc-split-feedstock#17

[lidavidm]

I think it's fine to download nightly duckdb/in the future we could even test against both in the workflow (just repeat the steps)

[joellubi]

@lidavidm @zeroshade

As I looked more closely I realized it's not only Windows failing but also Unix CGO (e.g. macos-14). Is there anything that needs to be done to properly locate the library after installing with conda? I didn't see libsqlite getting copied into another directory, so I wasn't sure why only duckdb wouldn't work the same way. Here's the test failure:

=== RUN   TestDriverMgrDuckDB
    wrapper_duckdb_test.go:54: 
        	Error Trace:	/Users/runner/work/arrow-adbc/arrow-adbc/go/adbc/drivermgr/wrapper_duckdb_test.go:54
        	            				/Users/runner/go/pkg/mod/github.com/stretchr/testify@v1.9.0/suite/suite.go:157
        	            				/Users/runner/work/arrow-adbc/arrow-adbc/go/adbc/drivermgr/wrapper_duckdb_test.go:162
        	Error:      	Received unexpected error:
        	            	Internal: [Driver Manager] [DriverManager] dlopen() failed: dlopen(duckdb, 0x0006): tried: '/duckdb' (no such file), '/Users/runner/local/lib/duckdb' (no such file), 'duckdb' (no such file), '/System/Volumes/Preboot/Cryptexes/OSduckdb' (no such file), '/usr/lib/duckdb' (no such file, not in dyld cache), 'duckdb' (no such file)
        	            	dlopen() failed: dlopen(libduckdb.dylib, 0x0006): tried: '/libduckdb.dylib' (no such file), '/Users/runner/local/lib/libduckdb.dylib' (no such file), 'libduckdb.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OSlibduckdb.dylib' (no such file), '/usr/lib/libduckdb.dylib' (no such file, not in dyld cache), 'libduckdb.dylib' (no such file)

[lidavidm]

I think we install into /Users/runner/local/lib/ and set DYLD_LIBRARY_PATH when we build the SQLite driver, so we'd need to make sure DuckDB ends up in the same path.

[lidavidm]

Hmm, but duckdb should be in the Conda env. Possibly add $CONDA_PREFIX/lib explicitly to DYLD_LIBRARY_PATH?

[joellubi]

Hmm, but duckdb should be in the Conda env. Possibly add $CONDA_PREFIX/lib explicitly to DYLD_LIBRARY_PATH?

That did the trick. I think previously we were using libraries that were either built as part of the workflow or included in the distribution itself (I think this may have been the case with sqlite).

[joellubi]

I looked into running the workflow against nightly duckdb...

While I think it would be nice to test against nightlies, they are not currently uploaded to any package manager. The files need to be downloaded and unzipped (link). This isn't too bad but the naming and paths aren't very consistent across platforms and architectures, so while we could get this working now it will very likely be a brittle workflow if/when the names/paths change in the future.

According to the release calendar, an official release including the required substrait fixes should be available within a week or so. In the interest of simplicity, I think we can just leave this PR as a draft and re-run the workflow in a week to confirm the tests pass.

[lidavidm]

Cool, thanks for investigating.

[joellubi]

I rebased and reran the workflows with the newer release of duckdb. The test failures resolved as expected. The issue remains however that there is unfortunately no libduckdb package published to conda-forge for windows...