Repository navigation
refactor: simplify compute, remove dead code, fix ComputeKernelBenchmark - #597
Merged
Merged
Conversation
…t path Compute.filteredSum had no caller outside a benchmark: Calcite and the CLI use filteredAggregate, matcher and groupedAggregate, and filteredAggregate already folds a filtered SUM (with MIN/MAX and counts) in one pass. Its 393-line FusedFilterSum kernel goes, with the dict lane's sum-only loops (DictFilter.tryFilteredSum, sumLong, sumDouble) and the helpers only they used (NumericColumns.requireNumeric, isFloating). The dict lane also counted COUNT(*) with its own three-shape scan (tryFilteredCount, count): it is now the long fold with no aggregate column, so one scan serves both. DictFilterTest's oracle runs through filteredAggregate and also checks the COUNT(*) count, which had no test. ComputeKernelBenchmark's fusedFilteredSumAlp becomes fusedFilteredAggregateAlp on the same workload; the dict sum-only method goes, as fusedFilteredAggregateDict measures that lane. The compute package drops from 3,380 to 2,687 lines. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…up cleans up Setup asserted 100 chunks, one per writeChunk batch, but the writer has repartitioned into Rust-sized chunks since #470 (763 here), and it asserted measure decodes to a LazyForLongArray, which it no longer does. Every fork failed in setup. It now checks the total row count and asserts only the two filter columns' encodings (ALP price, dict category), the ones that pick a kernel lane. JMH skips the teardown of a failed setup, so each failed fork left its 1.2 GB file in the temp directory: 62 had piled up (74 GB). A failed setup now runs the teardown itself. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Nothing in the project read these:
- WriteOptions.compressionRatioThreshold: always 0.90, copied through every
withXxx and never consulted by the writer, a knob that did nothing
- the TUI's progress parameter ("unused; kept for API stability") and its
two show overloads nothing called, with TuiCommand's progress bar built
only to be passed in; the shallow tree build has no peeks to report
- LeBitReader.byteOffset, ScanIterator.ChunkSpec.sliceOffsetFor and
DictColumnState.chunkValidity(int), reached only from tests or not at all
- DictEncodingEncoder.encodeUtf8Cascade's unused ctx, a swap assignment in
PcoHistogram.medianOfThree that is never read, two pattern variables
named "ignored" (now _)
- in tests, thirteen copies of an unused private scalar(byte[]) helper and
an unused expected-top-value local in ArrayStatsTest
The tests that observed byteOffset and sliceOffsetFor now check the same
invariants through the next read and the record's own components.
docs/reference.md drops the threshold and no longer calls WriteOptions a
record.
Co-Authored-By: Claude Sonnet 5 <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
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.
Three commits:
Compute.filteredSum(no caller outside a benchmark;filteredAggregatealready folds a filtered SUM) with its 393-lineFusedFilterSumkernel, the dict lane's sum-only loops, and its separateCOUNT(*)scan (now the long fold with no aggregate column). The package goes from 3,380 to 2,687 lines;DictFilterTestnow also checksCOUNT(*), which had no test.ComputeKernelBenchmarkfailed in every fork's setup (it asserted 100 chunks; the writer repartitions into 763 since Writer: repartition columns before compressing, as Rust's write strategy does #470), and JMH skips teardown after a failed setup, so each fork left a 1.2 GB temp file. Setup now checks the total row count and cleans up on failure.WriteOptions.compressionRatioThreshold, the TUI's unused progress parameter and overloads, test-onlyLeBitReader.byteOffset/ChunkSpec.sliceOffsetFor, an unusedDictColumnStateaccessor, 13 copies of an unused test helper../mvnw verifygreen locally (3043 integration tests). No CHANGELOG entry: dead-API removal.🤖 Generated with Claude Code