refactor(duckdbservice): extract DuckDB-free arrow helpers into arrowmap subpackage#477
Merged
fuziontech merged 1 commit intomainfrom May 1, 2026
Merged
Conversation
This was referenced Apr 30, 2026
…map subpackage Step 1 of a 4-step plan to split duckgres into separate control-plane and worker binaries so the control plane can stop linking libduckdb (and the worker image can ship per-DuckDB-version variants). This change moves the DuckDB-free helpers (DuckDBTypeToArrow, QualifyTableName, QuoteIdent, SupportsLimit, and the struct/map/decimal type-string parsers) out of duckdbservice/arrow_helpers.go into a new duckdbservice/arrowmap package. arrowmap has zero dependency on github.com/duckdb/duckdb-go; it's plain string-to-arrow.DataType mapping. The duckdbservice package keeps the helpers callable as before via thin re-export shims so existing call sites are unaffected. server/flightsqlingress is updated to import arrowmap directly for the three call sites that don't need DuckDB types (the AppendValue call site stays on duckdbservice for now; splitting AppendValue's duckdb-bound branches lands in a follow-up). This PR is purely a refactor — no behavior changes, no new binary, no CI changes. It establishes the package boundary and pattern that subsequent PRs build on. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fuziontech
force-pushed
the
feature/binary-split-pr1-arrowmap
branch
from
d559dee to
763e55b
Compare
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>
4 tasks
fuziontech
added a commit
that referenced
this pull request
May 1, 2026
Adds a controlplane-no-libduckdb job that builds cmd/duckgres-controlplane (both default tags and -tags kubernetes) and then runs go list -deps ./cmd/duckgres-controlplane | grep duckdb-go go list -deps ./controlplane | grep duckdb-go If anything matches, the job fails the build. This locks in the binary-split achievement from PRs #477-#498. Without this guard, someone could land a server/* extraction that grows a new duckdb-go transitive import and silently re-link libduckdb into the CP build — which would defeat the matrix-build per-DuckDB-version goal. The error message points at the right diagnostic command and suggests the right fix (subpackage extraction / registration hook / interface boundary) rather than encouraging anyone to suppress the check. Verified locally: - go list -deps ./cmd/duckgres-controlplane | grep duckdb-go is empty - go list -deps ./controlplane | grep duckdb-go is empty - same with -tags kubernetes 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
DuckDBTypeToArrow,QualifyTableName,QuoteIdent,SupportsLimit, struct/map/decimal type-string parsers) out ofduckdbservice/arrow_helpers.gointo a newduckdbservice/arrowmappackage with zeroduckdb-godependencyWhy
PR #462 added per-tenant DuckDB/DuckLake version pinning, but every
imagereferenced from the config store today is the all-in-oneduckgresbinary, which pins one DuckDB version per build. To make that pin per-tenant in production we want to:(DuckDB version × arch)from a single CI matrix.The CP can't be DuckDB-free until the packages it imports are DuckDB-free. Today
controlplane → server → duckdb-goandcontrolplane → server/flightsqlingress → duckdbservice → duckdb-go. This PR carves out a small piece ofduckdbservice(the type-string mapper and identifier quoter) into a leaf package that any caller can use without dragging libduckdb along.AppendValueis not moved in this PR — it has type switches onduckdb.Interval,duckdb.Decimal,duckdb.UUID,duckdb.OrderedMap,duckdb.Map. Splitting it (DuckDB-free core + DuckDB-bound extension) is the next step.What's next
DuckLakeConfigandducklake_migration.gointo a newserver/ducklakepackage. The migration code is already DuckDB-free at the import level (it uses pgx against the tenant metadata Postgres) but currently lives in the same package asserver.go, which links DuckDB.server/split intoserver/wire,server/exec,server/flight, etc. Add a CI step that runsgo list -deps ./cmd/duckgres-controlplaneand fails if anyduckdb-gopackage shows up.cmd/duckgres-controlplaneandcmd/duckgres-workerbinaries; new Dockerfile per binary; standalone all-in-one binary kept for dev.imagecolumn semantics.Test plan
go build ./...clean (excluding pre-existingtests/perfissue unrelated to this PR)go build -tags kubernetes ./controlplane/...cleango test ./duckdbservice/arrowmap/...— all moved tests pass in the new packagego test ./duckdbservice/... ./server/... ./controlplane/...— green (admin testcontainer failures pre-exist on main and require Docker)go list -deps ./duckdbservice/arrowmapcontains zeroduckdb-gopackagesgo vet ./...clean🤖 Generated with Claude Code