fix(array): don't panic on unsupported arrow types#8564
Conversation
`DType::from_arrow` hit `unimplemented!` for unsupported arrow types (duration, interval, fixed-size binary), surfacing as a panic. add a fallible `TryFromArrowType` conversion that returns a `VortexError`, and route the session and python entry points through it so unsupported types raise a clean `ValueError` instead. Closes vortex-data#8346 Signed-off-by: Han Damin <miniex@daminstudio.net>
AdamGS
left a comment
There was a problem hiding this comment.
This seems good, just running CI to see if everything else is good.
Merging this PR will not alter performance
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | chunked_varbinview_canonical_into[(1000, 10)] |
154.8 µs | 190.9 µs | -18.93% |
| ❌ | Simulation | slice_empty_vortex |
310 ns | 368.3 ns | -15.84% |
| ⚡ | Simulation | chunked_varbinview_into_canonical[(1000, 10)] |
204.4 µs | 168.7 µs | +21.17% |
| ⚡ | Simulation | bitwise_not_vortex_buffer_mut[128] |
273.6 ns | 244.4 ns | +11.93% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing miniex:fix/arrow-unsupported-dtype (136c0a6) with develop (4668b6e)
Footnotes
-
4 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. ↩
|
|
Summary
DType::from_arrowhit theunimplemented!fallback arm for unsupported Arrow types (Duration,Interval,FixedSizeBinary), which surfaced in Python as aPanicExceptioninstead of a clean error. I added a fallibleTryFromArrowTypeconversion that returns aVortexErroron that arm, kept the infallibleFromArrowTypeas a thin delegate for existing trusted callers, and routed theArrowSessionleaf path and the Python entry points through it so unsupported types raise a cleanValueError.The error keeps the
not supportedmarker so it lands in the fuzzer's existing unsupported-type skip path; theDuration/FixedSizeBinarygeneration exclusions can be lifted once the fuzzer is ondevelop.Closes: #8346
Testing
Added an rstest in
vortex-array/src/dtype/arrow.rscheckingDuration/Interval/FixedSizeBinaryerror cleanly as a leaf and nested in a field/schema, plus a parametrized regression test invortex-python/test/test_array.pyfor the issue's repro; both of the issue's Python snippets now raiseValueErrorinstead of panicking.cargo nextest run -p vortex-array(3060 tests) anduv run --all-packages pytest vortex-python/test/test_array.pypass;fmt --all,clippy --all-targets --all-features,cargo doc,basedpyrightandruffare clean.I'm Korean, so sorry if any wording reads a little awkward.