Skip to content

compute: store accumulable reduce accumulators in columnar form - #38718

Draft
frankmcsherry wants to merge 2 commits into
MaterializeInc:mainfrom
frankmcsherry:columnar-accum
Draft

compute: store accumulable reduce accumulators in columnar form#38718
frankmcsherry wants to merge 2 commits into
MaterializeInc:mainfrom
frankmcsherry:columnar-accum

Conversation

@frankmcsherry

@frankmcsherry frankmcsherry commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Motivation

The accumulable reduce (sum, count, any, all) keeps one Accum per aggregate in the diff of its input arrangement, as (Vec<Accum>, Diff). Accum is an enum whose Numeric variant carries a 64 byte Decimal<27> plus four counters, and whose other variants hold an i128, so every slot is 112 bytes regardless of which variant it holds. A count or integer sum needs 24 of those bytes. Reduces with many groups over integer sums pay for decimals they never use.

Description

Accum now derives Columnar, and the "ArrangeAccumulable" arrangement stores its diffs in differential's Coltainer<R>, the columnar BatchContainer, instead of a columnation stack. The derived container keeps one set of columns per variant plus a discriminant, so each accumulator occupies only its own variant's fields. Accum itself, its Semigroup/Multiply arithmetic, and the output arrangement are unchanged.

