mirror of
https://github.com/lancedb/lancedb.git
synced 2026-11-14 06:36:54 +00:00
fix(storage): probe actual table store capability
This commit is contained in:
@@ -258,8 +258,8 @@ pub struct ListingDatabase {
|
||||
// Session for object stores and caching
|
||||
session: Arc<lance::session::Session>,
|
||||
|
||||
// Whether failed opens may inspect exact native directory metadata. This is
|
||||
// granted only for stores resolved through Lance's built-in local providers.
|
||||
// Whether the session uses the default registry that LanceDB created. The
|
||||
// actual table store is still resolved and checked for each open location.
|
||||
table_storage_probe: TableStorageProbe,
|
||||
|
||||
// Namespace-backed database for child namespace operations
|
||||
@@ -572,8 +572,7 @@ impl ListingDatabase {
|
||||
&os_params,
|
||||
)
|
||||
.await?;
|
||||
let table_storage_probe =
|
||||
TableStorageProbe::for_resolved_store(&object_store, uses_default_registry);
|
||||
let table_storage_probe = TableStorageProbe::for_registry(uses_default_registry);
|
||||
if object_store.is_local() {
|
||||
Self::try_create_dir(&storage_base_uri).context(CreateDirSnafu {
|
||||
path: storage_base_uri.clone(),
|
||||
@@ -641,8 +640,7 @@ impl ListingDatabase {
|
||||
&ObjectStoreParams::default(),
|
||||
)
|
||||
.await?;
|
||||
let table_storage_probe =
|
||||
TableStorageProbe::for_resolved_store(&object_store, uses_default_registry);
|
||||
let table_storage_probe = TableStorageProbe::for_registry(uses_default_registry);
|
||||
if object_store.is_local() {
|
||||
Self::try_create_dir(path).context(CreateDirSnafu { path })?;
|
||||
}
|
||||
|
||||
+86
-24
@@ -179,25 +179,23 @@ async fn map_dataset_not_found(
|
||||
|
||||
/// How a failed table open may distinguish an empty directory from a missing table.
|
||||
///
|
||||
/// Object stores cannot represent empty directories, so the default is to use only
|
||||
/// exact-key and target-prefix probes. Native directory metadata is permitted only
|
||||
/// when the connection resolved the store through Lance's built-in local provider.
|
||||
/// Object stores cannot represent empty directories, so an unknown provider uses only
|
||||
/// exact-key and target-prefix probes unless its resolved store proves that it shares
|
||||
/// the native filesystem namespace. Connections that create Lance's default registry
|
||||
/// internally can trust its built-in local providers without the extra proof.
|
||||
#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
|
||||
pub(crate) enum TableStorageProbe {
|
||||
#[default]
|
||||
ObjectStore,
|
||||
NativeDirectory,
|
||||
VerifyResolvedStore,
|
||||
TrustedDefaultRegistry,
|
||||
}
|
||||
|
||||
impl TableStorageProbe {
|
||||
pub(crate) fn for_resolved_store(
|
||||
object_store: &lance_io::object_store::ObjectStore,
|
||||
uses_default_registry: bool,
|
||||
) -> Self {
|
||||
if uses_default_registry && Self::supports_native_directories(object_store) {
|
||||
Self::NativeDirectory
|
||||
pub(crate) fn for_registry(uses_default_registry: bool) -> Self {
|
||||
if uses_default_registry {
|
||||
Self::TrustedDefaultRegistry
|
||||
} else {
|
||||
Self::ObjectStore
|
||||
Self::VerifyResolvedStore
|
||||
}
|
||||
}
|
||||
|
||||
@@ -209,6 +207,30 @@ impl TableStorageProbe {
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
async fn resolved_store_supports_native_directories(
|
||||
self,
|
||||
object_store: &lance_io::object_store::ObjectStore,
|
||||
) -> bool {
|
||||
if !Self::supports_native_directories(object_store) {
|
||||
return false;
|
||||
}
|
||||
if self == Self::TrustedDefaultRegistry {
|
||||
return true;
|
||||
}
|
||||
|
||||
// `ObjectStoreProvider` does not expose concrete type identity or a directory
|
||||
// capability. Verify the resolved store behavior with one exact, unrelated
|
||||
// local file instead. This is bounded and prevents a custom provider that only
|
||||
// reports a `file` scheme from being bypassed by host filesystem metadata.
|
||||
let Ok(probe) = tempfile::NamedTempFile::new() else {
|
||||
return false;
|
||||
};
|
||||
let Ok(probe_path) = object_store::path::Path::from_absolute_path(probe.path()) else {
|
||||
return false;
|
||||
};
|
||||
object_store.exists(&probe_path).await.unwrap_or(false)
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether storage contains an entry for `uri`, even though no dataset could be loaded.
|
||||
@@ -240,18 +262,12 @@ async fn table_storage_exists(
|
||||
return Ok(false);
|
||||
}
|
||||
|
||||
let storage_probe = if requires_store_probe {
|
||||
TableStorageProbe::ObjectStore
|
||||
} else {
|
||||
storage_probe.unwrap_or_else(|| {
|
||||
TableStorageProbe::for_resolved_store(&object_store, uses_default_registry)
|
||||
})
|
||||
};
|
||||
// The scheme is only a necessary path-format check here. Provider provenance is
|
||||
// carried separately in `storage_probe`, so a custom provider that reports `file`
|
||||
// cannot cause us to bypass its ObjectStore implementation.
|
||||
if storage_probe == TableStorageProbe::NativeDirectory
|
||||
&& TableStorageProbe::supports_native_directories(&object_store)
|
||||
let storage_probe =
|
||||
storage_probe.unwrap_or_else(|| TableStorageProbe::for_registry(uses_default_registry));
|
||||
if !requires_store_probe
|
||||
&& storage_probe
|
||||
.resolved_store_supports_native_directories(&object_store)
|
||||
.await
|
||||
{
|
||||
let local_path = std::path::PathBuf::from(lance_io::local::to_local_path(&path));
|
||||
// A local table URI intentionally grants access to this exact path; LanceDB does
|
||||
@@ -4253,6 +4269,52 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_open_recovery_detects_empty_local_dir_with_explicit_default_session() {
|
||||
let tmp_dir = tempdir().unwrap();
|
||||
let table_name = "explicit-default-session";
|
||||
std::fs::create_dir(tmp_dir.path().join(format!("{table_name}.lance"))).unwrap();
|
||||
|
||||
let db = connect(tmp_dir.path().to_str().unwrap())
|
||||
.session(Arc::new(lance::session::Session::default()))
|
||||
.execute()
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
db.table_names().execute().await.unwrap(),
|
||||
vec![table_name.to_string()]
|
||||
);
|
||||
let err = db.open_table(table_name).execute().await.unwrap_err();
|
||||
assert!(
|
||||
matches!(&err, Error::TableCorrupted { name, .. } if name == table_name),
|
||||
"listed empty table must be corrupt, got {err:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_open_recovery_detects_empty_local_custom_location() {
|
||||
let tmp_dir = tempdir().unwrap();
|
||||
let table_name = "local-location";
|
||||
let table_path = tmp_dir.path().join(format!("{table_name}.lance"));
|
||||
std::fs::create_dir(&table_path).unwrap();
|
||||
|
||||
let db = connect("memory:///open-local-location-repro")
|
||||
.execute()
|
||||
.await
|
||||
.unwrap();
|
||||
let err = db
|
||||
.open_table(table_name)
|
||||
.location(table_path.to_str().unwrap())
|
||||
.execute()
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(
|
||||
matches!(&err, Error::TableCorrupted { name, .. } if name == table_name),
|
||||
"empty built-in local location must be corrupt, got {err:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
struct OpenRecoveryTestStore {
|
||||
inner: Arc<dyn object_store::ObjectStore>,
|
||||
|
||||
Reference in New Issue
Block a user