Skip to content

Commit

Permalink
VarBin to arrow conversion uses reinterpret cast to convert offsets t…
Browse files Browse the repository at this point in the history
…o I32/64 (#1644)

In cases where the cast would have failed we will end up with an invalid
array and fail at some point down the line. It's unclear if we should
even allow U32 offsets that are longer than I32 since that's not arrow
compatible.
  • Loading branch information
robert3005 authored Dec 11, 2024
1 parent a7f6f6b commit 60b4978
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions vortex-array/src/array/varbin/arrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ pub(crate) fn varbin_to_arrow(varbin_array: &VarBinArray) -> VortexResult<ArrayR
.map_err(|err| err.with_context("Failed to canonicalize offsets"))?;
let offsets = match offsets.ptype() {
PType::I32 | PType::I64 => offsets,
PType::U64 => try_cast(offsets, PType::I64.into())?.into_primitive()?,
PType::U32 => try_cast(offsets, PType::I32.into())?.into_primitive()?,
PType::U64 => offsets.reinterpret_cast(PType::I64),
PType::U32 => offsets.reinterpret_cast(PType::I32),

// Unless it's u64, everything else can be converted into an i32.
_ => try_cast(offsets.to_array(), PType::I32.into())
Expand Down

0 comments on commit 60b4978

Please sign in to comment.