Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion rust-toolchain.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# We keep this pinned to keep clippy and rustfmt in sync between local and CI.
# Feel free to upgrade to bring in new lints.
[toolchain]
channel = "1.94.0"
channel = "1.97.0"
components = ["rustfmt", "clippy", "rust-analyzer"]
8 changes: 1 addition & 7 deletions rust/lance-arrow/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1022,13 +1022,7 @@ fn merge_list_struct(left: &dyn Array, right: &dyn Array) -> Arc<dyn Array> {
fn normalize_validity(
validity: Option<&arrow_buffer::NullBuffer>,
) -> Option<&arrow_buffer::NullBuffer> {
validity.and_then(|v| {
if v.null_count() == v.len() {
None
} else {
Some(v)
}
})
validity.filter(|v| v.null_count() != v.len())
}

/// Helper function to merge validity buffers from two struct arrays
Expand Down
12 changes: 4 additions & 8 deletions rust/lance-datafusion/src/logical_expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,10 @@ pub fn resolve_column_type(expr: &Expr, schema: &Schema) -> Option<DataType> {
field_path.push(c.name.as_str());
break;
}
Expr::ScalarFunction(udf) => {
if udf.name() == GetFieldFunc::default().name() {
let name = get_as_string_scalar_opt(&udf.args[1])?;
field_path.push(name);
current_expr = &udf.args[0];
} else {
return None;
}
Expr::ScalarFunction(udf) if udf.name() == GetFieldFunc::default().name() => {
let name = get_as_string_scalar_opt(&udf.args[1])?;
field_path.push(name);
current_expr = &udf.args[0];
}
_ => return None,
}
Expand Down
12 changes: 4 additions & 8 deletions rust/lance-datafusion/src/planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ impl Planner {
}
_ => Err(Error::invalid_input(format!(
"Unsupported function args: {:?}",
&func.args
func.args
))),
}
}
Expand Down Expand Up @@ -1062,13 +1062,9 @@ impl TreeNodeVisitor<'_> for ColumnCapturingVisitor {
self.columns.insert(path);
self.current_path.clear();
}
Expr::ScalarFunction(udf) => {
if udf.name() == GetFieldFunc::default().name() {
if let Some(name) = get_as_string_scalar_opt(&udf.args[1]) {
self.current_path.push_front(name.to_string())
} else {
self.current_path.clear();
}
Expr::ScalarFunction(udf) if udf.name() == GetFieldFunc::default().name() => {
if let Some(name) = get_as_string_scalar_opt(&udf.args[1]) {
self.current_path.push_front(name.to_string())
} else {
self.current_path.clear();
}
Expand Down
3 changes: 1 addition & 2 deletions rust/lance-encoding/src/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1949,8 +1949,7 @@ impl StructuralBatchDecodeStream {
next_task.into_batch(emitted_batch_size_warning)?
};
let num_rows = batch.num_rows() as u64;
if num_rows > 0 {
let bpr = data_size / num_rows;
if let Some(bpr) = data_size.checked_div(num_rows) {
let prev = bytes_per_row_feedback.load(Ordering::Relaxed);
let next = if prev == 0 || bpr >= prev {
// First batch or actual size is larger than estimate:
Expand Down
4 changes: 2 additions & 2 deletions rust/lance-encoding/src/encodings/physical/packed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ impl PerValueCompressor for PackedStructVariablePerValueEncoder {
let mut field_data = Vec::with_capacity(self.fields.len());
let mut field_metadata = Vec::with_capacity(self.fields.len());

for (field, child_block) in self.fields.iter().zip(struct_block.children.into_iter()) {
for (field, child_block) in self.fields.iter().zip(struct_block.children) {
let compressor = crate::compression::CompressionStrategy::create_per_value(
&self.strategy,
field,
Expand Down Expand Up @@ -688,7 +688,7 @@ impl VariablePerValueDecompressor for PackedStructVariablePerValueDecompressor {
}

let mut children = Vec::with_capacity(self.fields.len());
for (field, accumulator) in self.fields.iter().zip(accumulators.into_iter()) {
for (field, accumulator) in self.fields.iter().zip(accumulators) {
match (field, accumulator) {
(
VariablePackedStructFieldDecoder {
Expand Down
2 changes: 1 addition & 1 deletion rust/lance-encoding/src/previous/encodings/logical/blob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ impl BlobFieldEncoder {
.nulls()
.cloned()
.unwrap_or(NullBuffer::new_valid(binarray.len()));
for (w, is_valid) in binarray.value_offsets().windows(2).zip(nulls.into_iter()) {
for (w, is_valid) in binarray.value_offsets().windows(2).zip(&nulls) {
if is_valid {
let start = w[0] as u64;
let end = w[1] as u64;
Expand Down
2 changes: 1 addition & 1 deletion rust/lance-file/src/reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1939,7 +1939,7 @@ impl FileMetadataProvider {
.collect::<Vec<_>>();
let metadata_bytes = io.submit_request(ranges, 0).await?;
for ((result_index, column_index, _), bytes) in
missing_columns.into_iter().zip(metadata_bytes.into_iter())
missing_columns.into_iter().zip(metadata_bytes)
{
let column_metadata = pbfile::ColumnMetadata::decode(bytes)?;
let column_info = FileReader::meta_to_col_info(
Expand Down
10 changes: 3 additions & 7 deletions rust/lance-index/src/scalar/bitmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -891,8 +891,7 @@ impl BitmapBatchWriter {
return Ok(());
}
let keys_array =
ScalarValue::iter_to_array(self.keys.drain(..).collect::<Vec<_>>().into_iter())
.unwrap();
ScalarValue::iter_to_array(self.keys.drain(..).collect::<Vec<_>>()).unwrap();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -C 4 'fn iter_to_array|iter_to_array\(' \
  rust Cargo.toml Cargo.lock \
  "${CARGO_HOME:-$HOME/.cargo}/registry/src" 2>/dev/null || true

Repository: lance-format/lance

Length of output: 9752


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the exact ScalarValue::iter_to_array definition and any nearby iterator bounds.
rg -n -C 6 'pub fn iter_to_array|fn iter_to_array|iter_to_array<|IntoIterator' \
  rust "${CARGO_HOME:-$HOME/.cargo}/registry/src" 2>/dev/null | head -n 200

# Show the relevant bitmap writer block for context.
sed -n '880,905p' rust/lance-index/src/scalar/bitmap.rs

# Check whether drain(..) is already used directly with iter_to_array elsewhere in the repo.
rg -n 'iter_to_array\([^)]*drain\(' rust/lance-index rust/lance 2>/dev/null || true

Repository: lance-format/lance

Length of output: 13036


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find the exact definition/signature of ScalarValue::iter_to_array.
rg -n -C 5 'iter_to_array' \
  rust "${CARGO_HOME:-$HOME/.cargo}/registry/src" \
  -g '*.rs' 2>/dev/null | head -n 200

# Show the surrounding implementation in the repository where the bad allocation occurs.
sed -n '888,900p' rust/lance-index/src/scalar/bitmap.rs

# Show nearby call sites that already pass iterators directly.
sed -n '1268,1305p' rust/lance-index/src/scalar/bitmap.rs

Repository: lance-format/lance

Length of output: 11594


Avoid the extra Vec allocation here.
ScalarValue::iter_to_array already accepts an IntoIterator, so self.keys.drain(..) can be passed directly instead of draining into a temporary Vec first.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rust/lance-index/src/scalar/bitmap.rs` at line 894, Remove the intermediate
Vec allocation in the ScalarValue::iter_to_array call by passing
self.keys.drain(..) directly, preserving the existing behavior.

Source: Coding guidelines

let total_size: usize = self.serialized.iter().map(|b| b.len()).sum();
let mut binary_builder = BinaryBuilder::with_capacity(self.serialized.len(), total_size);
for b in self.serialized.drain(..) {
Expand Down Expand Up @@ -1123,10 +1122,7 @@ async fn drain_same_key_bitmaps(
let merged_key = OrderableScalarValue(key);
advance_cursor_and_push(cursors, heap, item.shard_idx).await?;

loop {
let Some(Reverse(next_item)) = heap.peek() else {
break;
};
while let Some(Reverse(next_item)) = heap.peek() {
if next_item.key != merged_key {
break;
}
Expand Down Expand Up @@ -1280,7 +1276,7 @@ impl BitmapIndexPlugin {
let bitmap_size = bytes.len();

if cur_bytes + bitmap_size > MAX_BITMAP_ARRAY_LENGTH {
let keys_array = ScalarValue::iter_to_array(cur_keys.clone().into_iter()).unwrap();
let keys_array = ScalarValue::iter_to_array(cur_keys.clone()).unwrap();
let mut binary_builder = BinaryBuilder::new();
for b in &cur_bitmaps {
binary_builder.append_value(b);
Expand Down
6 changes: 1 addition & 5 deletions rust/lance-index/src/scalar/btree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5327,12 +5327,8 @@ mod tests {
mapping.insert(old_id, None);
}

let mut new_id_counter = 100_000;

// Remap all other rows
for old_id in (0..1000).chain(10000..15000) {
let new_id = new_id_counter;
new_id_counter += 1;
for (new_id, old_id) in (100_000..).zip((0..1000).chain(10000..15000)) {
mapping.insert(old_id, Some(new_id));
}

Expand Down
14 changes: 7 additions & 7 deletions rust/lance-index/src/scalar/btree/flat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,13 +218,13 @@ impl FlatIndex {
));
}
}
(Bound::Included(lower) | Bound::Excluded(lower), Bound::Unbounded) => {
if lower.is_null() {
return Ok(NullableRowAddrSet::new(
Default::default(),
self.all_addrs_map.clone(),
));
}
(Bound::Included(lower) | Bound::Excluded(lower), Bound::Unbounded)
if lower.is_null() =>
{
return Ok(NullableRowAddrSet::new(
Default::default(),
self.all_addrs_map.clone(),
));
}
_ => {}
},
Expand Down
2 changes: 1 addition & 1 deletion rust/lance-index/src/scalar/fmindex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1701,7 +1701,7 @@ impl FMIndexScalarIndex {
}
pfiles.sort_by_key(|(id, _)| *id);
let io_parallelism = store.io_parallelism().max(1);
let mut parts = futures::stream::iter(pfiles.into_iter())
let mut parts = futures::stream::iter(pfiles)
.map(|(id, name)| {
let store = Arc::clone(&store);
async move {
Expand Down
4 changes: 2 additions & 2 deletions rust/lance-index/src/scalar/inverted.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ pub async fn build_global_bm25_scorer(
let (mut total_tokens, mut num_docs, first_token_docs) =
first_index.bm25_stats_for_terms(&terms).await?;
let mut token_docs = HashMap::with_capacity(terms.len());
for (term, count) in terms.iter().cloned().zip(first_token_docs.into_iter()) {
for (term, count) in terms.iter().cloned().zip(first_token_docs) {
token_docs.insert(term, count);
}

Expand All @@ -100,7 +100,7 @@ pub async fn build_global_bm25_scorer(
index.bm25_stats_for_terms(&terms).await?;
total_tokens += segment_total_tokens;
num_docs += segment_num_docs;
for (term, count) in terms.iter().zip(segment_token_docs.into_iter()) {
for (term, count) in terms.iter().zip(segment_token_docs) {
*token_docs
.get_mut(term)
.expect("global scorer terms should already be initialized") += count;
Expand Down
24 changes: 8 additions & 16 deletions rust/lance-index/src/scalar/zonemap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,10 +265,8 @@ impl ZoneMapIndex {
return Ok(zone.nan_count > 0);
}
}
ScalarValue::Float64(Some(f)) => {
if f.is_nan() {
return Ok(zone.nan_count > 0);
}
ScalarValue::Float64(Some(f)) if f.is_nan() => {
return Ok(zone.nan_count > 0);
}
_ => {}
}
Expand All @@ -295,10 +293,8 @@ impl ZoneMapIndex {
return Ok(false); // Nothing is greater than NaN
}
}
ScalarValue::Float64(Some(f)) => {
if f.is_nan() {
return Ok(false); // Nothing is greater than NaN
}
ScalarValue::Float64(Some(f)) if f.is_nan() => {
return Ok(false); // Nothing is greater than NaN
}
_ => {}
}
Expand All @@ -322,10 +318,8 @@ impl ZoneMapIndex {
return Ok(zone.nan_count > 0 || zone_min <= e);
}
}
ScalarValue::Float64(Some(f)) => {
if f.is_nan() {
return Ok(zone.nan_count > 0 || zone_min <= e);
}
ScalarValue::Float64(Some(f)) if f.is_nan() => {
return Ok(zone.nan_count > 0 || zone_min <= e);
}
_ => {}
}
Expand All @@ -345,10 +339,8 @@ impl ZoneMapIndex {
return Ok(true);
}
}
ScalarValue::Float64(Some(f)) => {
if f.is_nan() {
return Ok(true);
}
ScalarValue::Float64(Some(f)) if f.is_nan() => {
return Ok(true);
}
_ => {}
}
Expand Down
4 changes: 2 additions & 2 deletions rust/lance-index/src/vector/bq/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3930,7 +3930,7 @@ mod tests {
.into_iter()
.map(|(id, dist)| (id as u64, dist))
.collect::<Vec<_>>();
expected.sort_by(|left, right| left.0.cmp(&right.0));
expected.sort_by_key(|left| left.0);

let mut heap = BinaryHeap::with_capacity(k);
let mut distances = Vec::new();
Expand All @@ -3952,7 +3952,7 @@ mod tests {
.into_iter()
.map(|node| (node.id, node.dist.0))
.collect::<Vec<_>>();
actual.sort_by(|left, right| left.0.cmp(&right.0));
actual.sort_by_key(|left| left.0);

assert_eq!(actual.len(), expected.len());
for ((actual_id, actual_dist), (expected_id, expected_dist)) in
Expand Down
4 changes: 2 additions & 2 deletions rust/lance-index/src/vector/flat/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,7 @@ mod tests {
.zip(dists.values().iter())
.map(|(row_id, dist)| (*row_id, *dist))
.collect::<Vec<_>>();
results.sort_by(|left, right| left.0.cmp(&right.0));
results.sort_by_key(|left| left.0);
results
}

Expand All @@ -579,7 +579,7 @@ mod tests {
.into_iter()
.map(|node| (node.id, node.dist.0))
.collect::<Vec<_>>();
results.sort_by(|left, right| left.0.cmp(&right.0));
results.sort_by_key(|left| left.0);
results
}

Expand Down
4 changes: 2 additions & 2 deletions rust/lance-index/src/vector/flat/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ impl VectorStore for FlatFloatStorage {

fn append_batch(&self, batch: RecordBatch, _vector_column: &str) -> Result<Self> {
// TODO: use chunked storage
let new_batch = concat_batches(&batch.schema(), vec![&self.batch, &batch].into_iter())?;
let new_batch = concat_batches(&batch.schema(), vec![&self.batch, &batch])?;
let mut storage = self.clone();
storage.row_ids = Arc::new(
new_batch
Expand Down Expand Up @@ -296,7 +296,7 @@ impl VectorStore for FlatBinStorage {

fn append_batch(&self, batch: RecordBatch, _vector_column: &str) -> Result<Self> {
// TODO: use chunked storage
let new_batch = concat_batches(&batch.schema(), vec![&self.batch, &batch].into_iter())?;
let new_batch = concat_batches(&batch.schema(), vec![&self.batch, &batch])?;
let mut storage = self.clone();
storage.row_ids = Arc::new(
new_batch
Expand Down
26 changes: 12 additions & 14 deletions rust/lance-io/src/object_writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,25 +392,23 @@ impl AsyncWrite for ObjectWriter {
let fut = Box::pin(async move { store.put_multipart(path.as_ref()).await });
self.state = UploadState::CreatingUpload(fut);
}
// TODO: Make max concurrency configurable from storage options.
UploadState::InProgress {
upload,
part_idx,
futures,
..
} => {
// TODO: Make max concurrency configurable from storage options.
if futures.len() < max_upload_parallelism() {
let data = Self::next_part_buffer(
&mut mut_self.buffer,
*part_idx,
mut_self.use_constant_size_upload_parts,
);
futures.spawn(
Self::put_part(upload.as_mut(), data, *part_idx, None)
.instrument(tracing::Span::current()),
);
*part_idx += 1;
}
} if futures.len() < max_upload_parallelism() => {
let data = Self::next_part_buffer(
&mut mut_self.buffer,
*part_idx,
mut_self.use_constant_size_upload_parts,
);
futures.spawn(
Self::put_part(upload.as_mut(), data, *part_idx, None)
.instrument(tracing::Span::current()),
);
*part_idx += 1;
}
_ => {}
}
Expand Down
6 changes: 3 additions & 3 deletions rust/lance-namespace-impls/src/dir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1669,9 +1669,9 @@ impl DirectoryNamespace {

if needs_sort {
if descending {
table_versions.sort_by(|a, b| b.version.cmp(&a.version));
table_versions.sort_by_key(|v| std::cmp::Reverse(v.version));
} else {
table_versions.sort_by(|a, b| a.version.cmp(&b.version));
table_versions.sort_by_key(|v| v.version);
}
}

Expand Down Expand Up @@ -2403,7 +2403,7 @@ impl DirectoryNamespace {
}

fn table_full_uri(&self, table_name: &str) -> String {
format!("{}/{}.lance", &self.root, table_name)
format!("{}/{}.lance", self.root, table_name)
}

/// Get the object store path for a table (relative to base_path)
Expand Down
6 changes: 1 addition & 5 deletions rust/lance-table/src/rowids.rs
Original file line number Diff line number Diff line change
Expand Up @@ -355,11 +355,7 @@ impl RowIdSequence {
while (index - rows_passed) >= cur_seg_len {
rows_passed += cur_seg_len;
cur_seg = seg_iter.next();
if let Some(cur_seg) = cur_seg {
cur_seg_len = cur_seg.len();
} else {
return None;
}
cur_seg_len = cur_seg?.len();
}

Some(cur_seg.unwrap().get(index - rows_passed).unwrap())
Expand Down
Loading
Loading