Hardcode constant detection into the cascading compressor#8667
Merged
Conversation
The five per-type constant schemes (bool/int/float/string/binary) were "half builtin": they lived in vortex-compressor's builtins module but had to be registered by downstream crates for constant arrays to be detected. Constant handling was also scattered across three places: the compressor's own all-null short-circuit, the per-type constant schemes, and an inline is_constant check in TemporalScheme. Constant detection is now built directly into CascadingCompressor. A new crate-private `constant` module detects constant leaves using the cheapest available evidence per type (distinct counts when another scheme already requested them, max-min for integers, bool stats, and a vectorized is_constant scan otherwise) and encodes them as ConstantArray, wrapped in MaskedArray when nulls are present. The check runs after the empty/all-null short-circuits and before any scheme evaluation, and is skipped while compressing samples since a constant sample does not imply a constant array. Changes in behavior: - Constant detection now covers ALL leaf types uniformly, adding decimal and extension arrays. Constant extension arrays (e.g. timestamps) short-circuit to a top-level ConstantArray instead of competing with storage compression by size, which lets TemporalScheme drop its inline is_constant check. - Constant detection is unconditional: it applies even for a compressor constructed with an empty scheme list, and can no longer be disabled via exclude_schemes. - Constant string/binary arrays no longer pay for dict/FSST sampling before the (previously deferred, last-registered) constant check resolves; the constant pre-pass runs before all scheme estimation. The old scheme-based path found the first valid element with a per-index is_valid loop that re-derived validity on every call; the new compress_constant materializes the validity mask once and uses Mask::first. Signed-off-by: Connor Tsui <connor@spiraldb.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RLaoUmBcuLPmkxrwcPAEgZ Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
7574535 to
561de5b
Compare
Merging this PR will not alter performance
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing Footnotes
|
robert3005
approved these changes
Jul 7, 2026
brancz
pushed a commit
to polarsignals/vortex
that referenced
this pull request
Jul 9, 2026
Scalar serialization writes ScalarValue::Tuple as a protobuf ListValue regardless of whether the dtype is List or FixedSizeList, but list_from_proto only accepted List. Since vortex-data#8667 made constant detection unconditional in the cascading compressor, a constant FixedSizeList column (e.g. a UUID stored as fixed_size_list(u8)[16]) is written as a ConstantArray whose scalar metadata then fails to deserialize on read: expected List dtype for ListValue, got fixed_size_list(u8)[16] Scalar validation already handles FixedSizeList + Tuple (including the size check), so deserialization was the only gap. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Frederic Branczyk <frederic.branczyk@polarsignals.com>
brancz
added a commit
to polarsignals/vortex
that referenced
this pull request
Jul 9, 2026
Scalar serialization writes ScalarValue::Tuple as a protobuf ListValue regardless of whether the dtype is List or FixedSizeList, but list_from_proto only accepted List. Since vortex-data#8667 made constant detection unconditional in the cascading compressor, a constant FixedSizeList column (e.g. a UUID stored as fixed_size_list(u8)[16]) is written as a ConstantArray whose scalar metadata then fails to deserialize on read: expected List dtype for ListValue, got fixed_size_list(u8)[16] Scalar validation already handles FixedSizeList + Tuple (including the size check), so deserialization was the only gap. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Frederic Branczyk <fbranczyk@gmail.com>
brancz
added a commit
to polarsignals/vortex
that referenced
this pull request
Jul 9, 2026
Scalar serialization writes ScalarValue::Tuple as a protobuf ListValue regardless of whether the dtype is List or FixedSizeList, but list_from_proto only accepted List. Since vortex-data#8667 made constant detection unconditional in the cascading compressor, a constant FixedSizeList column (e.g. a UUID stored as fixed_size_list(u8)[16]) is written as a ConstantArray whose scalar metadata then fails to deserialize on read: expected List dtype for ListValue, got fixed_size_list(u8)[16] Scalar validation already handles FixedSizeList + Tuple (including the size check), so deserialization was the only gap. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Frederic Branczyk <fbranczyk@gmail.com>
brancz
added a commit
to polarsignals/vortex
that referenced
this pull request
Jul 9, 2026
Scalar serialization writes ScalarValue::Tuple as a protobuf ListValue regardless of whether the dtype is List or FixedSizeList, but list_from_proto only accepted List. Since vortex-data#8667 made constant detection unconditional in the cascading compressor, a constant FixedSizeList column (e.g. a UUID stored as fixed_size_list(u8)[16]) is written as a ConstantArray whose scalar metadata then fails to deserialize on read: expected List dtype for ListValue, got fixed_size_list(u8)[16] Scalar validation already handles FixedSizeList + Tuple (including the size check), so deserialization was the only gap. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Frederic Branczyk <fbranczyk@gmail.com>
connortsui20
pushed a commit
that referenced
this pull request
Jul 9, 2026
…8696) Scalar serialization writes `ScalarValue::Tuple` as a protobuf `ListValue` for both `List` and `FixedSizeList` dtypes, but `list_from_proto` only accepted `List`. Since #8667 made constant detection unconditional, a constant fixed-size list column (e.g. UUIDs stored as `fixed_size_list(u8)[16]`) is compressed to a `ConstantArray` whose scalar metadata then fails to deserialize on read: expected List dtype for ListValue, got fixed_size_list(u8)[16] so files written this way come back unreadable. Scalar validation already handles `FixedSizeList` + `Tuple` (including the size check), so deserialization was the only gap. Includes a proto round-trip test that fails without the fix. --------- Signed-off-by: Frederic Branczyk <fbranczyk@gmail.com> Co-authored-by: Claude Fable 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.
Summary
Closes: #8666
The five per-type constant schemes (bool/int/float/string/binary) were "half builtin": they lived in
vortex-compressor'sbuiltinsmodule but still had to be registered. Constant handling was also scattered across three places: the compressor's own all-null short-circuit, the per-type constant schemes, and an inlineis_constantcheck inTemporalScheme.Constant detection is now built directly into
CascadingCompressor. A new crate-privateconstantmodule detects constant leaves using the cheapest available evidence per type.Changes in behavior:
exclude_schemes. I think this is fine because it is essentially never a good idea to not check for a constant array (except maybe for tiny arrays, but I don't think we need to worry about that), and we have no code that tries to exclude constant array.is_validloop that re-derived validity on every call; the newcompress_constantfunction materializes the validity mask once and usesMask::first.