Skip to content

Add a scan for the range of valid decimal values - #10939

Merged
connortsui20 merged 3 commits into
developfrom
ct/decimal-valid-range
Oct 28, 2026
Merged

Add a scan for the range of valid decimal values#10939
connortsui20 merged 3 commits into
developfrom
ct/decimal-valid-range

Conversation

@connortsui20

@connortsui20 connortsui20 commented Oct 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

Decimal Widen, legacy decimal reads and DecimalByteParts splitting need raw extrema before choosing or checking precision. The existing decimal min/max implementation constructs checked scalars, so callers cannot use it directly for values outside the declared precision.

Changes

Expose the raw range from the existing pairwise scanner in aggregate_fn/fns/min_max/decimal.rs. Both the aggregate and new raw-range callers use that implementation. Scalar construction stays in the aggregate layer, and the raw helper ignores null slots without allocating or checking precision.

Tests cover all six storage widths, native extrema, null payloads, empty and all-null inputs, and aggregate result dtypes. Benchmarks add wide decimal cases to the existing min/max target. Checks are left to CI.

API Changes

Add public vortex_array::arrays::decimal::valid_range, re-exported from the existing decimal min/max implementation.

@connortsui20
Add a scan for the range of valid decimal values
d8f5edf 
Share one allocation-free range scan between decimal Widen encoding,
legacy decimal reads and DecimalByteParts lower-part selection. Ignore
null slots and avoid scalar construction or precision checks.

Test all six decimal widths, empty and all-null inputs, null-slot extrema
and values outside decimal precision.

Signed-off-by: Connor Tsui <[email protected]>
@connortsui20 connortsui20 added the changelog/feature A new feature label Oct 28, 2026
@connortsui20 connortsui20 mentioned this pull request Oct 28, 2026
Open
20 of 34 tasks
@codspeed

codspeed Bot commented Oct 28, 2026 •
edited
Loading

Copy link
Copy Markdown

Merging this PR will regress 1 benchmark

4 warnings

⚠️ Walltime results may be unreliable

Some benchmarks used the Walltime instrument outside CodSpeed Macro Runners, which can make the results inconsistent.

Learn more about Macro Runners

See affected benchmarks

⚠️ Unable to measure the execution of 1 benchmark

This benchmark measured no execution time, so its result is not comparable and counts as unchanged. This usually happens when the compiler removes the code under test.

Prevent compiler optimizations

See affected benchmarks

⚠️ 1 benchmark is dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

⚠️ Base and head ran on different runner types

Some benchmarks with significant performance changes were compared across different runner types, so the changes might not come from the code. Run base and head on the same runner type, or use CodSpeed Macro Runners for more stable results.

Learn more about Macro Runners

Open the report in CodSpeed to investigate

⚡ 9 improved benchmarks
❌ 1 regressed benchmark
✅ 2260 untouched benchmarks
🆕 4 new benchmarks
⏩ 370 skipped benchmarks1
🗄️ 30 archived benchmarks run2

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ Walltime mul_u64_nonnull_neon 20.4 µs 39 µs -47.76%
⚡ Walltime threshold_neon[64] 3.8 µs 2.6 µs +44.04%
⚡ Walltime filter_neon[64] 2.7 µs 1.9 µs +43.44%
⚡ Walltime threshold_neon[80] 4.7 µs 3.4 µs +38.32%
⚡ Walltime filter_neon[80] 3.4 µs 2.6 µs +27.19%
⚡ CPU Simulation encode_primitives[u8, (2000, 4)] 164.1 µs 136.2 µs +20.42%
⚡ CPU Simulation test_repeated_scan_scalar_at[int_str] 389.9 µs 338.5 µs +15.19%
⚡ CPU Simulation fsl_i32[16, 0.01] 14.8 µs 12.9 µs +14.51%
⚡ Walltime threshold_avx512[80] 2.8 µs 2.5 µs +13.91%
⚡ CPU Simulation take_filter_list_small_uncached_random_mask_random_indices[768, 10] 118.6 µs 105.4 µs +12.55%
🆕 CPU Simulation min_max_decimal_i128[NonNullable] N/A 418 µs N/A
🆕 CPU Simulation min_max_decimal_i128[ScatteredNulls] N/A 512.3 µs N/A
🆕 CPU Simulation min_max_decimal_i256[NonNullable] N/A 1.1 ms N/A
🆕 CPU Simulation min_max_decimal_i256[ScatteredNulls] N/A 673.9 µs N/A

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

Comparing ct/decimal-valid-range (1ba93b7) with develop (10d2d39)

Open in CodSpeed

Footnotes

  1. 370 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

  2. 30 benchmarks were run, but are now archived. If they were deleted in another branch, consider rebasing to remove them from the report. Instead if they were added back, click here to restore them. ↩

This was referenced Oct 28, 2026
Merged
Merged
@connortsui20
connortsui20 marked this pull request as ready for review October 28, 2026 20:05
@connortsui20
connortsui20 enabled auto-merge (squash) October 28, 2026 20:28
@connortsui20
connortsui20 merged commit 40e0b1c into develop Oct 28, 2026
116 of 118 checks passed
@connortsui20
connortsui20 deleted the ct/decimal-valid-range branch October 28, 2026 20:42
connortsui20 added a commit that referenced this pull request Oct 29, 2026
@connortsui20
Update the Widen and decimal plan for the PRs since 2026-10-26
053f392 
WD1 to WD4, CD3, CD3b and the decimal Interleave kernel merged, and
CD4, CD5 and WD6 are open drafts. The plan now lists CD3b (#10939) and
the Interleave kernel (CD4p, #10940) as dependencies of CD4, and CD3b
as a dependency of CD6 and CD7. CD7 merges right after CD5 (#10905).

D26 moves to CD6, because CD4 (#10946) adds no serialization lowering:
the default writer executes a Widen, and reads of old files are the
first ordinary source of one. The CD6 and WR1 briefs now carry that
choice. D35 no longer makes WD6 wait for WD5. #10908 joins the
follow-up issues, and the plan suggests settling the mechanism of

Signed-off-by: Connor Tsui <[email protected]>
#10748 before WD5.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@robert3005 robert3005 robert3005 approved these changes
@joseph-isaacs joseph-isaacs Awaiting requested review from joseph-isaacs

Assignees

No one assigned

Labels

changelog/feature A new feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@connortsui20 @robert3005