Supporting pieces, each small:

  • mz_repr::adt::numeric::OrderedNumericAgg is a newtype over NumericAgg with OrderedDecimal's order. The orphan rule blocks a Columnar impl on OrderedDecimal<NumericAgg>, so this type hosts one. Its container stores the decimal's raw parts as four columns, and its reference type is the owned value so comparisons on references keep decimal order. The coefficient column is one unit wider than the decimal (56 bytes, whole u64 words) because columnar's indexed byte store pads columns to words and cannot decode an element width that does not divide that padding.
  • Overflowing<T>: Columnar in mz_ore now reuses T's own columnar container rather than requiring &[T] to be castable to bytes. That gives Overflowing<i128>, the accumulator count type, a Columnar impl through columnar's byte-encoded i128 store. Diff is Overflowing<i64>, whose container still resolves to Vec<i64>, so arrangements on the existing columnar batcher paths see no layout change.
  • RowLayout, RowSpine, RowBuilder (and compute's RowAgent) gain a defaulted diff-container type parameter. The default is the existing columnation stack, so no other call site changes.
  • mz_timely_util::containers::HeapSize reports heap allocations for a batch container, implemented for ColumnationStack and Coltainer, and the RowSpine arrangement size logging is generic over the diff container. Each columnar column is one Vec, so the allocation count stays meaningful; columnar exposes no spare capacity, so the reported capacity for columnar diffs is a lower bound.

The batcher still stages updates in columnation chunks before they reach the arrangement, so Accum keeps its Columnation impl and its full width in that transient stage.

Measurements

Local, bin/environmentd --optimized, one worker. A table of 2M rows with 2M distinct keys, indexed as SELECT k, count(*), sum(int4), sum(int8), sum(int4), sum(float8) ... GROUP BY k. mz_arrangement_sizes for ArrangeAccumulable [val: empty], which is where the diffs live:

build records size capacity bytes/record
main 2,000,000 1.21 GB 1.98 GB (bulk) / 1.22 GB (incremental) 608
this PR 2,000,000 436 MB 445 MB 221

The 387 bytes saved per record are five 112 byte slots becoming four 24 byte SimpleNumber payloads and one 48 byte Float payload, plus the per-element discriminant this mixed-variant plan needs. The ReduceAccumulable output arrangement is byte-identical in both runs. Plans whose aggregates are all one variant, for example only count and integer sum, pay no per-element discriminant at all.

clusterd RSS after loading was 21% lower for bulk hydration (2.44 GB vs 3.10 GB) and 8% lower for 20 incremental inserts of 100k rows (3.09 GB vs 3.35 GB). RSS understates the arrangement saving here because the batcher's transient copies are the same on both builds and the macOS system allocator holds on to that peak.

Tests

  • mz_ore: test_columnar_i128_round_trip pushes Overflowing<i128> extremes through the container and back through bytes.
  • mz_repr: ordered_numeric_agg_columnar_round_trip does the same for decimals including NaN, infinities, and values at both precision extremes, and also decodes through columnar's indexed store, which fails without the widened coefficient column.
  • mz_compute: accum_columnar_round_trip pushes accumulators of every variant, in zero, accumulated, and negated states, through Accum's derived container and through Coltainer<(Vec<Accum>, Diff)> as a BatchContainer, checking round trips, that reference ordering agrees with owned ordering on every pair, and byte and indexed-store decoding.
  • Existing sqllogictest coverage of accumulable aggregates (aggregates.slt, numeric.slt, float.slt, reduce_mfp.slt) exercises the new arrangement end to end.

Follow-ups, not in this PR

  • A key-only Row builder that consumes Column chunks (the key/value spines already have RowRowColPagedBuilder) would let the batcher stage Accum columnar too and drop its Columnation impl.
  • The derived discriminant stores a u64 offset per element when variants are mixed; a fixed-arity diff container, or narrower offsets upstream, would remove that.
  • Coltainer::with_capacity ignores its size hint, so the builder path grows the diff columns by doubling; a reserve on columnar::Container would fix this upstream.
  • The sibling RowVal/RowRow/ValRow layouts could take the same diff-container parameter when a consumer appears.

No user-visible behavior change.

🤖 Generated with Claude Code

The accumulable reduce keeps one `Accum` per aggregate in the diff of its
input arrangement. `Accum` is an enum sized for its `Numeric` variant, so
every slot costs 112 bytes even when it holds a 24 byte `count` or integer
`sum`. Deriving `Columnar` for `Accum` and storing the arrangement's diffs
in differential's `Coltainer` lays the accumulators out by variant, so each
one pays only for its own fields.

Supporting changes: `OrderedNumericAgg` in `mz_repr` hosts the `Columnar`
impl the orphan rule forbids on `OrderedDecimal<NumericAgg>`;
`Overflowing<T>: Columnar` reuses `T`'s container so `Overflowing<i128>`
works; `RowLayout`, `RowSpine`, `RowBuilder` and `RowAgent` take a
defaulted diff-container parameter; a `HeapSize` trait in `mz_timely_util`
lets arrangement size logging cover both container kinds.

Measured on 2M keys with count, three integer sums and one float sum, the
accumulable arrangement shrinks from 1.21 GB to 436 MB (608 to 221 bytes
per record). The batcher still stages `Accum` in columnation chunks, so its
`Columnation` impl remains.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@antiguru antiguru left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this looks good. Minor point, which we can gain confidence on by testing, is that we unconditionally switch to the new implementation.

@antiguru antiguru left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed at 47385cd. Reusing columnar's derive is the right direction here, and the defaulted diff-container parameter on RowLayout/RowSpine/RowBuilder is a clean way to get there without touching other call sites.

Blocking

doctests is the build's only failure. The new doc line at src/row-spine/src/lib.rs:80 links [RowLayout], which is private, and -D rustdoc::private-intra-doc-links rejects it. Unbracketing the reference clears it.

Strong suggestions

No dyncfg gate. This switches the diff container for every accumulable reduce in one release, with no lever short of a revert. Columnar being the destination argues for the direction, not against gating the transition. #38716 wires an equivalent flag through dyncfgs.rs, get_variable_system_parameters, FlipFlagsAction and KNOWN_MISSING_FROM_LD in about fifteen lines, and that also buys CI randomization of both paths.

No test drives Accum through its own container. The two new tests cover the leaves and are well aimed, since Overflowing<i128> and the decimal's raw parts are exactly the two impls written by hand. Nothing exercises the composed enum, Coltainer<Accum> as a BatchContainer, or Semigroup and IsZero agreement across the container boundary. #38716's accums_agree_with_vec_accum is the shape worth borrowing, as it applies seeded random plus_equals and multiply sequences to a reference and compares at every step. Here the reference is Accum itself, so the test is cheap to write.

Coltainer::index reborrows the whole container per read. Differential's src/columnar/layout.rs:101 is self.container.borrow().get(index), and the returned item borrows that temporary, so it cannot be hoisted out of a merge loop. For Accum the borrow materializes thirteen fields across four variants plus the discriminant's two slices and the i128 stores, against a pointer offset for ColumnationStack. This is upstream behavior rather than anything in the change, but Accum is close to the worst shape for it and the cost lands on the merge and consolidation path. The AccumulateReductions feature benchmark has the right plan shape and reports wall clock alongside memory, so a run with and without would close the one axis the measurements do not cover.

mz_arrangement_sizes.allocations changes meaning for this arrangement. Its column comment is "The number of separate memory allocations backing the arrangement", and HeapSize for Coltainer fires one callback per byte slice while log_arrangement_size counts a callback with non-zero capacity as an allocation. The column therefore reports a column count, inflated roughly twentyfold for this container. Reporting size as capacity is defensible, since spare capacity is faulted lazily and serialized storage would make the two equal by definition. The allocation count is a different matter. If columnar exposes no per-allocation walk, the HeapSize doc comment is the right place to say so and name the effect on that column.

Nits

Vec<[u16; NUMERIC_AGG_WIDTH_USIZE]> will not survive the indexed-store path. Twenty-seven u16 is 54 bytes. columnar's FromBytes for &[[T; N]] reads from a word-padded &[u64] through bytemuck::cast_slice, which requires the byte length to divide 54, and its trim term is ((8 - tail) % 8) / 54, which is always zero and so cannot strip the word padding either. from_bytes is unaffected because as_bytes hands over exactly 54 bytes per element, which is why the round-trip test passes, and the i128 store is 16 bytes per element and fine. Nothing calls from_store today, so this is latent, but it sits on the serialized-data path. Widening the column to [u16; 28] makes it 56 bytes with no padding and removes both problems for two bytes per numeric accumulator, and reporting it upstream with a TODO at the type would work too.

from_store and element_sizes are untested in both new impls, for the same reason.

OrderedNumericAgg sits beside OrderedDecimal in mz_repr as a second ordered-decimal type. Orphan rules force it and the doc comment says so. A Deref<Target = NumericAgg> or a From pair would stop call sites reaching for .0.

Reference and owned Ord agree, because Overflowing<T> and OrderedNumericAgg both use the owned value as their reference type, so the derived reference ordering matches field for field. That is load-bearing and invisible, as differential's for<'a> Ref<'a, C>: Ord bound checks that an order exists rather than that it agrees. A future field whose reference type differs from its owned type would break it silently, which is worth a line at the #[columnar(derive(...))] attribute.

Verified, no action needed

  • The homogeneous-plan claim holds. Discriminant::push keeps offset = [tag, count] with an empty variant column while every push shares a variant, so a plan of only count and integer sum pays sixteen bytes per container rather than nine per element. That is the common shape and the strongest property this layout has over a per-slot tag byte.
  • Generalizing Overflowing<T>: Columnar to delegate to T::Container is the right shape, and Overflows is named nowhere outside overflowing.rs, so dropping its TC = Vec<T> default breaks no caller.
  • Diff is Overflowing<i64>, whose container still resolves to Vec<i64>, so the other arrangements on the columnar batcher paths see no layout change. That is worth stating outright in the description, since it is the reassurance a reviewer of that hunk needs and "primitive cases are unchanged" does not quite give it.

One bound worth keeping visible. The batcher still stages Accum at full width, which is why 64 percent off the arrangement is 21 percent off bulk-hydration RSS. It is already in the follow-ups, and it caps the peak-memory story until the key-only builder that consumes Column chunks lands.

🤖 Posted by Claude Code

Unbracket the `RowLayout` reference in `RowSpine`'s doc, which is a private
item and failed the rustdoc lint. Write the `RowSpine` arrangement-size impl
over the diff container alone, since the diff type is its `Owned` type.

Widen the numeric accumulator's coefficient column from 27 to 28 units so
each element is a whole number of `u64` words. columnar's indexed byte
store pads columns to words, and its array decoder cannot strip padding
whose width does not divide the element width, so the 54 byte column
panicked on decode. The round-trip tests now also decode through the
indexed store, which fails without the widening.

Add a test that drives `Accum` of every variant through its derived
container and through `Coltainer<(Vec<Accum>, Diff)>`, checking round
trips, byte decoding, and that reference ordering agrees with owned
ordering, which the columnar diff container relies on. Note that
agreement at the derive, and note what `HeapSize` reports for columnar
containers.

Co-Authored-By: Claude Fable 5.1 <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