Skip to content

Make from_arrow not panic#8242

Merged
connortsui20 merged 1 commit into
developfrom
ct/from-arrow
Jun 3, 2026
Merged

Make from_arrow not panic#8242
connortsui20 merged 1 commit into
developfrom
ct/from-arrow

Conversation

@connortsui20
Copy link
Copy Markdown
Contributor

Summary

Removes the assertion in the nulls function so that from_arrow errors instead of panicking if the nullable does not match the actually nullability of the arrow array.

Also documents the FromArrowArray trait and method.

Testing

2 basic unit tests.

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
@connortsui20 connortsui20 added the changelog/chore A trivial change label Jun 3, 2026
@connortsui20 connortsui20 enabled auto-merge (squash) June 3, 2026 19:19
Comment on lines +498 to +504
let null_count = nulls.map(NullBuffer::null_count).unwrap_or(0);
vortex_ensure_eq!(
null_count,
0,
"Cannot convert an Arrow array containing {null_count} nulls into a non-nullable Vortex array"
);
Ok(Validity::NonNullable)
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.

This was the main reason for this change, it doesn't make much sense to assert when basically all callers return results

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Jun 3, 2026

Merging this PR will improve performance by 16.11%

⚠️ 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 improved benchmarks
❌ 1 regressed benchmark
✅ 1271 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation bitwise_not_vortex_buffer_mut[128] 246.1 ns 275.3 ns -10.6%
Simulation chunked_bool_canonical_into[(1000, 10)] 45.2 µs 30.3 µs +49.28%
Simulation chunked_varbinview_into_canonical[(1000, 10)] 212.2 µs 175.8 µs +20.7%
Simulation chunked_varbinview_canonical_into[(100, 100)] 308.3 µs 273.3 µs +12.82%

Tip

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


Comparing ct/from-arrow (555566e) with develop (a958108)

Open in CodSpeed

@connortsui20 connortsui20 merged commit b1e8f76 into develop Jun 3, 2026
76 of 77 checks passed
@connortsui20 connortsui20 deleted the ct/from-arrow branch June 3, 2026 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/chore A trivial change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants