compute: native columnar consolidate_named (F1)#37756
Draft
antiguru wants to merge 1 commit into
Draft
Conversation
Replace the decode, consolidate, re-encode round-trip in the `Columnar` arm of `CollectionEdge::consolidate_named` with a native columnar consolidation. The old arm ran `columnar_to_vec` then `consolidate_named::<KeyBatcher>` then `vec_to_columnar`, allocating an owned `Row` per record in the decode. The new `columnar_consolidate` helper stays columnar throughout: it reshapes each record into the `((Row, ()), T, Diff)` key-batcher shape, merges via `Col2KeyBatcher` under a `columnar_exchange` pact, and unpacks back into a `Column`. Rows, times, and diffs are pushed borrowed, so no owned `Row` is materialized on the hot path. This uses `consolidate_pact`, the same primitive the `Vec` arm uses, not `mz_arrange_core`. A consolidate emits a consolidated collection, so building and reading back a maintained trace would be wasted work. `Col2KeyBatcher` and the `Vec` arm's `KeyBatcher` produce the same `ColumnationStack` output and differ only in their input chunker, so the unpack loop matches the `Vec` arm's. This is the temporary second consolidate variant. At teardown T3 it becomes the sole implementation. It serves Union's `concat_many` then `consolidate_named` path once Union inputs are columnar in Wave 2. Producers still emit `Vec`, so the columnar arm is dead in production and this change carries no runtime behavior change. The test `consolidate_named_preserves_columnar` now covers accumulation and cancellation across two distinct timestamps and asserts the columnar arm agrees with the `Vec` arm. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
antiguru
force-pushed
the
columnar-f1-consolidate
branch
from
July 20, 2026 20:06
d3f26c9 to
d74312c
Compare
antiguru
commented
Jul 21, 2026
Comment on lines
+342
to
+344
| let consolidated = consolidate_pact::<batcher::Chunker<_>, Col2KeyBatcher<Row, T, Diff>, _, _>( | ||
| keyed, exchange, name, | ||
| ); |
Member
Author
There was a problem hiding this comment.
There's a builder that produces Columnar chunks itself, so we wouldn't need the additional unpacking step.
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.
Replace the decode, consolidate, re-encode round-trip in the
Columnararm ofCollectionEdge::consolidate_namedwith a native columnar consolidation.The old arm ran
columnar_to_vec->consolidate_named::<KeyBatcher>->vec_to_columnar, allocating an ownedRowper record in the decode.The new
columnar_consolidatehelper stays columnar throughout: reshape into the((Row, ()), T, Diff)key-batcher shape, merge viaCol2KeyBatcherunder acolumnar_exchangepact, unpack back into aColumn, all with borrowed pushes.Uses
consolidate_pact(the primitive theVecarm already uses), notmz_arrange_core: a consolidate emits a consolidated collection, so building and reading back a maintained trace would be wasted work.Col2KeyBatcherand theVecarm'sKeyBatcherproduce the sameColumnationStackoutput and differ only in the input chunker, so the unpack loop matches theVecarm's.Temporary second variant; becomes the sole implementation at teardown T3.
Serves Union's
concat_many->consolidate_namedpath once Union inputs are columnar in Wave 2.Producers still emit
Vec, so the columnar arm is dead in production and this carries no runtime change.