diff --git a/rust/lance-namespace-impls/src/dir.rs b/rust/lance-namespace-impls/src/dir.rs index e2cac3504aa..6e6e61674b7 100644 --- a/rust/lance-namespace-impls/src/dir.rs +++ b/rust/lance-namespace-impls/src/dir.rs @@ -39,6 +39,7 @@ use lance_namespace::models::{ use lance_core::{Error, Result, box_error}; use lance_namespace::LanceNamespace; +use lance_namespace::NamespaceError; use lance_namespace::schema::arrow_schema_to_json; use crate::credentials::{ @@ -1198,9 +1199,10 @@ impl LanceNamespace for DirectoryNamespace { let status = self.check_table_status(&table_name).await; if !status.exists { - return Err(Error::namespace_source( - format!("Table does not exist: {}", table_name).into(), - )); + return Err(NamespaceError::TableNotFound { + message: table_name.to_string(), + } + .into()); } if status.is_deregistered { @@ -1346,9 +1348,10 @@ impl LanceNamespace for DirectoryNamespace { let status = self.check_table_status(&table_name).await; if !status.exists { - return Err(Error::namespace_source( - format!("Table does not exist: {}", table_name).into(), - )); + return Err(NamespaceError::TableNotFound { + message: table_name.to_string(), + } + .into()); } if status.is_deregistered { @@ -1564,9 +1567,10 @@ impl LanceNamespace for DirectoryNamespace { let status = self.check_table_status(&table_name).await; if !status.exists { - return Err(Error::namespace_source( - format!("Table does not exist: {}", table_name).into(), - )); + return Err(NamespaceError::TableNotFound { + message: table_name.to_string(), + } + .into()); } if status.is_deregistered { @@ -1998,10 +2002,24 @@ mod tests { use arrow_ipc::reader::StreamReader; use lance::dataset::Dataset; use lance_core::utils::tempfile::{TempStdDir, TempStrDir}; + use lance_namespace::error::{ErrorCode, NamespaceError}; use lance_namespace::models::{ CreateTableRequest, JsonArrowDataType, JsonArrowField, JsonArrowSchema, ListTablesRequest, }; use lance_namespace::schema::convert_json_arrow_schema; + + /// Assert that a `lance_core::Error` wraps a `NamespaceError::TableNotFound`. + fn assert_table_not_found(err: &lance_core::Error) { + match err { + lance_core::Error::Namespace { source, .. } => { + let ns_err = source + .downcast_ref::() + .expect("source should be NamespaceError"); + assert_eq!(ns_err.code(), ErrorCode::TableNotFound); + } + other => panic!("Expected Namespace error, got: {other}"), + } + } use std::io::Cursor; use std::sync::Arc; @@ -2231,13 +2249,9 @@ mod tests { request.id = Some(vec!["nonexistent".to_string()]); let result = namespace.describe_table(request).await; - assert!(result.is_err()); - assert!( - result - .unwrap_err() - .to_string() - .contains("Table does not exist") - ); + let err = result.unwrap_err(); + assert_table_not_found(&err); + assert!(err.to_string().contains("Table not found")); } #[tokio::test] @@ -2265,13 +2279,9 @@ mod tests { let mut request = TableExistsRequest::new(); request.id = Some(vec!["nonexistent".to_string()]); let result = namespace.table_exists(request).await; - assert!(result.is_err()); - assert!( - result - .unwrap_err() - .to_string() - .contains("Table does not exist") - ); + let err = result.unwrap_err(); + assert_table_not_found(&err); + assert!(err.to_string().contains("Table not found")); } #[tokio::test] @@ -3778,6 +3788,27 @@ mod tests { assert!(dataset.is_ok(), "Physical table data should still exist"); } + #[tokio::test] + async fn test_deregister_nonexistent_table_v1() { + use lance_namespace::models::DeregisterTableRequest; + + let temp_dir = TempStdDir::default(); + let temp_path = temp_dir.to_str().unwrap(); + + let namespace = DirectoryNamespaceBuilder::new(temp_path) + .manifest_enabled(false) + .dir_listing_enabled(true) + .build() + .await + .unwrap(); + + let mut request = DeregisterTableRequest::new(); + request.id = Some(vec!["nonexistent".to_string()]); + let err = namespace.deregister_table(request).await.unwrap_err(); + assert_table_not_found(&err); + assert!(err.to_string().contains("Table not found")); + } + #[tokio::test] async fn test_deregister_table_v1_already_deregistered() { use lance_namespace::models::DeregisterTableRequest; @@ -4634,8 +4665,8 @@ mod tests { ); let err_msg = result.unwrap_err().to_string(); assert!( - err_msg.contains("does not exist"), - "Error should mention table does not exist, got: {}", + err_msg.contains("not found"), + "Error should mention table not found, got: {}", err_msg ); } diff --git a/rust/lance-namespace-impls/src/dir/manifest.rs b/rust/lance-namespace-impls/src/dir/manifest.rs index 902c5462acc..248edd1edf2 100644 --- a/rust/lance-namespace-impls/src/dir/manifest.rs +++ b/rust/lance-namespace-impls/src/dir/manifest.rs @@ -1838,9 +1838,10 @@ impl LanceNamespace for ManifestNamespace { } } } - None => Err(Error::namespace_source( - format!("Table '{}' not found", object_id).into(), - )), + None => Err(NamespaceError::TableNotFound { + message: object_id.to_string(), + } + .into()), } } @@ -1862,9 +1863,10 @@ impl LanceNamespace for ManifestNamespace { if exists { Ok(()) } else { - Err(Error::namespace_source( - format!("Table '{}' not found", table_name).into(), - )) + Err(NamespaceError::TableNotFound { + message: table_name.to_string(), + } + .into()) } } @@ -2010,9 +2012,10 @@ impl LanceNamespace for ManifestNamespace { ..Default::default() }) } - None => Err(Error::namespace_source( - format!("Table '{}' not found", table_name).into(), - )), + None => Err(NamespaceError::TableNotFound { + message: table_name.to_string(), + } + .into()), } } @@ -2438,9 +2441,10 @@ impl LanceNamespace for ManifestNamespace { Self::construct_full_uri(&self.root, &info.location)? } None => { - return Err(Error::namespace_source( - format!("Table '{}' not found", object_id).into(), - )); + return Err(NamespaceError::TableNotFound { + message: object_id.to_string(), + } + .into()); } }; @@ -2458,12 +2462,26 @@ mod tests { use bytes::Bytes; use lance_core::utils::tempfile::TempStdDir; use lance_namespace::LanceNamespace; + use lance_namespace::error::{ErrorCode, NamespaceError}; use lance_namespace::models::{ CreateNamespaceRequest, CreateTableRequest, DescribeTableRequest, DropTableRequest, ListTablesRequest, TableExistsRequest, }; use rstest::rstest; + /// Assert that a `lance_core::Error` wraps a `NamespaceError::TableNotFound`. + fn assert_table_not_found(err: &lance_core::Error) { + match err { + lance_core::Error::Namespace { source, .. } => { + let ns_err = source + .downcast_ref::() + .expect("source should be NamespaceError"); + assert_eq!(ns_err.code(), ErrorCode::TableNotFound); + } + other => panic!("Expected Namespace error, got: {other}"), + } + } + fn create_test_ipc_data() -> Vec { use arrow::array::{Int32Array, StringArray}; use arrow::datatypes::{DataType, Field, Schema}; @@ -2551,7 +2569,7 @@ mod tests { let mut request = TableExistsRequest::new(); request.id = Some(vec!["nonexistent".to_string()]); let result = dir_namespace.table_exists(request).await; - assert!(result.is_err()); + assert_table_not_found(&result.unwrap_err()); // Create table let buffer = create_test_ipc_data(); @@ -2587,7 +2605,7 @@ mod tests { let mut request = DescribeTableRequest::new(); request.id = Some(vec!["nonexistent".to_string()]); let result = dir_namespace.describe_table(request).await; - assert!(result.is_err()); + assert_table_not_found(&result.unwrap_err()); // Create table let buffer = create_test_ipc_data(); @@ -2809,7 +2827,30 @@ mod tests { let mut drop_request = DropTableRequest::new(); drop_request.id = Some(vec!["nonexistent".to_string()]); let result = dir_namespace.drop_table(drop_request).await; - assert!(result.is_err()); + assert_table_not_found(&result.unwrap_err()); + } + + #[rstest] + #[case::with_optimization(true)] + #[case::without_optimization(false)] + #[tokio::test] + async fn test_deregister_nonexistent_table(#[case] inline_optimization: bool) { + use lance_namespace::models::DeregisterTableRequest; + + let temp_dir = TempStdDir::default(); + let temp_path = temp_dir.to_str().unwrap(); + + let dir_namespace = DirectoryNamespaceBuilder::new(temp_path) + .inline_optimization_enabled(inline_optimization) + .build() + .await + .unwrap(); + + let mut request = DeregisterTableRequest::new(); + request.id = Some(vec!["nonexistent".to_string()]); + let err = dir_namespace.deregister_table(request).await.unwrap_err(); + assert_table_not_found(&err); + assert!(err.to_string().contains("Table not found")); } #[rstest]