Skip to content

Commit

Permalink
review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
sanketkedia committed Nov 14, 2024
1 parent 6fb817e commit 540f528
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 52 deletions.
20 changes: 0 additions & 20 deletions rust/blockstore/src/arrow/block/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,26 +346,6 @@ impl Block {
}
}

pub fn get_all_data<'me, K: ArrowReadableKey<'me>, V: ArrowReadableValue<'me>>(
&'me self,
) -> Vec<(&'me str, K, V)> {
let prefix_arr = self
.data
.column(0)
.as_any()
.downcast_ref::<StringArray>()
.unwrap();
let mut result = Vec::new();
for i in 0..self.data.num_rows() {
result.push((
prefix_arr.value(i),
K::get(self.data.column(1), i),
V::get(self.data.column(2), i),
));
}
result
}

/// Get all the values for a given prefix & key range in the block
/// ### Panics
/// - If the underlying data types are not the same as the types specified in the function signature
Expand Down
19 changes: 0 additions & 19 deletions rust/blockstore/src/arrow/blockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -682,25 +682,6 @@ impl<'me, K: ArrowReadableKey<'me> + Into<KeyWrapper>, V: ArrowReadableValue<'me

true
}

pub async fn get_all_data(&'me self) -> Vec<(&'me str, K, V)> {
let block_ids = self.root.sparse_index.get_all_block_ids();
let mut result = vec![];
for block_id in block_ids {
let block = match self.get_block(block_id).await {
Ok(Some(block)) => block,
Ok(None) => {
continue;
}
Err(_) => {
continue;
}
};

result.extend(block.get_all_data());
}
result
}
}

#[cfg(test)]
Expand Down
4 changes: 0 additions & 4 deletions rust/blockstore/src/arrow/sparse_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,10 +321,6 @@ impl SparseIndexReader {
get_target_block(search_key, forward).id
}

pub(super) fn get_all_block_ids(&self) -> Vec<Uuid> {
self.data.forward.values().map(|v| v.id).collect()
}

/// Get all the block ids that contain keys in the given input search keys
pub(super) fn get_all_target_block_ids(&self, mut search_keys: Vec<CompositeKey>) -> Vec<Uuid> {
// Sort so that we can search in one iteration.
Expand Down
7 changes: 0 additions & 7 deletions rust/blockstore/src/types/reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,4 @@ impl<
}
}
}

pub async fn get_all_data(&'referred_data self) -> Vec<(&'referred_data str, K, V)> {
match self {
BlockfileReader::MemoryBlockfileReader(_) => todo!(),
BlockfileReader::ArrowBlockfileReader(reader) => reader.get_all_data().await,
}
}
}
10 changes: 8 additions & 2 deletions rust/index/src/spann/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ pub enum SpannIndexWriterConstructionError {
BlockfileReaderConstructionError,
#[error("Blockfile writer construction error")]
BlockfileWriterConstructionError,
#[error("Error loading version data from blockfile")]
BlockfileVersionDataLoadError,
}

impl ChromaError for SpannIndexWriterConstructionError {
Expand All @@ -41,6 +43,7 @@ impl ChromaError for SpannIndexWriterConstructionError {
Self::HnswIndexConstructionError => ErrorCodes::Internal,
Self::BlockfileReaderConstructionError => ErrorCodes::Internal,
Self::BlockfileWriterConstructionError => ErrorCodes::Internal,
Self::BlockfileVersionDataLoadError => ErrorCodes::Internal,
}
}
}
Expand Down Expand Up @@ -114,8 +117,11 @@ impl SpannIndexWriter {
}
};
// Load data using the reader.
let versions_data = reader.get_all_data().await;
versions_data.iter().for_each(|(_, key, value)| {
let versions_data = reader
.get_range(.., ..)
.await
.map_err(|_| SpannIndexWriterConstructionError::BlockfileVersionDataLoadError)?;
versions_data.iter().for_each(|(key, value)| {
versions_map.insert(*key, *value);
});
Ok(versions_map)
Expand Down

0 comments on commit 540f528

Please sign in to comment.