*: Phase 2.B — vendor pqarrow/builder + arrowutils into pkg/query/internal#6355
Open
thorfour wants to merge 1 commit into
Open
*: Phase 2.B — vendor pqarrow/builder + arrowutils into pkg/query/internal#6355thorfour wants to merge 1 commit into
thorfour wants to merge 1 commit into
Conversation
Phase 2.B of the FrostDB removal. Drops the pqarrow imports from the
query path by vendoring the two helper packages.
Why vendor instead of replace with arrow-go stdlib builders:
The Opt* builders (OptInt64Builder, OptInt32Builder, OptBooleanBuilder)
expose random-access mutation methods — Set(idx, v), Add(idx, delta),
Value(idx), AppendData([]int64) — that the flamegraph and table query
algorithms rely on for backfilling cumulative counts and child indices.
arrow-go's array.*Builder is append-only, so swapping would require
rewriting both algorithms. Vendoring is mechanical, low-risk, and
unblocks Phase 4 (drop frostdb from go.mod).
What's vendored:
* pkg/query/internal/builder/ — listbuilder.go, optbuilders.go,
recordbuilder.go, utils.go from frostdb/pqarrow/builder. Plus
optbuilders_test.go (passes against the vendored copy). The
AppendParquetValues methods on the Opt builders and the parquet-go
import are stripped — parca uses these builders only on the query
side and never to convert parquet values, so they're dead.
* pkg/query/internal/arrowutils/ — groupranges.go, merge.go,
nullarray.go, schema.go, sort.go, utils.go from
frostdb/pqarrow/arrowutils. Plus sort_test.go. merge_test.go and
schema_test.go are skipped — they depend on
frostdb/internal/records which is not importable from outside the
frostdb module.
Each vendored package gets a doc.go explaining provenance.
Consumers updated:
* pkg/query/{flamegraph_arrow,table}.go: import path swap from
frostdb/pqarrow/builder to internal/builder.
* pkg/query/columnquery.go: import path swap from
frostdb/pqarrow/arrowutils to internal/arrowutils.
* pkg/parcacol/querier.go: still on the upstream arrowutils because
pkg/parcacol can't import pkg/query/internal/arrowutils. That file
goes away in Phase 3 anyway.
go.mod: parquet-go is demoted from direct to indirect (still pulled in
by frostdb itself; goes away in Phase 4).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
✅ Meticulous spotted 0 visual differences across 288 screens tested: view results. Meticulous evaluated ~4 hours of user flows against your PR. Expected differences? Click here. Last updated for commit |
5 tasks
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
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
Phase 2.B of the FrostDB removal. Stacks on #6354 (Phase 2.A).
Drops the
pqarrowimports from the query path by vendoring the two helper packages.Why vendor instead of replacing with
arrow-gostdlib buildersThe Opt* builders (
OptInt64Builder,OptInt32Builder,OptBooleanBuilder) expose random-access mutation methods —Set(idx, v),Add(idx, delta),Value(idx),AppendData([]int64)— that the flamegraph and table query algorithms rely on for backfilling cumulative counts and child indices.arrow-go'sarray.*Builderis append-only, so swapping would require rewriting both algorithms. Vendoring is mechanical, low-risk, and unblocks Phase 4 (drop frostdb from go.mod).What's vendored
pkg/query/internal/builder/—listbuilder.go,optbuilders.go,recordbuilder.go,utils.gofromfrostdb/pqarrow/builder. Plusoptbuilders_test.go(passes against the vendored copy). TheAppendParquetValuesmethods on the Opt builders and theparquet-goimport are stripped — parca uses these builders only on the query side and never to convert parquet values, so they're dead code.pkg/query/internal/arrowutils/—groupranges.go,merge.go,nullarray.go,schema.go,sort.go,utils.gofromfrostdb/pqarrow/arrowutils. Plussort_test.go.merge_test.goandschema_test.goare skipped — they depend onfrostdb/internal/records, which is not importable from outside the frostdb module.Each vendored package gets a
doc.goexplaining provenance.Consumers updated
pkg/query/{flamegraph_arrow,table}.go: import path swap fromfrostdb/pqarrow/buildertointernal/builder.pkg/query/columnquery.go: import path swap fromfrostdb/pqarrow/arrowutilstointernal/arrowutils.pkg/parcacol/querier.go: still on the upstreamarrowutilsbecausepkg/parcacolcan't importpkg/query/internal/arrowutils. That file goes away in Phase 3.go.mod:parquet-gois demoted from direct to indirect (still pulled in by frostdb itself; goes away in Phase 4).Test plan
go build ./...go vet ./...go test -short ./...go test ./pkg/query/internal/builder/...)go test ./pkg/query/internal/arrowutils/...)🤖 Generated with Claude Code