compute: Emit TopK monotonic and basic output as the columnar edge - #37783
Open
antiguru wants to merge 3 commits into
Open
compute: Emit TopK monotonic and basic output as the columnar edge#37783antiguru wants to merge 3 commits into
antiguru wants to merge 3 commits into
Conversation
antiguru
force-pushed
the
columnar-p6-topk-output
branch
from
July 22, 2026 08:41
ddd570d to
c790948
Compare
antiguru
force-pushed
the
columnar-p6-topk-output
branch
from
July 22, 2026 16:24
c790948 to
31a84a6
Compare
antiguru
force-pushed
the
columnar-p6-topk-output
branch
from
July 22, 2026 17:51
31a84a6 to
2f27548
Compare
antiguru
force-pushed
the
columnar-p6-topk-output
branch
from
August 19, 2026 12:01
2f27548 to
1536039
Compare
antiguru
force-pushed
the
columnar-p6-topk-output
branch
from
August 19, 2026 13:17
1536039 to
15eb779
Compare
antiguru
force-pushed
the
columnar-p6-topk-output
branch
from
August 20, 2026 08:49
15eb779 to
10e7a85
Compare
antiguru
force-pushed
the
columnar-p6-topk-output
branch
from
August 20, 2026 09:13
10e7a85 to
8ee12e6
Compare
antiguru
force-pushed
the
columnar-p6-topk-output
branch
from
September 6, 2026 18:23
8ee12e6 to
40d648a
Compare
antiguru
commented
Sep 10, 2026
Comment on lines
324
to
326
| let (oks, errs) = self.build_topk( | ||
| ok_input, group_key, order_key, offset, limit, arity, buckets, | ||
| ); |
Member
Author
There was a problem hiding this comment.
Note: We should remember to push columnar containers further down into top-k.
Member
Author
There was a problem hiding this comment.
Recorded as a TODO on topk_result_to_columnar, the seam that owns the encode:
/// TODO: TopK renders its stages over `Vec` containers, so this encode sits at
/// the very end of the plan. Pushing columnar containers down through
/// `build_topk` and the monotonic path would remove it.
Both TopK paths reach the edge through that one function, the monotonic arm directly and the basic arm via build_topk, so a single note covers the whole operator rather than one per call site.
Doing the push-down itself needs the same treatment mz_join_core got in #38368: build_topk's stages chain through arrangements, so only the last stage can write the output edge, and the fallible-limit arm materializes rows before the demux. Happy to take it as a follow-up PR on top of the stack if you want it before this lands.
Posted by Claude Code
antiguru
marked this pull request as ready for review
September 10, 2026 11:29
antiguru
force-pushed
the
columnar-p6-topk-output
branch
from
September 10, 2026 11:33
40d648a to
8def045
Compare
Flip the two TopK plans that build a result collection (MonotonicTopK and Basic) to produce a `CollectionEdge::Columnar` via a shared `topk_result_to_columnar` helper. The bucketed MonotonicTop1 plan emits an arrangement and is materialized by the shared `as_specific_collection` path, so it is untouched. Both sites drop the hash-key pairing from a `consolidate_named`-consolidated `(hash_key, row)` collection. The hash key is a function of the row, so the drop is injective and the output has no within-batch duplicates. A non-consolidating `ColumnBuilder` therefore matches the prior `map` semantics, and the row is pushed borrowed. The err collection stays `Vec`, and the intra-operator machinery stays on `Vec`. Also corrects a stale comment on `map_topk_key`: its columnar arm runs once the input edge is columnar, which the upstream producers now emit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment-only, no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
TopK renders its stages over `Vec` containers, so the columnar output edge is produced by a single encode at the end of the plan. Note at that seam that the encode disappears once the stages themselves carry columnar containers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
antiguru
force-pushed
the
columnar-p6-topk-output
branch
from
September 10, 2026 12:09
23a66b9 to
b1c836e
Compare
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.
Monotonic and basic TopK emit their
from_collectionsoutput as the columnar edge. Bucketed TopK is already covered by the shared arrangement-materialization path (P5).Columnar dataflow-edge migration. Design doc:
doc/developer/design/20260720_columnar_dataflow_edges.md(#37744).Part of CPU-51.