From bd86cc5aa64029b20b247c1ec8223889fe8fea77 Mon Sep 17 00:00:00 2001 From: ChilePiquin Date: Thu, 3 Sep 2026 14:24:27 -0700 Subject: [PATCH] feat: default create_index to create-only --- nodejs/__test__/table.test.ts | 6 +-- nodejs/lancedb/indices.ts | 2 +- python/python/lancedb/remote/table.py | 16 ++------ python/python/lancedb/table.py | 26 ++++++------- python/python/tests/test_index.py | 12 +++++- python/python/tests/test_table.py | 16 ++++---- rust/lancedb/src/index.rs | 4 +- rust/lancedb/src/job.rs | 52 +++++++++++++++++--------- rust/lancedb/src/remote/table.rs | 44 ++++++++++++++++++---- rust/lancedb/src/table/create_index.rs | 5 +-- 10 files changed, 113 insertions(+), 70 deletions(-) diff --git a/nodejs/__test__/table.test.ts b/nodejs/__test__/table.test.ts index 6f80ca74e..c1544409f 100644 --- a/nodejs/__test__/table.test.ts +++ b/nodejs/__test__/table.test.ts @@ -1526,11 +1526,7 @@ describe("When creating an index", () => { it("should allow me to replace (or not) an existing index", async () => { await tbl.createIndex("id"); - // Default is replace=true - await tbl.createIndex("id"); - await expect(tbl.createIndex("id", { replace: false })).rejects.toThrow( - "already exists", - ); + await expect(tbl.createIndex("id")).rejects.toThrow("already exists"); await tbl.createIndex("id", { replace: true }); }); diff --git a/nodejs/lancedb/indices.ts b/nodejs/lancedb/indices.ts index dbeebf433..c154d2430 100644 --- a/nodejs/lancedb/indices.ts +++ b/nodejs/lancedb/indices.ts @@ -841,7 +841,7 @@ export interface IndexOptions { * and the same name, then an error will be returned. This is true even if * that index is out of date. * - * The default is true + * The default is false */ replace?: boolean; diff --git a/python/python/lancedb/remote/table.py b/python/python/lancedb/remote/table.py index 3eb9cbfa1..3dc1d054a 100644 --- a/python/python/lancedb/remote/table.py +++ b/python/python/lancedb/remote/table.py @@ -402,6 +402,7 @@ class RemoteTable(Table): /, *, config: IndexConfigType, + replace: bool = ..., wait_timeout: Optional[timedelta] = ..., name: Optional[str] = ..., train: bool = ..., @@ -416,7 +417,7 @@ class RemoteTable(Table): index_cache_size: Optional[int] = ..., num_partitions: Optional[int] = ..., num_sub_vectors: Optional[int] = ..., - replace: Optional[bool] = ..., + replace: bool = ..., accelerator: Optional[str] = ..., index_type: Literal[ "VECTOR", "IVF_FLAT", "IVF_SQ", "IVF_PQ", "IVF_HNSW_SQ", "IVF_HNSW_PQ" @@ -435,7 +436,7 @@ class RemoteTable(Table): index_cache_size: Optional[int] = None, num_partitions: Optional[int] = None, num_sub_vectors: Optional[int] = None, - replace: Optional[bool] = None, + replace: bool = False, accelerator: Optional[str] = None, index_type="vector", wait_timeout: Optional[timedelta] = None, @@ -479,7 +480,6 @@ class RemoteTable(Table): vector_column_name, accelerator, index_cache_size, - replace, ) if is_legacy: @@ -503,12 +503,6 @@ class RemoteTable(Table): "If you have 100M+ vectors to index," "please contact us at contact@lancedb.com" ) - if replace is not None: - logging.warning( - "replace is not supported on LanceDB cloud." - "Existing indexes will always be replaced." - ) - idx_type = index_type.upper() if idx_type == "VECTOR" or idx_type == "IVF_PQ": config = IvfPq( @@ -561,7 +555,7 @@ class RemoteTable(Table): column: str, *, config: IndexConfigType, - replace: Optional[bool] = None, + replace: bool = False, wait_timeout: Optional[timedelta] = None, name: Optional[str] = None, train: bool = True, @@ -593,7 +587,6 @@ class RemoteTable(Table): vector_column_name: str, accelerator: Optional[str], index_cache_size: Optional[int], - replace: Optional[bool], ) -> bool: """Detect if this is a legacy create_index call.""" if config is not None: @@ -605,7 +598,6 @@ class RemoteTable(Table): num_sub_vectors, accelerator, index_cache_size, - replace, ) ): return True diff --git a/python/python/lancedb/table.py b/python/python/lancedb/table.py index e397272fc..505861aaf 100644 --- a/python/python/lancedb/table.py +++ b/python/python/lancedb/table.py @@ -1132,7 +1132,7 @@ class Table(ABC): num_partitions: Optional[int] = None, num_sub_vectors: Optional[int] = None, vector_column_name: str = VECTOR_COLUMN_NAME, - replace: bool = True, + replace: bool = False, accelerator: Optional[str] = None, index_cache_size: Optional[int] = None, *, @@ -1166,7 +1166,7 @@ class Table(ABC): The index configuration object. If provided, uses the new unified API. Can be one of: IvfFlat, IvfPq, IvfSq, IvfRq, HnswPq, HnswSq, BTree, Bitmap, LabelList, Fm, FTS. - replace : bool, default True + replace : bool, default False Whether to replace an existing index on this column. wait_timeout : timedelta, optional Timeout to wait for async indexing to complete. @@ -1198,7 +1198,7 @@ class Table(ABC): column: str, *, config: IndexConfigType, - replace: Optional[bool] = None, + replace: bool = False, wait_timeout: Optional[timedelta] = None, name: Optional[str] = None, train: bool = True, @@ -1260,7 +1260,7 @@ class Table(ABC): self, column: str, *, - replace: bool = True, + replace: bool = False, index_type: ScalarIndexType = "BTREE", wait_timeout: Optional[timedelta] = None, name: Optional[str] = None, @@ -1272,7 +1272,7 @@ class Table(ABC): column : str The column to be indexed. Must be a boolean, integer, float, or string column. - replace : bool, default True + replace : bool, default False Replace the existing index if it exists. index_type: Literal["BTREE", "BITMAP", "LABEL_LIST"], default "BTREE" The type of index to create. @@ -2988,7 +2988,7 @@ class LanceTable(Table): num_partitions: Optional[int] = None, num_sub_vectors: Optional[int] = None, vector_column_name: str = VECTOR_COLUMN_NAME, - replace: bool = True, + replace: bool = False, accelerator: Optional[str] = None, index_cache_size: Optional[int] = None, num_bits: int = 8, @@ -3030,7 +3030,7 @@ class LanceTable(Table): The index configuration object. If provided, uses the new unified API. Can be one of: IvfFlat, IvfPq, IvfSq, IvfRq, HnswPq, HnswSq, BTree, Bitmap, LabelList, Fm, FTS. - replace : bool, default True + replace : bool, default False Whether to replace an existing index on this column. wait_timeout : timedelta, optional Timeout to wait for async indexing to complete. @@ -3169,7 +3169,7 @@ class LanceTable(Table): column: str, *, config: IndexConfigType, - replace: Optional[bool] = None, + replace: bool = False, wait_timeout: Optional[timedelta] = None, name: Optional[str] = None, train: bool = True, @@ -3424,7 +3424,7 @@ class LanceTable(Table): self, column: str, *, - replace: bool = True, + replace: bool = False, index_type: ScalarIndexType = "BTREE", name: Optional[str] = None, ): @@ -5313,7 +5313,7 @@ class AsyncTable: self, column: str, *, - replace: Optional[bool] = None, + replace: bool = False, config: Optional[ Union[ IvfFlat, @@ -5344,14 +5344,14 @@ class AsyncTable: ---------- column: str The column to index. - replace: bool, default True + replace: bool, default False Whether to replace the existing index If this is false, and another index already exists on the same columns and the same name, then an error will be returned. This is true even if that index is out of date. - The default is True + The default is False config: default None For advanced configuration you can specify the type of index you would like to create. You can also specify index-specific parameters when @@ -5409,7 +5409,7 @@ class AsyncTable: self, column: str, *, - replace: Optional[bool] = None, + replace: bool = False, config: Optional[ Union[ IvfFlat, diff --git a/python/python/tests/test_index.py b/python/python/tests/test_index.py index fe6ebe87a..03c7e913d 100644 --- a/python/python/tests/test_index.py +++ b/python/python/tests/test_index.py @@ -97,6 +97,9 @@ async def test_create_index_async_returns_done_job(some_table: AsyncTable): async def test_create_scalar_index(some_table: AsyncTable): # Can create await some_table.create_index("id") + # Can't recreate by default + with pytest.raises(RuntimeError, match="already exists"): + await some_table.create_index("id") # Can recreate if replace=True await some_table.create_index("id", replace=True) indices = await some_table.list_indices() @@ -110,7 +113,7 @@ async def test_create_scalar_index(some_table: AsyncTable): with pytest.raises(RuntimeError, match="already exists"): await some_table.create_index("id", replace=False) # can also specify index type - await some_table.create_index("id", config=BTree()) + await some_table.create_index("id", config=BTree(), replace=True) await some_table.drop_index("id_idx") indices = await some_table.list_indices() @@ -351,13 +354,18 @@ async def test_full_text_search_index(some_table: AsyncTable): async def test_create_vector_index(some_table: AsyncTable): # Can create await some_table.create_index("vector") + # Can't recreate by default + with pytest.raises(RuntimeError, match="already exists"): + await some_table.create_index("vector") # Can recreate if replace=True await some_table.create_index("vector", replace=True) # Can't recreate if replace=False with pytest.raises(RuntimeError, match="already exists"): await some_table.create_index("vector", replace=False) # Can also specify index type - await some_table.create_index("vector", config=IvfPq(num_partitions=100)) + await some_table.create_index( + "vector", config=IvfPq(num_partitions=100), replace=True + ) indices = await some_table.list_indices() assert len(indices) == 1 assert indices[0].index_type == "IvfPq" diff --git a/python/python/tests/test_table.py b/python/python/tests/test_table.py index 82ad045c8..6582d326d 100644 --- a/python/python/tests/test_table.py +++ b/python/python/tests/test_table.py @@ -1600,7 +1600,7 @@ def test_create_index_method(mock_create_index, mem_db: DBConnection): ) mock_create_index.assert_called_with( "my_vector", - replace=True, + replace=False, config=expected_config, wait_timeout=None, name=None, @@ -1620,7 +1620,7 @@ def test_create_index_method(mock_create_index, mem_db: DBConnection): ) mock_create_index.assert_called_with( "my_vector", - replace=True, + replace=False, config=expected_config, wait_timeout=None, name=None, @@ -1646,7 +1646,7 @@ def test_create_index_name_and_train_parameters( expected_config = IvfPq() # Default config mock_create_index.assert_called_with( "vector", - replace=True, + replace=False, config=expected_config, wait_timeout=None, name="my_custom_index", @@ -1657,7 +1657,7 @@ def test_create_index_name_and_train_parameters( table.create_index(vector_column_name="vector", train=False) mock_create_index.assert_called_with( "vector", - replace=True, + replace=False, config=expected_config, wait_timeout=None, name=None, @@ -1668,7 +1668,7 @@ def test_create_index_name_and_train_parameters( table.create_index(vector_column_name="vector", name="my_index_name", train=True) mock_create_index.assert_called_with( "vector", - replace=True, + replace=False, config=expected_config, wait_timeout=None, name="my_index_name", @@ -1705,7 +1705,7 @@ def test_create_index_new_api(mock_create_index, mem_db: DBConnection): table.create_index("vector", config=IvfPq(distance_type="l2")) mock_create_index.assert_called_with( "vector", - replace=True, + replace=False, config=IvfPq(distance_type="l2"), wait_timeout=None, name=None, @@ -1716,7 +1716,7 @@ def test_create_index_new_api(mock_create_index, mem_db: DBConnection): table.create_index("category", config=BTree()) mock_create_index.assert_called_with( "category", - replace=True, + replace=False, config=BTree(), wait_timeout=None, name=None, @@ -1727,7 +1727,7 @@ def test_create_index_new_api(mock_create_index, mem_db: DBConnection): table.create_index("text", config=FTS(with_position=True)) mock_create_index.assert_called_with( "text", - replace=True, + replace=False, config=FTS(with_position=True), wait_timeout=None, name=None, diff --git a/rust/lancedb/src/index.rs b/rust/lancedb/src/index.rs index c693dc056..9af757d64 100644 --- a/rust/lancedb/src/index.rs +++ b/rust/lancedb/src/index.rs @@ -200,14 +200,14 @@ impl IndexBuilder { parent, index, columns, - replace: true, + replace: false, train: true, wait_timeout: None, name: None, } } - /// Whether to replace the existing index, the default is `true`. + /// Whether to replace the existing index, the default is `false`. /// /// If this is false, and another index already exists on the same columns /// and the same name, then an error will be returned. This is true even if diff --git a/rust/lancedb/src/job.rs b/rust/lancedb/src/job.rs index 22f1a0450..5eca923c4 100644 --- a/rust/lancedb/src/job.rs +++ b/rust/lancedb/src/job.rs @@ -40,6 +40,7 @@ impl TerminalResult { } } + #[cfg(feature = "remote")] pub(crate) fn remote(value: Option, request_id: String) -> Self { Self { value, @@ -47,30 +48,46 @@ impl TerminalResult { } } + #[cfg(feature = "remote")] pub(crate) fn value(&self) -> Option<&Value> { self.value.as_ref() } fn decode(self) -> Result { - let value = self.value.ok_or_else(|| match &self.request_id { - Some(request_id) => Error::Http { - source: "successful typed job response did not contain a result".into(), - request_id: request_id.clone(), - status_code: None, - }, - None => Error::Runtime { + let value = self.value.ok_or_else(|| { + #[cfg(feature = "remote")] + if let Some(request_id) = &self.request_id { + return Error::Http { + source: "successful typed job response did not contain a result".into(), + request_id: request_id.clone(), + status_code: None, + }; + } + Error::Runtime { message: "successful typed job did not contain a result".to_string(), - }, + } })?; - serde_json::from_value(value).map_err(|error| match self.request_id { - Some(request_id) => Error::Http { - source: format!("failed to parse typed job result: {error}").into(), - request_id, - status_code: None, - }, - None => Error::Runtime { - message: format!("failed to parse typed job result: {error}"), - }, + serde_json::from_value(value).map_err(|error| { + #[cfg(feature = "remote")] + { + match self.request_id { + Some(request_id) => Error::Http { + source: format!("failed to parse typed job result: {error}").into(), + request_id, + status_code: None, + }, + None => Error::Runtime { + message: format!("failed to parse typed job result: {error}"), + }, + } + } + #[cfg(not(feature = "remote"))] + { + let _ = self.request_id; + Error::Runtime { + message: format!("failed to parse typed job result: {error}"), + } + } }) } } @@ -117,6 +134,7 @@ impl Job<()> { } } + #[cfg(feature = "remote")] pub(crate) fn new(handle: Box) -> Self { Self { inner: JobInner::Handle { diff --git a/rust/lancedb/src/remote/table.rs b/rust/lancedb/src/remote/table.rs index 5faffa8c7..f126bb8b7 100644 --- a/rust/lancedb/src/remote/table.rs +++ b/rust/lancedb/src/remote/table.rs @@ -524,13 +524,10 @@ impl RemoteTable { _ => resolve_arrow_field_path(&schema, &column)?, }; let mut body = serde_json::json!({ - "column": canonical_column + "column": canonical_column, + "replace": index.replace, }); - if !index.replace { - body["replace"] = false.into(); - } - // Add name parameter if provided (for backwards compatibility, only include if Some) if let Some(ref name) = index.name { body["name"] = serde_json::Value::String(name.clone()); @@ -6327,7 +6324,7 @@ mod tests { } #[tokio::test] - async fn test_create_index_forwards_replace_false_on_existing_route() { + async fn test_create_index_forwards_default_replace_false_on_existing_route() { let table = Table::new_with_handler("my_table", move |request| { assert_eq!(request.method(), "POST"); match request.url().path() { @@ -6354,7 +6351,40 @@ mod tests { table .create_index(&["a"], Index::BTree(Default::default())) - .replace(false) + .execute() + .await + .unwrap(); + } + + #[tokio::test] + async fn test_create_index_forwards_explicit_replace_true_on_existing_route() { + let table = Table::new_with_handler("my_table", move |request| { + assert_eq!(request.method(), "POST"); + match request.url().path() { + "/v1/table/my_table/describe/" => { + let schema = Schema::new(vec![Field::new("a", DataType::Int32, false)]); + http::Response::builder() + .status(200) + .body(describe_response(&schema)) + .unwrap() + } + "/v1/table/my_table/create_index/" => { + let body = request.body().unwrap().as_bytes().unwrap(); + let body: serde_json::Value = serde_json::from_slice(body).unwrap(); + assert_eq!(body["replace"], json!(true)); + + http::Response::builder() + .status(200) + .body("{}".to_string()) + .unwrap() + } + path => panic!("Unexpected path: {}", path), + } + }); + + table + .create_index(&["a"], Index::BTree(Default::default())) + .replace(true) .execute() .await .unwrap(); diff --git a/rust/lancedb/src/table/create_index.rs b/rust/lancedb/src/table/create_index.rs index e30c310ac..685adaf35 100644 --- a/rust/lancedb/src/table/create_index.rs +++ b/rust/lancedb/src/table/create_index.rs @@ -725,12 +725,11 @@ mod tests { .await .unwrap(); - // Rebuilding the same index without replace fails once the build + // Rebuilding the same index without explicit replace fails once the build // starts, so the failure reaches the job rather than execute_async. let job = Arc::new( table .create_index(&["id"], Index::BTree(BTreeIndexBuilder::default())) - .replace(false) .execute_async() .await .unwrap(), @@ -769,7 +768,6 @@ mod tests { let job = table .create_index(&["id"], Index::BTree(BTreeIndexBuilder::default())) - .replace(false) .execute_async() .await .unwrap(); @@ -1106,6 +1104,7 @@ mod tests { // Can also specify btree table .create_index(&["i"], Index::BTree(BTreeIndexBuilder::default())) + .replace(true) .execute() .await .unwrap();