Skip to content

timely-util: vendor ColumnationStack and merge batcher from differential-dataflow#35760

Merged
antiguru merged 8 commits intoMaterializeInc:mainfrom
antiguru:absorb_batchers
Apr 8, 2026
Merged

timely-util: vendor ColumnationStack and merge batcher from differential-dataflow#35760
antiguru merged 8 commits intoMaterializeInc:mainfrom
antiguru:absorb_batchers

Conversation

@antiguru
Copy link
Copy Markdown
Member

@antiguru antiguru commented Mar 27, 2026 •
edited
Loading

Summary

  • Vendors TimelyStack from differential-dataflow into mz-timely-util as ColumnationStack, along with ColumnationChunker, BatchContainer/BuilderInput/InternalMerge impls, and a new ColumnationMerger that directly implements the Merger trait.
  • ColumnationMerger replaces DD's generic InternalMerger<MC> with a direct Merger impl for ColumnationStack that performs per-item merge and extract with capacity checks.
  • Updates all consumers across mz-compute, mz-storage, mz-storage-operators, and mz-storage-types to import from mz_timely_util::columnation instead of differential_dataflow::containers.
  • Adds a clippy disallowed-types entry for TimelyStack to prevent accidental use of the DD type.

This prepares for removing columnation support from differential-dataflow. The Batcher, Merger, and InternalMerge traits remain in DD — only the columnation-specific implementations and container types are vendored.

Test plan

  • cargo check --all-targets passes
  • cargo test -p mz-timely-util --lib passes
  • cargo test -p mz-storage-types passes
  • CI

🤖 Generated with Claude Code

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 27, 2026

Thanks for opening this PR! Here are a few tips to help make the review process smooth for everyone.

PR title guidelines

  • Use imperative mood: "Fix X" not "Fixed X" or "Fixes X"
  • Be specific: "Fix panic in catalog sync when controller restarts" not "Fix bug" or "Update catalog code"
  • Prefix with area if helpful: compute: , storage: , adapter: , sql:

Pre-merge checklist

  • The PR title is descriptive and will make sense in the git log.
  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).

@antiguru antiguru force-pushed the absorb_batchers branch 2 times, most recently from feb6caf to 396825b Compare March 28, 2026 14:02
@antiguru antiguru requested a review from DAlperin March 31, 2026 12:53
@antiguru antiguru marked this pull request as ready for review March 31, 2026 12:54
@antiguru antiguru requested a review from a team as a code owner March 31, 2026 12:54
@frankmcsherry
Copy link
Copy Markdown
Contributor

frankmcsherry commented Mar 31, 2026

Fwiw, excited to see this popped out of DD and moved over here. :D

@antiguru antiguru force-pushed the absorb_batchers branch 3 times, most recently from 2d76d93 to f1c9497 Compare April 8, 2026 13:18
@antiguru antiguru changed the title timely-util: vendor ColumnationStack from differential-dataflow timely-util: vendor ColumnationStack and merge batcher from differential-dataflow Apr 8, 2026
frankmcsherry
frankmcsherry approved these changes Apr 8, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@frankmcsherry frankmcsherry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very clear, and imo the right thing to do.

antiguru and others added 8 commits April 8, 2026 19:21
@antiguru @claude
vendor ColumnationStack and related types into mz-timely-util
fce7b4c 
Copy TimelyStack from differential-dataflow, renamed to ColumnationStack.
Also vendors ColumnationChunker, the InternalMerge impl for columnation
stacks, and the ColInternalMerger type alias. This prepares for removing
columnation support from differential-dataflow.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@antiguru @claude
update mz-timely-util to use vendored ColumnationStack
35464c6 
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@antiguru @claude
update mz-compute to use vendored ColumnationStack
ff262f6 
Also adds BatchContainer and BuilderInput impls for ColumnationStack
in mz-timely-util, which are needed by differential-dataflow's Layout
and Builder traits.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@antiguru @claude
update mz-storage crates to use vendored ColumnationStack
1f61123 
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@antiguru @claude
update SLT tests, add TimelyStack deny lint, fix doctest
91ecf85 
Update expected type names in relations.slt from
differential_dataflow::containers::TimelyStack to
mz_timely_util::columnation::ColumnationStack.

Add clippy disallowed-types entry for TimelyStack to prevent
accidental use of the DD type instead of our vendored copy.

Fix doctest type inference failure caused by new DrainContainer impl.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@antiguru @claude
fix import ordering and add safety comment
1ba8c5a 
Move crate::columnation import after external crate imports in
columnar.rs. Add safety justification to unsafe block in
ColumnationStack::retain_from.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@antiguru @claude
update InternalMerge::extract to DD 0.22 position-based signature
ce15fa2 
DD 0.22 added a position parameter to InternalMerge::extract with
per-item capacity checks, fixing the sizing bug. Use DD's InternalMerger
directly instead of a custom ColumnationMerger.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@antiguru
Formatting
324aa43 
Signed-off-by: Moritz Hoffmann <[email protected]>
@antiguru antiguru enabled auto-merge (squash) April 8, 2026 17:22
@antiguru antiguru merged commit f498b6e into MaterializeInc:main Apr 8, 2026
118 checks passed
@antiguru antiguru deleted the absorb_batchers branch April 8, 2026 17:57
mgree pushed a commit to mgree/materialize that referenced this pull request Apr 13, 2026
@antiguru @claude @mgree
timely-util: vendor ColumnationStack and merge batcher from different…
219c32c 
…ial-dataflow (MaterializeInc#35760)

## Summary

- Vendors `TimelyStack` from differential-dataflow into `mz-timely-util`
as `ColumnationStack`, along with `ColumnationChunker`,
`BatchContainer`/`BuilderInput`/`InternalMerge` impls, and a new
`ColumnationMerger` that directly implements the `Merger` trait.
- `ColumnationMerger` replaces DD's generic `InternalMerger<MC>` with a
direct `Merger` impl for `ColumnationStack` that performs per-item merge
and extract with capacity checks.
- Updates all consumers across `mz-compute`, `mz-storage`,
`mz-storage-operators`, and `mz-storage-types` to import from
`mz_timely_util::columnation` instead of
`differential_dataflow::containers`.
- Adds a clippy `disallowed-types` entry for `TimelyStack` to prevent
accidental use of the DD type.

This prepares for removing columnation support from
differential-dataflow. The `Batcher`, `Merger`, and `InternalMerge`
traits remain in DD — only the columnation-specific implementations and
container types are vendored.

## Test plan

- [x] `cargo check --all-targets` passes
- [x] `cargo test -p mz-timely-util --lib` passes
- [x] `cargo test -p mz-storage-types` passes
- [ ] CI

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Signed-off-by: Moritz Hoffmann <[email protected]>
Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@teskje teskje teskje approved these changes

@frankmcsherry frankmcsherry frankmcsherry approved these changes

@DAlperin DAlperin Awaiting requested review from DAlperin

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@antiguru @frankmcsherry @teskje