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 }} 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. 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/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 48104cb..a11ad0c 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: @@ -60,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 @@ -99,7 +118,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/docs/COMPATIBILITY.md b/docs/COMPATIBILITY.md index 160a95f..ec356d6 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,61 @@ 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`). +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. + +## Opening a database -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. +| 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 | -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`). +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 @@ -51,7 +93,7 @@ finally the standard `LoadLibrary` search path (`PATH`). | `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 | @@ -102,3 +144,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 | diff --git a/driver.go b/driver.go index 65600f7..5ff0a5f 100644 --- a/driver.go +++ b/driver.go @@ -5,11 +5,13 @@ import ( "database/sql" "database/sql/driver" "io" + "net/url" "reflect" + "strings" "github.com/pkg/errors" - "github.com/fpt/go-pduckdb/internal/duckdb" + "github.com/calvinchengx/go-pduckdb/internal/duckdb" ) // Initialize and register the driver @@ -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/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/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/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/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/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/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 } 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 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/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/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" ] diff --git a/pduckdb.go b/pduckdb.go index 70fa132..40e9c46 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 @@ -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 } 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" )