From 3c47c74a4738f71daa5ff1883e65efefc78933b7 Mon Sep 17 00:00:00 2001 From: ChilePiquin Date: Mon, 31 Aug 2026 10:11:14 -0700 Subject: [PATCH] Restrict selected index UUIDs to reservations --- rust/lancedb/src/index.rs | 13 ++-- rust/lancedb/src/table/create_index.rs | 96 ++++++++++++++++++++++---- 2 files changed, 89 insertions(+), 20 deletions(-) diff --git a/rust/lancedb/src/index.rs b/rust/lancedb/src/index.rs index 1c0a45c5b..b6328dca3 100644 --- a/rust/lancedb/src/index.rs +++ b/rust/lancedb/src/index.rs @@ -219,18 +219,16 @@ impl IndexBuilder { self } - /// Use a caller-selected UUID for the created index. + /// Use a caller-selected UUID for an empty index reservation. /// /// This is supported for native LanceDB tables. Remote tables currently /// reject caller-selected UUIDs because the remote create-index protocol /// does not carry index UUIDs. /// - /// The UUID must not already belong to a surviving committed index on the - /// table. If the UUID is already in use by an unrelated index, index - /// creation fails before building the new index. `replace(true)` may - /// replace an index with the same name, and may reuse that index's UUID - /// only when the replaced index is removed by the same create-index - /// transaction. + /// The builder must also use [`Self::train(false)`]. Trained builds write + /// physical index files before commit and therefore cannot safely accept a + /// caller-selected storage UUID. If the UUID is already in use by a + /// non-empty index, index creation fails before building. /// /// # Examples /// @@ -247,6 +245,7 @@ impl IndexBuilder { /// .create_index(&["user_id"], Index::BTree(BTreeIndexBuilder::default())) /// .name("user_id_btree_index".to_string()) /// .index_uuid(index_uuid) + /// .train(false) /// .execute() /// .await?; /// # Ok(()) diff --git a/rust/lancedb/src/table/create_index.rs b/rust/lancedb/src/table/create_index.rs index 4905f7352..9c6f9f0b8 100644 --- a/rust/lancedb/src/table/create_index.rs +++ b/rust/lancedb/src/table/create_index.rs @@ -151,23 +151,17 @@ impl NativeTable { let (column, lance_idx_params, index_type) = prepared; let mut dataset = (*self.dataset.get().await?).clone(); let columns = [column.as_str()]; - let index_name = opts - .name - .as_deref() - .map(str::to_owned) - .unwrap_or_else(|| format!("{}_idx", column)); if let Some(index_uuid) = opts.index_uuid { let indices = dataset.load_indices().await?; if let Some(existing_index) = indices.iter().find(|index| index.uuid == index_uuid) { - let is_matching_empty_reservation = existing_index.name == index_name - && existing_index - .fragment_bitmap - .as_ref() - .is_some_and(|fragments| fragments.is_empty()); - // Let Lance's name/replace handling classify retries of the - // caller's own empty reservation. - if !is_matching_empty_reservation { + let is_empty_reservation = existing_index + .fragment_bitmap + .as_ref() + .is_some_and(|fragments| fragments.is_empty()); + // Let Lance's name/replace handling classify idempotent + // retries of an empty reservation. + if !is_empty_reservation { return Err(Error::InvalidInput { message: format!( "Index UUID '{}' is already used by index '{}'", @@ -176,6 +170,11 @@ impl NativeTable { }); } } + if opts.train { + return Err(Error::InvalidInput { + message: "index_uuid is only supported when train(false) creates an empty index reservation".to_string(), + }); + } } let mut builder = dataset @@ -1112,6 +1111,7 @@ mod tests { .create_index(&["i"], Index::BTree(BTreeIndexBuilder::default())) .name("i_idx".to_string()) .index_uuid(index_uuid) + .train(false) .execute() .await .unwrap(); @@ -1138,6 +1138,7 @@ mod tests { .create_index(&["a"], Index::BTree(BTreeIndexBuilder::default())) .name("a_idx".to_string()) .index_uuid(index_uuid) + .train(false) .execute() .await .unwrap(); @@ -1146,6 +1147,7 @@ mod tests { .create_index(&["b"], Index::BTree(BTreeIndexBuilder::default())) .name("b_idx".to_string()) .index_uuid(index_uuid) + .train(false) .execute() .await .unwrap_err(); @@ -1158,6 +1160,28 @@ mod tests { assert_eq!(index.index_uuid, Some(index_uuid.to_string())); } + #[tokio::test] + async fn test_create_index_rejects_selected_uuid_trained_build() { + let conn = connect("memory://").execute().await.unwrap(); + let batch = record_batch!(("i", Int32, [1])).unwrap(); + let table = conn + .create_table("my_table", batch) + .execute() + .await + .unwrap(); + let index_uuid = uuid::Uuid::new_v4(); + + let err = table + .create_index(&["i"], Index::BTree(BTreeIndexBuilder::default())) + .name("i_idx".to_string()) + .index_uuid(index_uuid) + .execute() + .await + .unwrap_err(); + + assert!(err.to_string().contains("train(false)")); + } + #[tokio::test] async fn test_create_index_allows_selected_uuid_empty_reservation_retry() { let conn = connect("memory://").execute().await.unwrap(); @@ -1198,6 +1222,52 @@ mod tests { assert_eq!(index.index_uuid, Some(index_uuid.to_string())); } + #[tokio::test] + async fn test_create_index_retries_default_list_element_fts_reservation() { + let conn = connect("memory://").execute().await.unwrap(); + let mut values = ListBuilder::new(StringBuilder::new()); + values.values().append_value("alpha"); + values.append(true); + let values: ArrayRef = Arc::new(values.finish()); + let schema = Arc::new(Schema::new(vec![Field::new( + "tags", + values.data_type().clone(), + false, + )])); + let batch = RecordBatch::try_new(schema, vec![values]).unwrap(); + let table = conn + .create_table("my_table", batch) + .execute() + .await + .unwrap(); + let index_uuid = uuid::Uuid::new_v4(); + let params = || { + Index::FTS( + FtsIndexBuilder::default().document_granularity(DocumentGranularity::ListElement), + ) + }; + + table + .create_index(&["tags"], params()) + .index_uuid(index_uuid) + .train(false) + .replace(false) + .execute() + .await + .unwrap(); + + let err = table + .create_index(&["tags"], params()) + .index_uuid(index_uuid) + .train(false) + .replace(false) + .execute() + .await + .unwrap_err(); + assert!(err.to_string().contains("already exists")); + assert!(!err.to_string().contains("already used by index")); + } + #[tokio::test] async fn test_create_fm_index() { let tmp_dir = tempdir().unwrap();