Columnar rendering: complete migration (Prompts 2.3–10.1)#179
Open
antiguru wants to merge 11 commits intocolumnar_renderingfrom
Open
Columnar rendering: complete migration (Prompts 2.3–10.1)#179antiguru wants to merge 11 commits intocolumnar_renderingfrom
antiguru wants to merge 11 commits intocolumnar_renderingfrom
Conversation
- Use Rc::clone(&results) instead of results.clone() for Rc pointers - Replace vec![...] with array literal where Vec is unnecessary - Replace Iterator::zip (disallowed) with direct assert_eq https://claude.ai/code/session_01JHo5sTCSGPW5NavNE2b49d
…atches New prompts 11.1–11.6 to eliminate columnar→Vec conversions by operating directly on &RowRef from columnar containers. Key insight: DatumVec's borrow_with already accepts &RowRef (the columnar Ref<'_, Row> type), so operators can process columnar data without materializing owned Rows. https://claude.ai/code/session_01JHo5sTCSGPW5NavNE2b49d
…rompt 11.1) Add a columnar path to CollectionBundle::flat_map that iterates the columnar container via into_index_iter(), passing &RowRef directly to borrow_with_limit. This eliminates the columnar→Vec conversion and avoids allocating owned Row values. Only timestamps and diffs are converted to owned (cheap scalar copies). https://claude.ai/code/session_01JHo5sTCSGPW5NavNE2b49d
…tly (Prompt 11.2) Add as_specific_columnar_collection that returns the columnar collection without conversion when key is None. Optimize as_columnar_collection_core to detect identity MFPs and skip the columnar→Vec→columnar round-trip. https://claude.ai/code/session_01JHo5sTCSGPW5NavNE2b49d
Identity MFPs return columnar directly (11.2). Non-identity MFPs iterate columnar via &RowRef (11.1) but output is Vec-based due to map_fallible Ok/Err split. Updated doc comment to reflect current state. https://claude.ai/code/session_01JHo5sTCSGPW5NavNE2b49d
render_reduce calls flat_map which now iterates &RowRef directly from columnar containers (11.1). No Vec conversion needed for key/value extraction. Verification only, no code changes. https://claude.ai/code/session_01JHo5sTCSGPW5NavNE2b49d
…t 11.5) Add a columnar path to render_flat_map that uses unary_fallible directly on Column<(Row, T, Diff)> containers. Iterates &RowRef without allocating owned Rows for expression evaluation. Changed drain_through_mfp to accept &RowRef. Vec fallback retained for arrangement key paths. https://claude.ai/code/session_01JHo5sTCSGPW5NavNE2b49d
…ctly (Prompt 11.6) Add arrange_columnar_collection that takes ColumnarCollection and iterates &RowRef from columnar containers for key/value expression evaluation, avoiding the columnar→Vec conversion. Wire ensure_collections to use it when identity MFP + no input_key + columnar available. The passthrough stream stays columnar throughout the arrangement loop. https://claude.ai/code/session_01JHo5sTCSGPW5NavNE2b49d
Replace Columnar::into_owned with copy_from on reusable buffers in all columnar iteration loops. This avoids allocating new Row/Timestamp/Diff values each iteration, reusing the buffer's existing allocation instead. Affected operators: ColumnarToVec, NegateColumnar, ColumnarFlatMap, FlatMapStageColumnar, FormArrangementKeyColumnar. https://claude.ai/code/session_01JHo5sTCSGPW5NavNE2b49d
Change the flat_map closure signature from (DatumVecBorrow, T, Diff) to (DatumVecBorrow, &T, &Diff). This eliminates unnecessary clones in the columnar path (references to copy_from buffers are passed directly) and in the arrangement path (owned values from buffer.drain are passed by reference). Callers clone/copy only when they actually need ownership. https://claude.ai/code/session_01JHo5sTCSGPW5NavNE2b49d
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
2 participants
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
Completes the columnar rendering migration (Prompts 2.3 through 10.1), building on the foundation from the earlier PR (Prompts 0.1–2.2).
Changes by phase
as_columnar_collection_coremethod; wiredGet::CollectionandMfpto prefer columnar pathensure_collectionshandles columnar-only inputs for arrangement creationcollectionfield fromCollectionBundleentirely. Data flows exclusively throughcolumnar_collection.from_collectionsauto-converts Vec→columnar. Addedas_vec_collection()for on-demand conversion at operator boundaries. Net -50 lines.Key architectural decisions
as_vec_collection()converts columnar→Vec at operator boundaries (arrangements, sinks, etc.)from_collectionsauto-converts: Operators producing Vec output seamlessly convert to columnarDataflowErroris not suited for columnar layoutSkipped
DatumColumn/ColumnDatum/rows_to_columns/MfpPlan::evaluate_batch) which is not present in the codebaseTest plan
cargo check -p mz-computepassescargo check -p mz-compute --testspassescargo clippy -p mz-compute --all-targets— zero warningsbin/fmt(rustfmt) passesrender/columnar.rscover round-trip, negate, union, and constant conversionshttps://claude.ai/code/session_01JHo5sTCSGPW5NavNE2b49d