feat(reader): extract per-zone SUM fold into reader.compute.ZoneReducer#183
Merged
Conversation
Move the zone-map SUM fold out of the Calcite adapter into a reusable reader-side primitive (ADR 0013 §6, whole-zone tier). ZoneReducer.sum folds the per-zone SUM rows from ScanIterator.columnZoneStats with no data segment decoded, returning null when no zone carries a usable sum so the caller streams instead — identical semantics to the inlined fold it replaces. Lives in reader.compute as the seam a future vortex-compute module extracts: it depends only on the public reader scan API, so the move is mechanical once a second consumer appears. The predicate/residual tier waits on its consumer (Calcite aggregate + WHERE push-down). VortexAggregates now delegates SUM to ZoneReducer; behaviour unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add an overflowed-zone case to ZoneReducerTest: zone maps enabled but one I64 chunk's SUM overflows (Math.addExact drops it), so the zone-map table is present yet a zone carries no usable sum — exercising the fold's bail path distinctly from the absent-table case. Also add WriteOptions.withZoneMaps(boolean), mirroring withGlobalDict / withZstd, so the no-zone-map test reads as WriteOptions.defaults().withZoneMaps(false) instead of a brittle positional constructor call. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
What
Moves the zone-map
SUMfold out of the Calcite adapter into a reusable reader-side primitive —reader.compute.ZoneReducer.sum(column)— the whole-zone tier of ADR 0013 §6 aggregate push-down.ZoneReducer.sumfolds the per-zoneSUMrows fromScanIterator.columnZoneStatswith no data segment decoded, returningnullwhen no zone carries a usable sum (no zone map, or an overflowed zone) so the caller streams instead. Semantics are identical to the inlined fold it replaces;VortexAggregatesnow delegates to it.Why here
reader.computeis the seam a futurevortex-computemodule extracts.ZoneReducerdepends only on the public reader scan API, so the extraction is mechanical once a second consumer (arrow bridge, query façade) lands. The reduce primitive belongs inreader, not the Calcite adapter.Scope
ADR 0013 stays Proposed. The predicate/residual tier (boundary zones a
WHEREpartially selects) is deliberately not built here: it has no consumer yet — Calcite aggregates the whole column. The next increment wires Calcite aggregate +WHEREpush-down (the consumer), then addsZoneReducerpredicate support on top.Tests
ZoneReducerTest(writer module, round-trip): integer sum → exactLong; float sum →Double; no zone map →null.VortexAdapterCoverageTestunchanged and green — Calcite behaviour is identical through the delegation.🤖 Generated with Claude Code