Repository navigation
feat(engine): toward partitioned operators, a csv overwrite's source in slices, swept per partition - #1134
Merged
azimafroozeh merged 4 commits intoOct 22, 2026
Conversation
…in slices, swept per partition
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f504e3eda
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
5 tasks done
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.
What & why
A
load csvoverwrite parses its captured source with one Arrow reader and extracts what its checks consult (ids, each uniqueness check's keys, cardinality sources) in one pass over the held delta after the parse ends. On SF30 the parse is 190 s of the 413 s load (#1121's phase table) and the validation 175 s (the width-1 run below), both on one thread, while the planner already printsmax_partitionson the source and the sink writes with eight writers. This PR cuts the capture on record ends into 8 MiB slices in the resolve pass that already walks it, runs the source as up tostage_write_concurrency(default 8) partitions that each claim the next slice from a shared counter and tag their batches with its index, and has the validation drain every partition at once, sweeping each one's batches into a partial as they arrive, then sort the held batches by slice and merge the partials (ids unioned, each uniqueness check's first holders sharded by key hash and merged shard by shard, each shard on its own worker) before the checks judge once as before. A cut lands only after a complete record, so the slices concatenate to the single reader's rows in its order; every extracted fact carries its(slice, row)position, so the merged image and the first violation are the single pass's; and a refused slice stops the claiming while the slices in flight finish, so the lowest failed slice's record is the refusal on every run. SF1 loads in 8.6 s instead of 13.2, SF30 in 261 s instead of 413.Backing issue / RFC
docs/rfcs/2026-10-15-bulk-load-as-a-query-plan.md(feat(load): plan CSV overwrites and reuse validation reads #1026, perf(load): check edge endpoints in an unchained integer key set #1085, feat(explain): cut pipelines at breakers on every route and measure a load's operators #1099, perf(load): fan out a csv overwrite's Lance writers over file-sized chunks #1121). The RFC's workers read ranges of the source; here they claim slices of the whole capture the load already holds, so every cut is proved before a partition starts and the declaredsingle_scanfallback never fires. D11 (each phase holds its whole input) is unchanged. That and perf(load): fan out a csv overwrite's Lance writers over file-sized chunks #1121's writer fan-out owe the RFC a sentence.Checklist
with_csv_slice_bytescuts at 64 bytes: the cut (record ends numbered from the source, no header, CRLF cut after the line feed, never inside a quoted field, a header alone is one empty slice, a slice's parse error names the source line), the source (three partitions claim every slice once and tag their batches; a refused slice stops the claims), the breaker (partitions drain back into source order; the lowest failed slice names the refusal; the first repeated key in source order is the refusal from one and from three partitions), the planner's partition count (8 MiB is 1, 8 MiB + 1 is 2, capped by the load workers), the end-to-endload_csv_overwrite_from_many_slices_keeps_source_order(2,000 rows, every slice in the receipt, keys read back in order), and the gqt-coreload pipeline … partitions <n>round trip. The GQT corpus (412 cases) is unchanged: every fixture is under 8 MiB, and every case's refusal equals the sequential sweep's.docs/user/queries/explain-load.md,docs/user/queries/explain.md,docs/user/queries/settings.md,docs/user/mutations/index.md,crates/omnigraph-gqt/README.md;docs/dev/execution.mdanddocs/dev/writes.mdfor the mechanismchangelog.d/load-csv-parse-partitions.performance.mdLocal verification
load csv … mode overwritestatements throughomnigraph mutate --json, release builds ofmain(888223b7) and this branch at the default width, 2 rounds interleaved, phases read fromexecuted.operatorsas in feat(explain): cut pipelines at breakers on every route and measure a load's operators #1099: load 13.19 → 8.64 s; Comment 2.63 → 1.15 s (parse 1.31 → 0.43 s, validate 0.81 → 0.25 s), Post 1.26 → 0.61 s, HasMember 1.19 → 0.65 s; the 16 statements under 8 MiB and the write phase unchanged.OMNIGRAPH_LOAD_CONCURRENCY1 vs 8, one round each: parse 168.6 → 48.4 s (3.5×; HasMember, LikeComment, LikePost 4.1×), validate 175.4 → 171.1 s; width 1 also serializes perf(load): fan out a csv overwrite's Lance writers over file-sized chunks #1121's writers, so the load total (481 → 296 s) overstates the parse gain.Notes for reviewers
Source::with_partitionswith per-partition metrics, the breaker draining N inputs,partitionson the planner's pipelines), but only the load's source is wide; every query operator and the sink still run one stream.stage_write_concurrency(OMNIGRAPH_LOAD_CONCURRENCY, default 8) now also caps the source's partitions, one per 8 MiB of source;1restores the single reader, and a source under 8 MiB plans one partition at any width.openapi.jsonchanges shape, additive:LoadOperatorExecutedOutputgainspartitionsandslices(LoadSliceExecutedOutput: the slice, the partition that claimed it, its rows and timings); the explain'spipelinesentries gainpartitionsunder the sameexplain_version.