Skip to content

timely-util: vendor ColumnationStack from differential-dataflow#35760

Open
antiguru wants to merge 6 commits intoMaterializeInc:mainfrom
antiguru:absorb_batchers
Open

timely-util: vendor ColumnationStack from differential-dataflow#35760
antiguru wants to merge 6 commits intoMaterializeInc:mainfrom
antiguru:absorb_batchers

Conversation

@antiguru
Copy link
Copy Markdown
Member

Summary

  • Vendors TimelyStack from differential-dataflow into mz-timely-util as ColumnationStack, along with ColumnationChunker, the InternalMerge impl, BatchContainer/BuilderInput impls, and the ColInternalMerger type alias.
  • 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.
  • Fixes a doctest type inference regression in operator.rs caused by the new DrainContainer impl.

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

Stacked on #35674.

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

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

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

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) <noreply@anthropic.com>
antiguru and others added 5 commits April 7, 2026 12:54
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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) <noreply@anthropic.com>
…hecks

The generic InternalMerger from differential-dataflow delegates extract()
to InternalMerge::extract which processes entire buffers without capacity
checks, leading to oversized output containers. This was introduced in
TimelyDataflow/differential-dataflow#689.

Replace it with ColumnationMerger, a direct Merger impl that iterates
items individually with capacity checks after each push — restoring the
pre-#689 behavior. The InternalMerge impl on ColumnationStack is retained
because DD's Builder requires it as a trait bound.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants