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
27 changes: 27 additions & 0 deletions rust/lance-index/src/scalar/inverted/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1206,6 +1206,33 @@ impl InvertedIndex {
self.partitions.len() == 1 && self.partitions[0].is_legacy()
}

/// Read only the index's [`InvertedIndexParams`],
/// Contains more complete info than manifest's lossy `InvertedIndexDetails`.
pub async fn load_params(store: &dyn IndexStore) -> Result<InvertedIndexParams> {
match store.open_index_file(METADATA_FILE).await {
Ok(reader) => {
let params = reader
.schema()
.metadata
.get("params")
.ok_or(Error::index("params not found in metadata".to_owned()))?;
Ok(serde_json::from_str::<InvertedIndexParams>(params)?)
}
Err(_) => {
// Legacy format: params live in the tokens file (see
// `load_legacy_index`).
let reader = store.open_index_file(TOKENS_FILE).await?;
Ok(reader
.schema()
.metadata
.get("tokenizer")
.map(|s| serde_json::from_str::<InvertedIndexParams>(s))
.transpose()?
.unwrap_or_default())
Comment on lines +1212 to +1231

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the relevant file and surrounding symbols.
FILE="rust/lance-index/src/scalar/inverted/index.rs"

echo "== outline =="
ast-grep outline "$FILE" --view expanded || true

echo
echo "== line numbers around load/load_params =="
grep -n "async fn load\|async fn load_params\|METADATA_FILE\|TOKENS_FILE\|params not found\|tokenizer" -n "$FILE" || true

echo
echo "== relevant slices =="
sed -n '1140,1265p' "$FILE"

Repository: lance-format/lance

Length of output: 1922


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="rust/lance-index/src/scalar/inverted/index.rs"

grep -n "async fn load\|async fn load_params\|METADATA_FILE\|TOKENS_FILE\|params not found\|tokenizer" "$FILE" || true
sed -n '1180,1265p' "$FILE"

Repository: lance-format/lance

Length of output: 1922


Only fall back when the metadata file is missing.
Err(_) here treats permission, I/O, and corrupt-metadata failures as the legacy format, which hides the real error behind a tokens-file read or default params. Match the store’s not-found error only, and share this probe with load so both paths stay aligned.

🤖 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/inverted/index.rs` around lines 1216 - 1236,
Update the metadata probe in the parameter-loading path to fall back to the
legacy TOKENS_FILE format only when open_index_file(METADATA_FILE) returns the
store’s not-found error; propagate permission, I/O, and metadata errors
unchanged. Extract or reuse a shared metadata-presence probe between this path
and load so both use identical not-found handling.

Source: Coding guidelines

}
}
}

pub async fn load(
store: Arc<dyn IndexStore>,
frag_reuse_index: Option<Arc<dyn RowIdRemapper>>,
Expand Down
47 changes: 47 additions & 0 deletions rust/lance/src/dataset/tests/dataset_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4113,3 +4113,50 @@ async fn test_manifest_read_recovers_from_stale_size() {
assert_eq!(indices.len(), 1);
assert_eq!(indices[0].name, "id_idx");
}

/// `load_segment_params` must match the fully opened segment's params,
/// including `custom_stop_words` — the field `InvertedIndexDetails` loses.
#[tokio::test]
async fn test_load_segment_params_full_fidelity() {
use crate::index::DatasetIndexInternalExt;
use lance_index::metrics::NoOpMetricsCollector;
use lance_index::scalar::inverted::InvertedIndex;

let batch = RecordBatch::try_new(
arrow_schema::Schema::new(vec![Field::new("text", DataType::Utf8, false)]).into(),
vec![Arc::new(StringArray::from(vec![
"the quick brown fox",
"lazy dogs sleep",
]))],
)
.unwrap();
let schema = batch.schema();
let stream = RecordBatchIterator::new(vec![batch].into_iter().map(Ok), schema);
let mut dataset = Dataset::write(stream, "memory://test/segment_params", None)
.await
.unwrap();
Comment on lines +4121 to +4137

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use the standard Rust test setup conventions.

Move the use declarations to file scope, construct the batch with record_batch!(), and use plain "memory://" instead of a path-qualified URI.

As per coding guidelines, “Place use imports at the top of the file,” “Use record_batch!() from arrow_array to construct RecordBatch in tests,” and “Use plain "memory://" URIs in tests.”

🤖 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/src/dataset/tests/dataset_index.rs` around lines 4121 - 4137,
Update the test setup around Dataset::write: move the shown imports to file
scope, replace the manual RecordBatch construction with arrow_array’s
record_batch!() macro, and change the storage URI to plain "memory://". Preserve
the existing test data and schema behavior.

Source: Coding guidelines


let params = InvertedIndexParams::default().custom_stop_words(Some(vec!["quick".to_string()]));
dataset
.create_index(&["text"], IndexType::Inverted, None, &params, true)
.await
.unwrap();

let segments = crate::index::scalar::load_segments(&dataset, "text")
.await
.unwrap()
.expect("FTS index segments");
let read = crate::index::scalar::load_segment_params(&dataset, &segments[0])
.await
.unwrap();

let generic = dataset
.open_generic_index("text", &segments[0].uuid, &NoOpMetricsCollector)
.await
.unwrap();
let opened = generic
.as_any()
.downcast_ref::<InvertedIndex>()
.expect("inverted index");
assert_eq!(&read, opened.params());
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
2 changes: 1 addition & 1 deletion rust/lance/src/index/scalar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub(crate) mod inverted;
pub(crate) mod label_list;
pub(crate) mod zonemap;

pub use inverted::{load_segment_details, load_segments};
pub use inverted::{load_segment_details, load_segment_params, load_segments};

pub use crate::index::scalar_logical::{LogicalScalarIndex, load_named_scalar_segments};

Expand Down
11 changes: 10 additions & 1 deletion rust/lance/src/index/scalar/inverted.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use lance_core::ROW_ID;
use lance_index::metrics::NoOpMetricsCollector;
use lance_index::pbold::InvertedIndexDetails;
use lance_index::scalar::index_files_to_table;
use lance_index::scalar::inverted::InvertedIndex;
use lance_index::scalar::inverted::{InvertedIndex, InvertedIndexParams};
use lance_index::scalar::lance_format::LanceIndexStore;
use lance_index::scalar::registry::VALUE_COLUMN_NAME;
use lance_table::format::IndexMetadata;
Expand Down Expand Up @@ -220,6 +220,15 @@ pub async fn load_segment_details(
})
}

/// Read one segment's [`InvertedIndexParams`]
pub async fn load_segment_params(
dataset: &Dataset,
segment: &IndexMetadata,
) -> Result<InvertedIndexParams> {
let store = LanceIndexStore::from_dataset_for_existing(dataset, segment).await?;
InvertedIndex::load_params(&store).await
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
Loading