Skip to content

Commit

Permalink
[BUG] Fix I32Array BW compat (#2815)
Browse files Browse the repository at this point in the history
## Description of changes

*Summarize the changes made by this PR.*
 - Improvements & Bug fixes
- In #2729 we changed to UInt32Array but some old data may be
Int32Array. This is a rather ugly hack to preserve that behavior.
 - New functionality
	 - None

## Test plan
*How are these changes tested?*
We are scoping cross-version tests, which are needed in general.
- [x] Tests pass locally with `pytest` for python, `yarn test` for js,
`cargo test` for rust

## Documentation Changes
None
  • Loading branch information
HammadB authored Sep 17, 2024
1 parent b77f73b commit 8412076
Showing 1 changed file with 31 additions and 7 deletions.
38 changes: 31 additions & 7 deletions rust/blockstore/src/arrow/block/value/int32array_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::{
key::KeyWrapper,
};
use arrow::{
array::{Array, ListArray, UInt32Array},
array::{Array, Int32Array, ListArray, UInt32Array},
util::bit_util,
};
use std::{mem::size_of, sync::Arc};
Expand Down Expand Up @@ -50,12 +50,36 @@ impl<'referred_data> ArrowReadableValue<'referred_data> for &'referred_data [u32
let list_array = array.as_any().downcast_ref::<ListArray>().unwrap();
let start = list_array.value_offsets()[index] as usize;
let end = list_array.value_offsets()[index + 1] as usize;
let u32array = list_array
.values()
.as_any()
.downcast_ref::<UInt32Array>()
.unwrap();
&u32array.values()[start..end]

// 9/17 In order to support backwards compatability before #2729 (https://github.com/chroma-core/chroma/pull/2729)
// which had this stored as a Int32Array, we first try to downcast to a UInt32Array and then if that fails
// we downcast to a Int32Array, if that fails we panic.
let u32array = list_array.values().as_any().downcast_ref::<UInt32Array>();
match u32array {
Some(u32array) => &u32array.values()[start..end],
None => {
let i32array = list_array.values().as_any().downcast_ref::<Int32Array>();
match i32array {
Some(i32array) => {
// &i32array.values()[start..end] as &[u32]
// We are forced to use unsafe here because we are casting a &[i32] to a &[u32]
// this is safe as of 9/17 ONLY because we use exclusively positive values here,
// we should introduce versioning to the blockfile
// to ensure that this sort of behavior is only done when needed.
// (Yes this is not great :( )
return unsafe {
std::slice::from_raw_parts(
i32array.values()[start..end].as_ptr() as *const u32,
i32array.values()[start..end].len(),
)
};
}
None => panic!(
"Expected UInt32Array or Int32Array (for legacy reasons) got neither"
),
}
}
}
}

fn add_to_delta<K: ArrowWriteableKey>(
Expand Down

0 comments on commit 8412076

Please sign in to comment.