From fad2b8eab45c34f0cde7f3c55150453fb9e0ddb3 Mon Sep 17 00:00:00 2001 From: Calvin Cheng Date: Sun, 23 Aug 2026 10:01:16 +0800 Subject: [PATCH 01/10] Make the Windows struct-by-value ABI assumption explicit and checked --- internal/duckdb/abi_windows_amd64.go | 11 +++++++++++ internal/duckdb/abi_windows_arm64.go | 8 ++++++++ internal/duckdb/abi_windows_unsupported.go | 15 +++++++++++++++ internal/duckdb/register_result_windows.go | 16 +++++++++++++++- 4 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 internal/duckdb/abi_windows_amd64.go create mode 100644 internal/duckdb/abi_windows_arm64.go create mode 100644 internal/duckdb/abi_windows_unsupported.go diff --git a/internal/duckdb/abi_windows_amd64.go b/internal/duckdb/abi_windows_amd64.go new file mode 100644 index 0000000..d67970e --- /dev/null +++ b/internal/duckdb/abi_windows_amd64.go @@ -0,0 +1,11 @@ +package duckdb + +// windowsIndirectAggregateThreshold is the largest aggregate the Win64 calling +// convention passes in a register. An aggregate of 1, 2, 4 or 8 bytes is passed +// by value in a register; every other size, including anything larger than 8 +// bytes, is passed as a hidden pointer to a caller-allocated copy. +// +// That is what makes the workaround in register_result_windows.go sound: at the +// ABI level, a function declared to take duckdb_result by value actually +// receives a duckdb_result *. +const windowsIndirectAggregateThreshold = 8 diff --git a/internal/duckdb/abi_windows_arm64.go b/internal/duckdb/abi_windows_arm64.go new file mode 100644 index 0000000..56b1ce4 --- /dev/null +++ b/internal/duckdb/abi_windows_arm64.go @@ -0,0 +1,8 @@ +package duckdb + +// windowsIndirectAggregateThreshold is the largest composite AAPCS64 passes in +// registers. A composite of 16 bytes or less is passed in up to two general +// registers; anything larger is passed indirectly, as a pointer to a +// caller-allocated copy, which is the same property the amd64 workaround relies +// on. Windows on ARM64 follows AAPCS64 for this case. +const windowsIndirectAggregateThreshold = 16 diff --git a/internal/duckdb/abi_windows_unsupported.go b/internal/duckdb/abi_windows_unsupported.go new file mode 100644 index 0000000..3d4e175 --- /dev/null +++ b/internal/duckdb/abi_windows_unsupported.go @@ -0,0 +1,15 @@ +//go:build windows && !amd64 && !arm64 + +package duckdb + +// go-pduckdb supports windows/amd64 and windows/arm64 only. +// +// The struct-by-value workaround in register_result_windows.go depends on the +// calling convention passing a large aggregate as a hidden pointer. The 32-bit +// Windows conventions push such aggregates onto the stack by value instead, so +// the workaround would read the wrong memory rather than fail. DuckDB also +// publishes no 32-bit Windows library, so there is nothing to load. +// +// Failing to build is deliberate: the alternative is a binary that corrupts +// results at run time. +type windowsArchitectureNotSupported [-1]int diff --git a/internal/duckdb/register_result_windows.go b/internal/duckdb/register_result_windows.go index df4bab6..8531992 100644 --- a/internal/duckdb/register_result_windows.go +++ b/internal/duckdb/register_result_windows.go @@ -1,6 +1,20 @@ package duckdb -import "github.com/ebitengine/purego" +import ( + "unsafe" + + "github.com/ebitengine/purego" +) + +// duckdb_result must be large enough that the calling convention passes it +// indirectly; the registrations below depend on it. If the mirror struct is +// ever reduced past that threshold, registering these functions with a pointer +// argument would silently read the wrong memory -- so the build fails instead. +// +// This can only check the Go mirror. The C API exposes no way to ask for the +// real size of duckdb_result, so a mirror that has drifted from the C struct is +// caught by the tests, not by this. +const _ = uint(unsafe.Sizeof(DuckDBResultRaw{}) - windowsIndirectAggregateThreshold - 1) // registerResultByValueFuncs registers the DuckDB functions that take duckdb_result // by value. purego does not support struct-by-value arguments on Windows. However, the From ac83d94d3a30f988d0581546f9d02b1814b013e6 Mon Sep 17 00:00:00 2001 From: Calvin Cheng Date: Sun, 23 Aug 2026 10:01:16 +0800 Subject: [PATCH 02/10] Load an absolute DLL path with LoadLibraryEx; search the executable's directory --- internal/duckdb/library.go | 41 +++++++++++++++------ internal/duckdb/library_windows.go | 59 +++++++++++++++++++++++++++--- 2 files changed, 84 insertions(+), 16 deletions(-) diff --git a/internal/duckdb/library.go b/internal/duckdb/library.go index faf9180..446b784 100644 --- a/internal/duckdb/library.go +++ b/internal/duckdb/library.go @@ -51,22 +51,41 @@ func getLibraryPaths() []string { case "linux": locations = getLinuxLibraryPaths() case "windows": - // Windows standard locations - prioritize current directory as it's the most likely location - currentDir, err := os.Getwd() - if err == nil { - locations = append(locations, filepath.Join(currentDir, "duckdb.dll")) - } - // Then add other standard locations - locations = append(locations, - "duckdb.dll", // Current directory (relative path) - filepath.Join(os.Getenv("ProgramFiles"), "DuckDB", "duckdb.dll"), - filepath.Join(os.Getenv("ProgramFiles(x86)"), "DuckDB", "duckdb.dll"), - ) + locations = getWindowsLibraryPaths() } return locations } +// getWindowsLibraryPaths returns a list of paths to search for the DuckDB DLL +// on Windows. +// +// The executable's own directory comes first: it is the one location a shipped +// binary can rely on, and unlike the working directory it does not change with +// however the program happened to be launched. The working directory is still +// searched, right after, because that is where a DLL dropped next to a script +// or unzipped for a quick run ends up. +func getWindowsLibraryPaths() []string { + locations := []string{} + + if exe, err := os.Executable(); err == nil { + locations = append(locations, filepath.Join(filepath.Dir(exe), "duckdb.dll")) + } + if currentDir, err := os.Getwd(); err == nil { + locations = append(locations, filepath.Join(currentDir, "duckdb.dll")) + } + + locations = append(locations, + filepath.Join(os.Getenv("ProgramFiles"), "DuckDB", "duckdb.dll"), + filepath.Join(os.Getenv("ProgramFiles(x86)"), "DuckDB", "duckdb.dll"), + // A bare name last, so the standard search -- PATH included -- is the + // fallback rather than the first thing tried. + "duckdb.dll", + ) + + return locations +} + // getMacOSLibraryPaths returns a list of paths to search for the DuckDB library on macOS func getMacOSLibraryPaths() []string { locations := []string{} diff --git a/internal/duckdb/library_windows.go b/internal/duckdb/library_windows.go index aa99a2b..0befe20 100644 --- a/internal/duckdb/library_windows.go +++ b/internal/duckdb/library_windows.go @@ -1,10 +1,59 @@ package duckdb -import "syscall" +import ( + "path/filepath" + "syscall" + "unsafe" +) +// Loader flags for LoadLibraryExW. Declared here rather than taken from +// golang.org/x/sys so the package keeps no external dependency (#270). +const ( + loadLibrarySearchDLLLoadDir = 0x00000100 + loadLibrarySearchDefaultDirs = 0x00001000 +) + +var ( + kernel32 = syscall.NewLazyDLL("kernel32.dll") + loadLibraryExW = kernel32.NewProc("LoadLibraryExW") +) + +// openLibrary loads the DuckDB DLL. +// +// An absolute path goes through LoadLibraryExW with the search restricted to +// the DLL's own directory and the process's default directories. That is both +// safer -- the legacy search reaches the current directory and PATH, which is +// how DLL planting works -- and more correct, because it lets a DuckDB DLL find +// dependencies sitting beside it rather than only ones already on PATH. +// +// A bare name still goes through the standard search, PATH included. Users and +// CI rely on putting the DLL's directory on PATH, and quietly removing that +// would break them; the caller decides by passing a path or a name. func openLibrary(name string) (uintptr, error) { - // Use [syscall.LoadLibrary] here to avoid external dependencies (#270). - // For actual use cases, [golang.org/x/sys/windows.NewLazySystemDLL] is recommended. - handle, err := syscall.LoadLibrary(name) - return uintptr(handle), err + if !filepath.IsAbs(name) { + // Use [syscall.LoadLibrary] here to avoid external dependencies (#270). + // For actual use cases, [golang.org/x/sys/windows.NewLazySystemDLL] is recommended. + handle, err := syscall.LoadLibrary(name) + return uintptr(handle), err + } + + wide, err := syscall.UTF16PtrFromString(name) + if err != nil { + return 0, err + } + if err := loadLibraryExW.Find(); err != nil { + handle, lerr := syscall.LoadLibrary(name) + return uintptr(handle), lerr + } + // wide stays reachable from this frame for the whole call, so the + // conversion cannot outlive the buffer. + handle, _, err := loadLibraryExW.Call( + uintptr(unsafe.Pointer(wide)), + 0, + loadLibrarySearchDLLLoadDir|loadLibrarySearchDefaultDirs, + ) + if handle == 0 { + return 0, err + } + return handle, nil } From 12ed9c713a4e1fdf4f3a60dd5e2e33bc3ce341c1 Mon Sep 17 00:00:00 2001 From: Calvin Cheng Date: Sun, 23 Aug 2026 10:24:33 +0800 Subject: [PATCH 03/10] CI: cover linux/arm64, macos/amd64 and windows/arm64 --- .github/workflows/go.yml | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index e995076..bb433d8 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -10,13 +10,35 @@ on: jobs: unit-tests: + name: ${{ matrix.name }} strategy: fail-fast: false matrix: - os: [ubuntu-latest, macos-latest, windows-latest] + include: + - name: linux/amd64 + os: ubuntu-latest + asset: libduckdb-linux-amd64.zip + - name: linux/arm64 + os: ubuntu-24.04-arm + asset: libduckdb-linux-arm64.zip + - name: macos/arm64 + os: macos-latest + asset: libduckdb-osx-universal.zip + - name: macos/amd64 + os: macos-15-intel + asset: libduckdb-osx-universal.zip + - name: windows/amd64 + os: windows-latest + asset: libduckdb-windows-amd64.zip + - name: windows/arm64 + os: windows-11-arm + asset: libduckdb-windows-arm64.zip runs-on: ${{ matrix.os }} + env: + DUCKDB_VERSION: v1.5.4 + steps: - uses: actions/checkout@v4 @@ -29,7 +51,7 @@ jobs: - name: Install DuckDB library (Linux) if: runner.os == 'Linux' run: | - curl -sSL https://github.com/duckdb/duckdb/releases/download/v1.5.4/libduckdb-linux-amd64.zip -o archive.zip + curl -sSL https://github.com/duckdb/duckdb/releases/download/${DUCKDB_VERSION}/${{ matrix.asset }} -o archive.zip sudo unzip -j archive.zip libduckdb.so -d /usr/local/lib sudo ldconfig rm archive.zip @@ -37,7 +59,7 @@ jobs: - name: Install DuckDB library (macOS) if: runner.os == 'macOS' run: | - curl -sSL https://github.com/duckdb/duckdb/releases/download/v1.5.4/libduckdb-osx-universal.zip -o archive.zip + curl -sSL https://github.com/duckdb/duckdb/releases/download/${DUCKDB_VERSION}/${{ matrix.asset }} -o archive.zip sudo unzip -j archive.zip libduckdb.dylib -d /usr/local/lib rm archive.zip @@ -45,7 +67,7 @@ jobs: if: runner.os == 'Windows' shell: pwsh run: | - Invoke-WebRequest -Uri https://github.com/duckdb/duckdb/releases/download/v1.5.4/libduckdb-windows-amd64.zip -OutFile archive.zip + Invoke-WebRequest -Uri "https://github.com/duckdb/duckdb/releases/download/$env:DUCKDB_VERSION/${{ matrix.asset }}" -OutFile archive.zip Expand-Archive -Path archive.zip -DestinationPath "$env:RUNNER_TEMP\duckdb" -Force # go test runs each package with its own directory as the working directory, # so the DLL must be resolvable via the standard DLL search path (PATH), not cwd. From bbfd9ff57ec31f664bb9102ae55cd6d6f87f6f79 Mon Sep 17 00:00:00 2001 From: Calvin Cheng Date: Sun, 23 Aug 2026 10:24:33 +0800 Subject: [PATCH 04/10] CI: run the integration tests on musl too --- .github/workflows/integ.yml | 16 +++++++++--- internal/integ/Dockerfile | 4 +-- internal/integ/Dockerfile.musl | 45 ++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 5 deletions(-) create mode 100644 internal/integ/Dockerfile.musl diff --git a/.github/workflows/integ.yml b/.github/workflows/integ.yml index c400577..bda276a 100644 --- a/.github/workflows/integ.yml +++ b/.github/workflows/integ.yml @@ -9,7 +9,17 @@ on: - main jobs: - unit-tests: + integration-tests: + name: ${{ matrix.name }} + strategy: + fail-fast: false + matrix: + include: + - name: glibc + dockerfile: ./internal/integ/Dockerfile + - name: musl + dockerfile: ./internal/integ/Dockerfile.musl + runs-on: ubuntu-latest steps: @@ -19,7 +29,7 @@ jobs: uses: docker/setup-buildx-action@v3 - name: Build docker image - run: docker build -t go-pduckdb/integ -f ./internal/integ/Dockerfile . + run: docker build -t go-pduckdb/integ-${{ matrix.name }} -f ${{ matrix.dockerfile }} . - name: Run integration tests - run: docker run --rm go-pduckdb/integ + run: docker run --rm go-pduckdb/integ-${{ matrix.name }} diff --git a/internal/integ/Dockerfile b/internal/integ/Dockerfile index 028f30e..8dd9e8e 100644 --- a/internal/integ/Dockerfile +++ b/internal/integ/Dockerfile @@ -13,8 +13,8 @@ COPY . . # Set GOARCH for the build ENV GOARCH=${GOARCH} -# Download and install DuckDB library before running tests -# NOTE: Official DuckDB builds require glibc. Not musl. +# Download and install DuckDB library before running tests. +# This image is the glibc build; see Dockerfile.musl for the musl one. RUN curl -sSfL https://github.com/duckdb/duckdb/releases/download/v1.5.4/libduckdb-linux-${LIBARCH}.zip -o archive.zip \ && unzip -j archive.zip libduckdb.so \ && rm archive.zip \ diff --git a/internal/integ/Dockerfile.musl b/internal/integ/Dockerfile.musl new file mode 100644 index 0000000..bcc0d55 --- /dev/null +++ b/internal/integ/Dockerfile.musl @@ -0,0 +1,45 @@ +# The musl build of the integration tests. +# +# DuckDB publishes a musl shared library alongside the glibc one, so a +# statically linked Go binary and a musl-based image -- Alpine, or a distroless +# static base -- are a supported combination. Proving it here keeps that true: +# the glibc image would happily pass while an Alpine deployment failed to load +# the library at all. +FROM golang:1.24-alpine AS builder + +ARG GOARCH=amd64 +ARG LIBARCH=amd64 +ARG DUCKDB_VERSION=v1.5.4 + +# libduckdb.so links against libstdc++, which an Alpine base does not carry. +RUN apk add --no-cache bash curl unzip libstdc++ + +WORKDIR /app + +COPY . . + +ENV GOARCH=${GOARCH} + +RUN curl -sSfL https://github.com/duckdb/duckdb/releases/download/${DUCKDB_VERSION}/libduckdb-linux-${LIBARCH}-musl.zip -o archive.zip \ + && unzip -j archive.zip libduckdb.so \ + && rm archive.zip \ + && cp libduckdb.so /usr/local/lib/ + +RUN go get . + +RUN go test -v ./... + +RUN ./internal/integ/build.sh + +FROM alpine:3.21 AS tester + +RUN apk add --no-cache bash libstdc++ + +WORKDIR /app +COPY --from=builder /app/out ./out +COPY --from=builder /app/internal/integ/run.sh . +COPY --from=builder /usr/local/lib/libduckdb.so /usr/local/lib/ + +RUN chmod +x /app/run.sh + +ENTRYPOINT [ "/app/run.sh" ] From 14d5e2b97d904512b8f676f84eba54ba03d86d91 Mon Sep 17 00:00:00 2001 From: Calvin Cheng Date: Sun, 23 Aug 2026 10:24:33 +0800 Subject: [PATCH 05/10] Document the platform matrix as it now stands --- docs/COMPATIBILITY.md | 66 +++++++++++++++++++++++++++++++------------ 1 file changed, 48 insertions(+), 18 deletions(-) diff --git a/docs/COMPATIBILITY.md b/docs/COMPATIBILITY.md index 160a95f..621424c 100644 --- a/docs/COMPATIBILITY.md +++ b/docs/COMPATIBILITY.md @@ -8,9 +8,11 @@ Legend: ✅ supported · ⚠️ supported with caveats · ❌ not supported (yet | Platform | Status | CI-tested | Notes | |---|---|---|---| -| Linux (amd64/arm64) | ✅ | amd64 | Requires purego ≥ v0.10.0 (struct-by-value support) | -| macOS (amd64/arm64) | ✅ | arm64 | | -| Windows (amd64) | ⚠️ | amd64 | Works via an ABI workaround — see [Windows workaround](#windows-workaround) | +| Linux (amd64/arm64) | ✅ | both | Requires purego ≥ v0.10.0 (struct-by-value support) | +| Linux musl (amd64/arm64) | ✅ | amd64 | DuckDB publishes a musl build; the image must also carry `libstdc++` | +| macOS (amd64/arm64) | ✅ | both | | +| Windows (amd64/arm64) | ✅ | both | Works via an ABI workaround — see [Windows workaround](#windows-workaround) | +| Windows (386/arm) | ❌ | — | Refused at build time; see below | | FreeBSD / NetBSD | ⚠️ | no | Compiles (covered by build tags), untested | ### Windows workaround @@ -21,21 +23,48 @@ API functions take a `duckdb_result` struct by value: - `duckdb_fetch_chunk` - `duckdb_result_return_type` -The driver works around this by relying on a Win64 calling-convention detail: -aggregates larger than 8 bytes are passed as a **hidden pointer** to a -caller-allocated copy. Since `duckdb_result` is 48 bytes, at the ABI level these -functions actually receive a `duckdb_result *`. On Windows the driver registers -them with an explicit pointer argument and wraps them to preserve the by-value -signature used on other platforms (see -`internal/duckdb/register_result_windows.go`). - -This is an ABI-level workaround, not an officially supported purego feature. It -is exercised in CI on `windows-latest` (amd64) on every push, but be aware of it -if you hit Windows-specific crashes around result fetching. - -DLL discovery on Windows: `DUCKDB_LIBRARY_PATH`, then `duckdb.dll` in the -current directory, `%ProgramFiles%\DuckDB\`, `%ProgramFiles(x86)%\DuckDB\`, and -finally the standard `LoadLibrary` search path (`PATH`). +The driver works around this by relying on a property both 64-bit Windows +calling conventions share: an aggregate too large for registers is passed as a +**hidden pointer** to a caller-allocated copy. On amd64 (Win64) that means +anything other than 1, 2, 4 or 8 bytes; on arm64 (AAPCS64) anything over 16 +bytes. `duckdb_result` is 48 bytes, so at the ABI level these functions actually +receive a `duckdb_result *`. The driver registers them with an explicit pointer +argument and wraps them to preserve the by-value signature used on other +platforms (see `internal/duckdb/register_result_windows.go`). + +This is an ABI-level workaround, not an officially supported purego feature, so +the assumption it rests on is checked rather than trusted: + +- The per-architecture threshold is stated explicitly in + `internal/duckdb/abi_windows_amd64.go` and `abi_windows_arm64.go`. +- A **compile-time assertion** fails the build if `duckdb_result` is ever + reduced to a size the convention would pass in registers, because registering + a by-value function with a pointer argument would then read the wrong memory + silently. Note this can only check the Go mirror of the struct: the C API + offers no way to ask for the real size, so a mirror that has drifted from the + C definition is caught by the tests, not by the assertion. +- 32-bit Windows (`386`, `arm`) **fails to build**. Those conventions push large + aggregates onto the stack by value, so the workaround would be wrong rather + than merely unsupported — and DuckDB publishes no 32-bit Windows library. + +Both 64-bit Windows targets run the unit tests in CI on every push. + +### DLL discovery on Windows + +In order: `DUCKDB_LIBRARY_PATH`, `duckdb.dll` beside the **executable**, then in +the current directory, then `%ProgramFiles%\DuckDB\` and +`%ProgramFiles(x86)%\DuckDB\`, and finally the bare name `duckdb.dll` through +the standard `LoadLibrary` search path (`PATH`). + +The executable's own directory comes before the working directory because it is +the one location a shipped binary can rely on; the working directory changes +with however the program was launched. + +An **absolute** path is loaded with `LoadLibraryExW` restricted to the DLL's own +directory and the process's default directories. That is safer than the legacy +search, which reaches the current directory and `PATH`, and it also lets a +DuckDB DLL resolve dependencies sitting beside it. A bare name still uses the +standard search so that putting a directory on `PATH` keeps working. ## database/sql driver interface @@ -102,3 +131,4 @@ Result values are decoded through DuckDB's data-chunk / vector API. | Go | ≥ 1.24 | | | purego | ≥ v0.10.0 | v0.10.0 added struct-by-value arguments on Linux, needed for `duckdb_fetch_chunk` | | DuckDB shared library | v1.5.x | CI tests against v1.5.4; nearby versions generally work since the C API is stable | +| `libstdc++` | — | Only on musl images (Alpine, distroless static): `libduckdb.so` links against it and a musl base does not carry it | From 60cfe69a730956bd864a42b4361d350622448b60 Mon Sep 17 00:00:00 2001 From: Calvin Cheng Date: Sun, 23 Aug 2026 10:29:04 +0800 Subject: [PATCH 06/10] Rename the module for this fork --- docs/COMPATIBILITY.md | 2 +- driver.go | 2 +- driver_test.go | 2 +- example/columntypes/main.go | 2 +- example/databasesql/main.go | 2 +- example/databasesql2/main.go | 2 +- example/enhancedtypes/main.go | 2 +- example/json/main.go | 2 +- example/multistatement/main.go | 2 +- example/simple/main.go | 2 +- go.mod | 2 +- internal/duckdb/statement.go | 2 +- pduckdb.go | 2 +- pduckdb_test.go | 2 +- types_test.go | 2 +- 15 files changed, 15 insertions(+), 15 deletions(-) diff --git a/docs/COMPATIBILITY.md b/docs/COMPATIBILITY.md index 621424c..0f31ae5 100644 --- a/docs/COMPATIBILITY.md +++ b/docs/COMPATIBILITY.md @@ -80,7 +80,7 @@ standard search so that putting a directory on `PATH` keeps working. | `ColumnTypes()`: database type name | ✅ | `RowsColumnTypeDatabaseTypeName` (e.g. `INTEGER`, `VARCHAR`) | | `ColumnTypes()`: nullable | ✅ | `RowsColumnTypeNullable` | | `ColumnTypes()`: precision & scale | ✅ | `RowsColumnTypePrecisionScale` | -| `Result.RowsAffected()` | ⚠️ | Broken for parameterized `Exec` — see [#23](https://github.com/fpt/go-pduckdb/issues/23) | +| `Result.RowsAffected()` | ⚠️ | Broken for parameterized `Exec` — see [#23](https://github.com/calvinchengx/go-pduckdb/issues/23) | | `Result.LastInsertId()` | ❌ | Not supported by DuckDB; returns an error | | Named parameters | ❌ | Positional only | diff --git a/driver.go b/driver.go index 65600f7..82e971c 100644 --- a/driver.go +++ b/driver.go @@ -9,7 +9,7 @@ import ( "github.com/pkg/errors" - "github.com/fpt/go-pduckdb/internal/duckdb" + "github.com/calvinchengx/go-pduckdb/internal/duckdb" ) // Initialize and register the driver diff --git a/driver_test.go b/driver_test.go index a018ead..ecb76ea 100644 --- a/driver_test.go +++ b/driver_test.go @@ -5,7 +5,7 @@ import ( "testing" "time" - "github.com/fpt/go-pduckdb/internal/duckdb" + "github.com/calvinchengx/go-pduckdb/internal/duckdb" "github.com/stretchr/testify/assert" ) diff --git a/example/columntypes/main.go b/example/columntypes/main.go index 387fc5c..e6ee9c8 100644 --- a/example/columntypes/main.go +++ b/example/columntypes/main.go @@ -6,7 +6,7 @@ import ( "log" "reflect" - _ "github.com/fpt/go-pduckdb" // Import for driver registration + _ "github.com/calvinchengx/go-pduckdb" // Import for driver registration ) func main() { diff --git a/example/databasesql/main.go b/example/databasesql/main.go index 6bf3c88..a462945 100644 --- a/example/databasesql/main.go +++ b/example/databasesql/main.go @@ -7,7 +7,7 @@ import ( "log" "time" - _ "github.com/fpt/go-pduckdb" // Import for driver registration only + _ "github.com/calvinchengx/go-pduckdb" // Import for driver registration only ) func main() { diff --git a/example/databasesql2/main.go b/example/databasesql2/main.go index ff76680..0b24749 100644 --- a/example/databasesql2/main.go +++ b/example/databasesql2/main.go @@ -5,7 +5,7 @@ import ( "fmt" "log" - _ "github.com/fpt/go-pduckdb" // Import for driver registration + _ "github.com/calvinchengx/go-pduckdb" // Import for driver registration ) func main() { diff --git a/example/enhancedtypes/main.go b/example/enhancedtypes/main.go index 82bc05c..fda2d3f 100644 --- a/example/enhancedtypes/main.go +++ b/example/enhancedtypes/main.go @@ -7,7 +7,7 @@ import ( "log" "time" - _ "github.com/fpt/go-pduckdb" // Import for driver registration + _ "github.com/calvinchengx/go-pduckdb" // Import for driver registration ) func main() { diff --git a/example/json/main.go b/example/json/main.go index 637afaf..0a12038 100644 --- a/example/json/main.go +++ b/example/json/main.go @@ -8,7 +8,7 @@ import ( "github.com/pkg/errors" - _ "github.com/fpt/go-pduckdb" // Import for driver registration + _ "github.com/calvinchengx/go-pduckdb" // Import for driver registration ) // Person represents a person with name, age, and custom attributes diff --git a/example/multistatement/main.go b/example/multistatement/main.go index 301f2e1..afe5633 100644 --- a/example/multistatement/main.go +++ b/example/multistatement/main.go @@ -5,7 +5,7 @@ import ( "database/sql" "log" - _ "github.com/fpt/go-pduckdb" + _ "github.com/calvinchengx/go-pduckdb" ) func main() { diff --git a/example/simple/main.go b/example/simple/main.go index 10fd7c3..489d47f 100644 --- a/example/simple/main.go +++ b/example/simple/main.go @@ -5,7 +5,7 @@ import ( "os" "time" - pd "github.com/fpt/go-pduckdb" + pd "github.com/calvinchengx/go-pduckdb" ) func Exists(filename string) bool { diff --git a/go.mod b/go.mod index b247bdb..f82d859 100644 --- a/go.mod +++ b/go.mod @@ -1,4 +1,4 @@ -module github.com/fpt/go-pduckdb +module github.com/calvinchengx/go-pduckdb go 1.24.2 diff --git a/internal/duckdb/statement.go b/internal/duckdb/statement.go index 82b51b3..31d52b4 100644 --- a/internal/duckdb/statement.go +++ b/internal/duckdb/statement.go @@ -5,7 +5,7 @@ import ( "fmt" "math" - "github.com/fpt/go-pduckdb/internal/convert" + "github.com/calvinchengx/go-pduckdb/internal/convert" "github.com/pkg/errors" ) diff --git a/pduckdb.go b/pduckdb.go index 70fa132..1629202 100644 --- a/pduckdb.go +++ b/pduckdb.go @@ -2,7 +2,7 @@ package pduckdb import ( - "github.com/fpt/go-pduckdb/internal/duckdb" + "github.com/calvinchengx/go-pduckdb/internal/duckdb" ) // DuckDB represents a DuckDB database instance diff --git a/pduckdb_test.go b/pduckdb_test.go index 587b1e8..c2a349d 100644 --- a/pduckdb_test.go +++ b/pduckdb_test.go @@ -3,7 +3,7 @@ package pduckdb import ( "testing" - "github.com/fpt/go-pduckdb/internal/duckdb" + "github.com/calvinchengx/go-pduckdb/internal/duckdb" ) // testDuckDB creates a mock DuckDB instance for testing diff --git a/types_test.go b/types_test.go index 75141f7..043025f 100644 --- a/types_test.go +++ b/types_test.go @@ -4,7 +4,7 @@ import ( "database/sql" "testing" - "github.com/fpt/go-pduckdb/internal/duckdb" + "github.com/calvinchengx/go-pduckdb/internal/duckdb" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) From 2bf3973e29a6186fe945c8cdec79af8b062f1155 Mon Sep 17 00:00:00 2001 From: Calvin Cheng Date: Sun, 23 Aug 2026 10:29:05 +0800 Subject: [PATCH 07/10] Say what this fork is and why --- README.md | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 48104cb..1e7aa7e 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,16 @@ # go-pduckdb is a PureGO driver for [DuckDB](https://duckdb.org/docs/stable/clients/c/api.html) +> **This is a fork** of [fpt/go-pduckdb](https://github.com/fpt/go-pduckdb) by Youichi +> Fujimoto (MIT), maintained so that Linux, macOS and Windows are all first-class: +> windows/arm64 support, a compile-time check on the Windows struct-by-value ABI +> workaround, macOS amd64 and Linux arm64 back in CI, and a musl build proven +> rather than assumed. See [docs/COMPATIBILITY.md](docs/COMPATIBILITY.md). +> +> The module path is `github.com/calvinchengx/go-pduckdb`; everything else +> matches upstream. Changes are offered back — see +> [fpt/go-pduckdb#37](https://github.com/fpt/go-pduckdb/pull/37) — and this fork +> exists to be merged out of existence if they land. + ## Introduction A DuckDB module for Go which doesn't require CGO. @@ -32,7 +43,7 @@ See [docs/COMPATIBILITY.md](./docs/COMPATIBILITY.md) for the full supported feat ## Installation ```bash -go get github.com/fpt/go-pduckdb +go get github.com/calvinchengx/go-pduckdb ``` Also, make sure to install DuckDB on your platform: @@ -99,7 +110,7 @@ import ( "fmt" "log" - _ "github.com/fpt/go-pduckdb" // Import for driver registration + _ "github.com/calvinchengx/go-pduckdb" // Import for driver registration ) func main() { From b2a8c41212a051624bdd917a46eb1cd4b4a5b7b2 Mon Sep 17 00:00:00 2001 From: Calvin Cheng Date: Sun, 23 Aug 2026 10:29:05 +0800 Subject: [PATCH 08/10] Drop the AI review workflow; it needs an upstream secret --- .github/workflows/ai-review.yml | 35 --------------------------------- 1 file changed, 35 deletions(-) delete mode 100644 .github/workflows/ai-review.yml diff --git a/.github/workflows/ai-review.yml b/.github/workflows/ai-review.yml deleted file mode 100644 index 647ca8c..0000000 --- a/.github/workflows/ai-review.yml +++ /dev/null @@ -1,35 +0,0 @@ -# AI code review for this repo's PRs, delegated to klein-cli's reusable -# workflow (github.com/fpt/klein-cli). The klein binary is installed from its -# latest release; all git/gh interaction happens inside that action. -# -# Trust model: `on: pull_request` (NOT pull_request_target) plus the same-repo -# `if` gate below means the API-key secret is only exposed to code from -# contributors who already have push access. Fork PRs get no secrets and are -# skipped. Do not switch to pull_request_target while checking out the PR head. -name: AI Review - -on: - pull_request: - types: [opened, synchronize, reopened] - -# `contents: write` is not used to push anything — the review writes no code. -# GitHub gates the `resolveReviewThread` GraphQL mutation on it (not on -# `pull-requests: write`, as one would expect), and the reusable workflow now -# resolves threads the reviewer has verified as fixed. It has to be granted -# here because a reusable workflow can only narrow the permissions it inherits; -# inside it, the job that checks out and runs this PR's code is narrowed back -# to `contents: read`, so no job holds both a write token and PR-authored code. -# See fpt/klein-cli#111. -permissions: - contents: write - pull-requests: write - -jobs: - review: - if: github.event.pull_request.head.repo.full_name == github.repository - uses: fpt/klein-cli/.github/workflows/ai-review-reusable.yml@main - with: - backend: openai - language: en - secrets: - openai-api-key: ${{ secrets.OPENAI_API_KEY }} From 3c541567a0afbc3b7c0c87b31c31c2b0899f9864 Mon Sep 17 00:00:00 2001 From: Calvin Cheng Date: Sun, 23 Aug 2026 12:02:03 +0800 Subject: [PATCH 09/10] Say the Makefile is POSIX-only; add the musl targets --- Makefile | 26 ++++++++++++++++++++++++-- README.md | 10 +++++++++- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 1807822..ee492b4 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,18 @@ -.PHONY: integ +# go-pduckdb — a DuckDB driver for Go that needs no CGO. +# +# These targets are convenience, and they assume GNU make and a POSIX shell: +# Linux, macOS, or WSL / Git Bash on Windows. They are NOT the build. +# +# The build and the tests are the Go toolchain alone and run natively on all +# three, on amd64 and arm64 -- CI proves it on six targets, and it invokes `go` +# directly rather than make for exactly that reason. On Windows: +# +# go test ./... +# +# with duckdb.dll resolvable (see "Library Path Configuration" in the README). +# `make help` and the docker targets are the parts that want a POSIX shell. + +.PHONY: run test fmt lint integ integ-arm64 integ-musl integ-musl-arm64 help run: ## Run the application CGO_ENABLED=0 go run example/simple/main.go @@ -26,6 +40,14 @@ integ-arm64: ## Run integration tests on arm64 docker build --platform linux/arm64 --build-arg GOARCH=arm64 --build-arg LIBARCH=arm64 -t go-pduckdb/integ-arm64 -f internal/integ/Dockerfile . && \ docker run --rm go-pduckdb/integ-arm64 +integ-musl: ## Run integration tests against the musl build of DuckDB + docker build --platform linux/amd64 -t go-pduckdb/integ-musl -f internal/integ/Dockerfile.musl . && \ + docker run --rm go-pduckdb/integ-musl + +integ-musl-arm64: ## Run integration tests against the musl build on arm64 + docker build --platform linux/arm64 --build-arg GOARCH=arm64 --build-arg LIBARCH=arm64 -t go-pduckdb/integ-musl-arm64 -f internal/integ/Dockerfile.musl . && \ + docker run --rm go-pduckdb/integ-musl-arm64 + help: ## Display this help - @grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | \ + @grep -E '^[a-zA-Z0-9_-]+:.*?## .*$$' $(MAKEFILE_LIST) | \ awk 'BEGIN {FS = ":.*?## "}; {printf "%-20s %s\n", $$1, $$2}' diff --git a/README.md b/README.md index 1e7aa7e..a11ad0c 100644 --- a/README.md +++ b/README.md @@ -71,7 +71,15 @@ For other Linux, Check official instruction: [Building DuckDB](https://duckdb.or ### Windows Download the DuckDB CLI from the [official website](https://duckdb.org/docs/installation/) and place the DLL in your system path. -Note: Windows support relies on an ABI-level workaround for purego's lack of struct-by-value arguments on Windows — see the [Windows workaround](./docs/COMPATIBILITY.md#windows-workaround) section in the compatibility docs. +Note: Windows support relies on an ABI-level workaround for purego's lack of struct-by-value arguments on Windows — see the [Windows workaround](./docs/COMPATIBILITY.md#windows-workaround) section in the compatibility docs. Both amd64 and arm64 are supported and both run the unit tests in CI on every push. + +Once `duckdb.dll` is resolvable, the tests are the Go toolchain and nothing else: + +```powershell +go test ./... +``` + +The `Makefile` is convenience rather than the build — its targets assume GNU make and a POSIX shell, so on Windows they want WSL or Git Bash. Nothing is lost by skipping it: `make test` is `go test ./...`, and the remaining targets build Docker images or run `gofumpt` and `golangci-lint`. CI invokes `go` directly for the same reason, which is what makes the Windows results mean anything. ## Library Path Configuration From 036c71681662bd2be0676a9ee93995e76ccdeb4b Mon Sep 17 00:00:00 2001 From: Calvin Cheng Date: Sun, 23 Aug 2026 13:50:41 +0800 Subject: [PATCH 10/10] Open a database with configuration, and so read-only duckdb_open takes no configuration, so there was no way to ask for a read-only database. A process whose only job is to read should not be able to write by accident, and asking the engine to enforce that is worth more than intending it. Options arrive as a DSN query string and go through duckdb_open_ext. The last ? separates, so a path containing one is still openable, and a path with no ? takes the original code path unchanged. --- docs/COMPATIBILITY.md | 13 +++++++ driver.go | 37 +++++++++++++++++- dsn_test.go | 83 +++++++++++++++++++++++++++++++++++++++++ internal/duckdb/db.go | 51 +++++++++++++++++++++++-- internal/duckdb/type.go | 4 ++ pduckdb.go | 15 +++++++- 6 files changed, 196 insertions(+), 7 deletions(-) create mode 100644 dsn_test.go diff --git a/docs/COMPATIBILITY.md b/docs/COMPATIBILITY.md index 0f31ae5..ec356d6 100644 --- a/docs/COMPATIBILITY.md +++ b/docs/COMPATIBILITY.md @@ -66,6 +66,19 @@ search, which reaches the current directory and `PATH`, and it also lets a DuckDB DLL resolve dependencies sitting beside it. A bare name still uses the standard search so that putting a directory on `PATH` keeps working. +## Opening a database + +| Feature | Status | Notes | +|---|---|---| +| Path | ✅ | `sql.Open("duckdb", "warehouse.duckdb")`, or `:memory:` | +| Configuration options | ✅ | As a DSN query string: `"warehouse.duckdb?access_mode=READ_ONLY&threads=2"` | +| Read-only | ✅ | `access_mode=READ_ONLY` — the database refuses writes, rather than the caller intending not to make any | + +Options go through `duckdb_open_ext`; a path with no `?` uses `duckdb_open` +unchanged. The **last** `?` separates path from options, so a database file +whose name contains one is still reachable. An option DuckDB does not +recognise fails the open with DuckDB's own message rather than being ignored. + ## database/sql driver interface | Feature | Status | Notes | diff --git a/driver.go b/driver.go index 82e971c..5ff0a5f 100644 --- a/driver.go +++ b/driver.go @@ -5,7 +5,9 @@ import ( "database/sql" "database/sql/driver" "io" + "net/url" "reflect" + "strings" "github.com/pkg/errors" @@ -20,10 +22,41 @@ func init() { // Driver implements database/sql/driver.Driver type Driver struct{} +// splitDSN separates the database path from DuckDB configuration options. +// +// The last `?` separates, not the first: a DuckDB path may legitimately +// contain one, and taking the first would make such a file unopenable through +// database/sql with no way to say otherwise. +func splitDSN(dsn string) (string, map[string]string) { + at := strings.LastIndex(dsn, "?") + if at < 0 { + return dsn, nil + } + query, err := url.ParseQuery(dsn[at+1:]) + if err != nil || len(query) == 0 { + return dsn, nil + } + settings := make(map[string]string, len(query)) + for name, values := range query { + settings[name] = values[len(values)-1] + } + return dsn[:at], settings +} + // Open returns a new connection to the database. -// The dsn is a connection string for the database. +// +// The dsn is the path to the database file, optionally followed by DuckDB +// configuration options as a query string: +// +// sql.Open("duckdb", "warehouse.duckdb?access_mode=READ_ONLY") +// sql.Open("duckdb", ":memory:") +// +// A path containing a `?` that is not meant as options can be written +// `./odd?name.duckdb?` -- the LAST `?` separates. Without one, the whole +// string is the path, so existing callers are unaffected. func (d *Driver) Open(dsn string) (driver.Conn, error) { - db, err := NewDuckDB(dsn) + path, settings := splitDSN(dsn) + db, err := NewDuckDBWithSettings(path, settings) if err != nil { return nil, err } diff --git a/dsn_test.go b/dsn_test.go new file mode 100644 index 0000000..e6a71a4 --- /dev/null +++ b/dsn_test.go @@ -0,0 +1,83 @@ +package pduckdb + +import ( + "database/sql" + "path/filepath" + "reflect" + "testing" +) + +func TestSplitDSN(t *testing.T) { + for _, c := range []struct { + name, dsn, path string + settings map[string]string + }{ + {"a plain path", "warehouse.duckdb", "warehouse.duckdb", nil}, + {"in memory", ":memory:", ":memory:", nil}, + {"read only", "w.duckdb?access_mode=READ_ONLY", "w.duckdb", + map[string]string{"access_mode": "READ_ONLY"}}, + {"several options", "w.duckdb?access_mode=READ_ONLY&threads=2", "w.duckdb", + map[string]string{"access_mode": "READ_ONLY", "threads": "2"}}, + // The LAST ? separates, so a path containing one is still openable. + {"a path with a question mark", "odd?name.duckdb?access_mode=READ_ONLY", + "odd?name.duckdb", map[string]string{"access_mode": "READ_ONLY"}}, + {"a trailing question mark is part of the path", "odd?name.duckdb?", + "odd?name.duckdb?", nil}, + } { + t.Run(c.name, func(t *testing.T) { + path, settings := splitDSN(c.dsn) + if path != c.path || !reflect.DeepEqual(settings, c.settings) { + t.Errorf("splitDSN(%q) = %q, %v; want %q, %v", c.dsn, path, settings, c.path, c.settings) + } + }) + } +} + +// A read-only database must refuse a write. Intending not to write is not the +// same as being unable to, and for a process whose only job is to read, the +// difference is the whole point of asking. +func TestReadOnlyRefusesAWrite(t *testing.T) { + path := filepath.Join(t.TempDir(), "w.duckdb") + + writable, err := sql.Open("duckdb", path) + if err != nil { + t.Skipf("no DuckDB library available: %v", err) + } + if _, err := writable.Exec("CREATE TABLE t (a INTEGER)"); err != nil { + t.Skipf("no DuckDB library available: %v", err) + } + if _, err := writable.Exec("INSERT INTO t VALUES (1)"); err != nil { + t.Fatal(err) + } + if err := writable.Close(); err != nil { + t.Fatal(err) + } + + readonly, err := sql.Open("duckdb", path+"?access_mode=READ_ONLY") + if err != nil { + t.Fatal(err) + } + defer func() { _ = readonly.Close() }() + + var n int + if err := readonly.QueryRow("SELECT COUNT(*) FROM t").Scan(&n); err != nil { + t.Fatalf("a read-only database should still read: %v", err) + } + if n != 1 { + t.Errorf("read %d rows, want 1", n) + } + if _, err := readonly.Exec("INSERT INTO t VALUES (2)"); err == nil { + t.Error("a read-only database accepted a write") + } +} + +func TestUnknownSettingIsReported(t *testing.T) { + db, err := sql.Open("duckdb", filepath.Join(t.TempDir(), "w.duckdb")+"?not_a_setting=1") + if err != nil { + t.Fatal(err) + } + defer func() { _ = db.Close() }() + if err := db.Ping(); err == nil { + t.Error("a setting DuckDB does not know should not open silently") + } +} diff --git a/internal/duckdb/db.go b/internal/duckdb/db.go index d79ee98..6dc0766 100644 --- a/internal/duckdb/db.go +++ b/internal/duckdb/db.go @@ -133,7 +133,13 @@ type DB struct { } // NewDB creates a new internal database instance -func NewDB(path string) (*DB, error) { +// NewDB opens a database. +// +// settings are DuckDB configuration options, applied before the database is +// opened -- `access_mode: READ_ONLY` is the one that motivated this, since a +// process that only ever reads should not be able to write by accident. With +// no settings the plain duckdb_open path is used, unchanged. +func NewDB(path string, settings map[string]string) (*DB, error) { db := &DB{} // Load DuckDB library @@ -146,6 +152,14 @@ func NewDB(path string) (*DB, error) { // Register DuckDB functions var open func(path string, out *DuckDBDatabase) DuckDBState purego.RegisterLibFunc(&open, lib, "duckdb_open") + var openExt func(path string, out *DuckDBDatabase, config DuckDBConfig, err **byte) DuckDBState + purego.RegisterLibFunc(&openExt, lib, "duckdb_open_ext") + var createConfig func(out *DuckDBConfig) DuckDBState + purego.RegisterLibFunc(&createConfig, lib, "duckdb_create_config") + var setConfig func(config DuckDBConfig, name, option string) DuckDBState + purego.RegisterLibFunc(&setConfig, lib, "duckdb_set_config") + var destroyConfig func(config *DuckDBConfig) + purego.RegisterLibFunc(&destroyConfig, lib, "duckdb_destroy_config") purego.RegisterLibFunc(&db.Connect, lib, "duckdb_connect") purego.RegisterLibFunc(&db.Close, lib, "duckdb_close") purego.RegisterLibFunc(&db.Disconnect, lib, "duckdb_disconnect") @@ -265,9 +279,38 @@ func NewDB(path string) (*DB, error) { // Open database var handle DuckDBDatabase - state := open(path, &handle) - if state != DuckDBSuccess { - return nil, fmt.Errorf("failed to open database: %s", path) + if len(settings) == 0 { + if open(path, &handle) != DuckDBSuccess { + return nil, fmt.Errorf("failed to open database: %s", path) + } + db.Handle = handle + return db, nil + } + + // With settings, go through duckdb_open_ext: it is the only entry point + // that takes a configuration, and the only way to ask for a read-only + // database. DuckDB reports why it refused through an out parameter that + // the caller owns and must free. + var config DuckDBConfig + if createConfig(&config) != DuckDBSuccess { + return nil, fmt.Errorf("failed to create a DuckDB configuration") + } + defer destroyConfig(&config) + for name, value := range settings { + if setConfig(config, name, value) != DuckDBSuccess { + return nil, fmt.Errorf("DuckDB rejected the setting %s=%s", name, value) + } + } + var openErr *byte + if openExt(path, &handle, config, &openErr) != DuckDBSuccess { + message := GoString(openErr) + if openErr != nil { + db.Free(unsafe.Pointer(openErr)) + } + if message == "" { + message = "no reason given" + } + return nil, fmt.Errorf("failed to open database %s: %s", path, message) } db.Handle = handle diff --git a/internal/duckdb/type.go b/internal/duckdb/type.go index 272a03d..310c633 100644 --- a/internal/duckdb/type.go +++ b/internal/duckdb/type.go @@ -362,6 +362,10 @@ type DuckDBPreparedStatement unsafe.Pointer // DuckDBDatabase represents a DuckDB database type DuckDBDatabase unsafe.Pointer +// DuckDBConfig is a duckdb_config: options applied when a database is opened. +// Created and destroyed by the caller, never by DuckDB. +type DuckDBConfig unsafe.Pointer + // DuckDBLogicalType represents a DuckDB logical type type DuckDBLogicalType unsafe.Pointer diff --git a/pduckdb.go b/pduckdb.go index 1629202..40e9c46 100644 --- a/pduckdb.go +++ b/pduckdb.go @@ -12,7 +12,20 @@ type DuckDB struct { // NewDuckDB creates a new DuckDB instance func NewDuckDB(path string) (*DuckDB, error) { - db, err := duckdb.NewDB(path) + return NewDuckDBWithSettings(path, nil) +} + +// NewDuckDBWithSettings opens a database with DuckDB configuration options +// applied before it opens. +// +// db, err := NewDuckDBWithSettings("warehouse.duckdb", +// map[string]string{"access_mode": "READ_ONLY"}) +// +// Read-only is the option this exists for. A process that only ever reads +// should not be able to write by accident, and asking the engine to enforce +// that is worth more than intending it. +func NewDuckDBWithSettings(path string, settings map[string]string) (*DuckDB, error) { + db, err := duckdb.NewDB(path, settings) if err != nil { return nil, err }