[Rust] Standalone vector index build and ANN search with Python API#797
Conversation
Signed-off-by: chenxu <chenxu@dmetasoul.com>
Signed-off-by: chenxu <chenxu@dmetasoul.com>
Signed-off-by: chenxu <chenxu@dmetasoul.com>
Signed-off-by: chenxu <chenxu@dmetasoul.com>
Signed-off-by: chenxu <chenxu@dmetasoul.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a483b6b55
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Extract first 700 train vectors + 5 test vectors from glove-200d into python/tests/vector/data/ (~554KB total) - Use relative paths in both Python tests and Rust tests - Fix VectorShardIndexBuilder::new() calls (remove removed index_prefix param) - Fix type annotation for Vec::new() in Rust test - CI can now run tests without external data dependency Signed-off-by: chenxu <chenxu@dmetasoul.com>
- Fix all clippy warnings in lakesoul-io (unused imports, identity map, collapsible ifs) - Move vector_search.rs into lakesoul-io/src/vector/search.rs - Use LakeSoulReader in VectorShardIndexBuilder::read_all_batches for proper merge-on-read handling - Return empty result (not full scan) when vector search yields no hits - Fix partition_desc construction to follow table's partition_by order - Reduce catalog test nlist to 8 for small per-bucket data (~125 vecs) - Clean up index files on disk between catalog test runs - Use new vector as query in incremental test for reliable verification Signed-off-by: chenxu <chenxu@dmetasoul.com>
- Remove identity map, use to_vec() instead of iter().copied().collect() - Allow clippy::useless_conversion and clippy::too_many_arguments - Replace needless_range_loop with iterator-based zip/enumerate Signed-off-by: chenxu <chenxu@dmetasoul.com>
- Auto-detect parquet schema from first file for proper column projection when using LakeSoulReader in read_all_batches - Use LocalFileSystem::new() (root at /) in tests so absolute index_prefix paths work correctly (matching Python behavior) - Mark test_read_parquet_schema and test_build_and_list_files as #[ignore] (require prepare_data.py) Signed-off-by: chenxu <chenxu@dmetasoul.com>
e5eb115 to
e55b697
Compare
- Remove empty lines after doc comments (cluster.rs, ivf/mod.rs)
- Add #![allow(dead_code)] to vendored rabitq modules with unused API
surface (fastscan, fastscan_kernel, kmeans)
- Wrap madvise call in unsafe{} block for Rust 2024 compliance (memory.rs)
- Fix Cargo.toml: remove non-existent spill_bench, add mor_bench
Signed-off-by: chenxu <chenxu@dmetasoul.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a2ee0fe12
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let u = u.trim_end_matches('/'); | ||
| std::path::Path::new(u) | ||
| .parent() | ||
| .and_then(|p| p.to_str()) | ||
| .map(|s| s.to_string()) |
There was a problem hiding this comment.
Preserve URL authorities when deriving the scan prefix
When a Python scan receives a file such as s3://bucket/table/part.parquet, treating the URL as a filesystem path derives s3:/bucket/table, and .with_prefix then makes register_object_store process an S3 URL without a host. This affects ordinary Python S3 scans, not only vector searches, and can fail before the correctly formed file URL is registered; parse URLs by scheme or avoid setting this prefix for non-local paths.
Useful? React with 👍 / 👎.
| for batch in &all_batches { | ||
| total += batch.ids.len(); | ||
| builder.insert_batch(batch.clone())?; | ||
| } |
There was a problem hiding this comment.
Insert only newly committed vectors during incremental builds
On every subsequent build_vector_index call, _group_files_by_shard supplies all files in the latest snapshot, and this loaded path inserts every row again without checking existing IDs. Rebuilding an unchanged table therefore appends a full duplicate copy each time; updates and compactions also retain obsolete vectors, causing unbounded index growth and duplicate or stale ANN candidates that can displace valid top-K results.
Useful? React with 👍 / 👎.
| // Build object_store_options for LakeSoulReader (converted from store_config) | ||
| let object_store_options = store_config.clone(); |
There was a problem hiding this comment.
Translate S3 credentials for the LakeSoul reader
For S3 index builds where credentials or a custom endpoint are supplied through LakeSoulCatalog.object_store_options rather than environment variables, _default_object_store_config converts them to keys such as access_key_id and endpoint, but cloning that map into LakeSoulIOConfig does not convert them back. The reader's S3 implementation only reads fs.s3a.access.key, fs.s3a.secret.key, fs.s3a.endpoint, and related keys, so the index store can authenticate while the reader that loads the Parquet vectors cannot.
Useful? React with 👍 / 👎.
| let pk_column = io_config | ||
| .primary_keys | ||
| .first() | ||
| .cloned() | ||
| .unwrap_or_else(|| "id".to_string()); |
There was a problem hiding this comment.
Retain the actual primary key for compacted scans
For a CompactionCommit, NativeMetadataClient.get_scan_plan_partitions deliberately passes an empty primary-key list, so this fallback selects a column literally named id. Any compacted table whose primary key has another name then builds the ANN ID filter against a nonexistent or unrelated column, making vector search fail or return incorrect rows even though the index was built with the real metadata primary key.
Useful? React with 👍 / 👎.
| Err(e) => { | ||
| warn!("Failed to load vector index at '{}': {:?}", prefix, e); | ||
| return Ok(None); | ||
| } |
There was a problem hiding this comment.
Propagate index load failures instead of returning no matches
If loading an existing index fails because of an object-store outage, denied permission, corrupt segment, or incompatible manifest, this branch converts the failure into Ok(None). The caller interprets that as an empty ANN result and injects a constant-false filter, silently reporting that the table has no matching rows; only a verified missing-index condition should be optional, while operational and corruption errors must propagate.
Useful? React with 👍 / 👎.
| Err(e) => { | ||
| tracing::warn!("Invalid vector search query: {}", e); | ||
| return Ok(filters); |
There was a problem hiding this comment.
Reject malformed vector queries instead of scanning all rows
When vector_search_query contains an invalid float, returning the original filters silently disables vector search. A caller expecting top-K ANN results can therefore receive the entire table and trigger a much more expensive scan; propagate the parse error so invalid search input cannot be mistaken for a successful query.
Useful? React with 👍 / 👎.
Summary
This PR implements single-machine vector index building and approximate nearest neighbor (ANN) search for LakeSoul, based on IVF+RaBitQ. Includes full Python Table API for end-to-end workflow.
Key Features
1. New crate:
lakesoul-vector2. Per-shard index building (
lakesoul-io)VectorShardIndexBuilder: reads parquet files, builds IVF+RaBitQ index per (partition, bucket) shardindex_prefixparameter needed{table_path}/_vector_index/{col}/{partition}/{bucket}/3. Vector search in read path (
lakesoul-io)LakeSoulReader::start()auto-injects vector search results as PK filterrerank_by_distance()4. Python Table API
5. E2E tests (glove-200d, multi-bucket verified)
Dependency Graph
Testing
cargo check uv run --directory python python tests/vector/test_e2e_glove.py uv run --directory python python tests/vector/test_e2e_glove.py --use-catalog # requires PG