Skip to content

fix grouped sum index for listview array#8255

Merged
onursatici merged 1 commit into
developfrom
os/grouped-sum-index
Jun 4, 2026
Merged

fix grouped sum index for listview array#8255
onursatici merged 1 commit into
developfrom
os/grouped-sum-index

Conversation

@onursatici

Copy link
Copy Markdown
Contributor

Summary

grouped sum fallback should index the groups correctly when summing

@onursatici onursatici added the changelog/fix A bug fix label Jun 4, 2026

if validity.value(offset) {
// validity is for the outer list view, so it must be indexed with `i`
if validity.value(i) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think a Mask iterator is far faster than value for every function

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yea but I don't think we have that now. I can add one though separately

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@robert3005 robert3005 changed the title fix grouped sum index fix grouped sum index for listview array Jun 4, 2026
@codspeed-hq

codspeed-hq Bot commented Jun 4, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 23.16%

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

❌ 3 regressed benchmarks
✅ 1504 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation chunked_bool_canonical_into[(1000, 10)] 30.3 µs 45.2 µs -33.03%
Simulation chunked_varbinview_canonical_into[(1000, 10)] 160.8 µs 197 µs -18.38%
Simulation chunked_varbinview_into_canonical[(1000, 10)] 175.6 µs 211.6 µs -17.02%

Tip

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


Comparing os/grouped-sum-index (49004bb) with develop (340d7be)

Open in CodSpeed

@onursatici onursatici merged commit 583b003 into develop Jun 4, 2026
88 of 90 checks passed
@onursatici onursatici deleted the os/grouped-sum-index branch June 4, 2026 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants