refactor(server): drop the _ "duckdb-go" blank import from server.go#497
Merged
fuziontech merged 1 commit intomainfrom May 1, 2026
Merged
refactor(server): drop the _ "duckdb-go" blank import from server.go#497fuziontech merged 1 commit intomainfrom
fuziontech merged 1 commit intomainfrom
Conversation
server/server.go used to blank-import github.com/duckdb/duckdb-go/v2 to register the "duckdb" sql/driver. The all-in-one binary already imports duckdbservice (which itself imports duckdb-go via the appender + value normalizer hooks added in PR #482, #494, #496), so the driver gets registered through that chain — server.go's blank import is redundant in production. server/ package tests already blank-import duckdb-go in the files that need a real DuckDB connection (bundled_extensions_test.go, catalog_test.go, chsql_test.go, transient_test.go, session_database_metadata_test.go, types_test.go), so removing the production blank import doesn't break the test binaries. The integration test binary (tests/integration/) imported `server` but not `duckdbservice`, so it relied on server.go's blank import for driver registration. Added a blank import of duckdbservice in tests/integration/harness.go to compensate — same effect, just routed through the package that's now responsible for registering all duckdb-go hooks (driver, AppendValue handler, value normalizer, COPY appender). After this PR, server/server.go has zero `import` statements that pull duckdb-go into the package's import graph. server still links duckdb-go overall via querylog.go and checkpoint.go (both call sql.Open("duckdb", ...) directly), but that's about runtime use of an already-registered driver rather than a Go-level package import. The follow-up PRs that move querylog and checkpoint into server/exec/ will close out the file-level duckdb-go references in server/. Verified: - go build ./... clean - go build -tags kubernetes ./... clean - go test -short ./server/ ./duckdbservice/... ./controlplane/... ./ — all green - go test -short -timeout 60s ./tests/integration/... — same failures as main (TestFallbackUtilityCommands etc.); confirmed pre-existing via stash + re-run; unrelated to this PR Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
fuziontech
added a commit
that referenced
this pull request
May 1, 2026
…ink libduckdb (#498) After the long sequence of subpackage extractions (PRs #477-#497) the controlplane import graph is fully free of github.com/duckdb/duckdb-go: go list -deps ./controlplane | grep duckdb-go # empty go list -tags kubernetes -deps ./controlplane | grep duckdb-go # empty go list -deps ./cmd/duckgres-controlplane | grep duckdb-go # empty go list -tags kubernetes -deps ./cmd/duckgres-controlplane | grep duckdb-go # empty This PR adds the corresponding entry-point binary at cmd/duckgres-controlplane. The binary builds and links without libduckdb, but the actual config-loading + RunControlPlane wiring is still a stub that errors out at runtime — duckgres' YAML / CLI / env config resolution lives in the package main file at the repo root alongside the all-in-one binary, and lifting that into a shared config-resolution package is its own focused PR. The point of this PR is to lock in the import-graph win: a CP-only build is now possible. Future PRs will: - Move resolveEffectiveConfig + the YAML/env/CLI plumbing out of main.go (repo root) into a shared package both binaries can import (still duckdb-free, since it's pure config wrangling). - Wire that into cmd/duckgres-controlplane/main.go and remove the stub error. - Add a CI step that runs the deps check above and fails the build if duckdb-go ever creeps back in. - Introduce cmd/duckgres-worker for the duckdb-service path so the matrix-build CI in the binary-split plan can target it. Verified: - go build ./cmd/duckgres-controlplane/... clean - go build -tags kubernetes ./cmd/duckgres-controlplane/... clean - go list -deps ./cmd/duckgres-controlplane | grep duckdb-go is empty - same with -tags kubernetes - The all-in-one duckgres binary at the repo root continues to build and run unchanged (it imports duckdbservice, which still pulls duckdb-go — that's expected for the worker / standalone modes). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
server/server.goused to blank-importgithub.com/duckdb/duckdb-go/v2to register the"duckdb"sql/driver. The all-in-one binary already importsduckdbservice(which itself importsduckdb-govia the appender + value normalizer hooks added in PR #482, #494, #496), so the driver gets registered through that chain —server.go's blank import is redundant in production.Test binary handling
Server-package tests already blank-import
duckdb-goin the files that need a real DuckDB connection (bundled_extensions_test.go,catalog_test.go,chsql_test.go,transient_test.go,session_database_metadata_test.go,types_test.go), so removing the production blank import doesn't break those test binaries.The integration test binary (
tests/integration/) importedserverbut notduckdbservice, so it relied onserver.go's blank import for driver registration. Added a blank import ofduckdbserviceintests/integration/harness.goto compensate — same effect, just routed through the package that's now responsible for registering all duckdb-go hooks (driver,AppendValuehandler, value normalizer, COPY appender).What's left
After this PR,
server/server.gohas zeroimportstatements that pull duckdb-go into the package's import graph.serverstill linksduckdb-gooverall viaquerylog.goandcheckpoint.go(both callsql.Open("duckdb", ...)directly), but that's about runtime use of an already-registered driver rather than a Go-level package import. The follow-up PRs that move querylog and checkpoint intoserver/exec/will close out the file-level duckdb-go references inserver/.Test plan
go build ./...cleango build -tags kubernetes ./...cleango test -short ./server/ ./duckdbservice/... ./controlplane/... ./— all greengo test -short -timeout 60s ./tests/integration/...— same failures as main (TestFallbackUtilityCommandsetc.); confirmed pre-existing viagit stash+ re-run; unrelated to this PR🤖 Generated with Claude Code