mirror of
https://github.com/lancedb/lancedb.git
synced 2026-10-26 05:26:14 +00:00
fix: validate cached remote tables on open (#4557)
## Summary Fixes #4401. - Validate cached remote table entries with the existing `describe` endpoint before returning a handle. - Preserve cached server-version behavior after a successful validation. - Add a regression test for a table dropped by another connection after its entry has been cached. A cached entry can otherwise outlive an external drop, allowing `open_table()` to return a handle that fails only on its first later operation. ## Verification [Linux validation](https://github.com/Ganesh1729-ui/lancedb/actions/runs/39420560974) passed against the exact runtime and test sources at PR head `d39092b01f6b49921b5d66ccedf29709a7ddc23d`, using the repository's Rust toolchain and `ci` profile: - `cargo fmt --all -- --check` - `cargo test --profile ci --locked -p lancedb --features remote --lib remote::db::tests::test_open_table -- --nocapture`: **8 passed**, including cached-table deletion, independent handle checkout, branch/version handling, schema seeding and describe-response handling. - Removing only the cached-hit existence guard makes `test_open_table_checks_that_a_cached_table_still_exists` fail at its `TableNotFound` assertion; restoring the fix makes the same test pass again. - `git diff --check` The validation workflow is on a separate fork branch and is excluded from this PR. These are focused, mock-backed remote unit tests; the full upstream matrix and live Cloud integration tests still need upstream CI approval. Local Windows compilation was unavailable because the MSVC environment has no `link.exe`; the linked Linux run completed successfully. --------- Co-authored-by: Ganesh172919 <ganeshreddy17291122@gmail.com> Co-authored-by: Xuanwo <github@xuanwo.io>
This commit is contained in:
co-authored by
Ganesh172919
Xuanwo
parent
a8305e0722
commit
cf5c9b5453
@@ -110,8 +110,18 @@ ignore = [
|
||||
# compact_str/smol_str, so clearing this requires polars to migrate.
|
||||
# https://rustsec.org/advisories/RUSTSEC-2026-0249
|
||||
{ id = "RUSTSEC-2026-0249", reason = "smartstring unmaintained via polars; no fixed upstream release" },
|
||||
# crypto-bigint < 0.7.6: timing side-channels in constant-time ops,
|
||||
# incl. subtraction, zero counting, division and Montgomery multiplication.
|
||||
# Reached only through aws-sdk-s3 sigv4a -> aws-sigv4 (p256/ecdsa); signed
|
||||
# requests carry the SigV4a credential-derived key. The patched 0.7.6 was
|
||||
# published 2026-10-23, but no published aws-sigv4 release uses it yet
|
||||
# (latest 1.6.3 pins crypto-bigint ^0.5), so clearing requires smithy-rs to
|
||||
# ship an updated aws-sigv4.
|
||||
# https://rustsec.org/advisories/RUSTSEC-2026-0387
|
||||
{ id = "RUSTSEC-2026-0387", reason = "crypto-bigint via aws-sigv4 (sigv4a); no fixed upstream aws-sigv4 release yet" },
|
||||
]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Licenses: only allow licenses we've reviewed as compatible with Apache-2.0.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -1614,6 +1614,17 @@ impl<S: HttpSend> Database for RemoteDatabase<S> {
|
||||
|
||||
// Every open gets its own checkout, schema cache, and freshness state.
|
||||
if let Some(version) = self.table_cache.get(&cache_key).await {
|
||||
// A cached version can outlive a drop from another connection. Verify that
|
||||
// the table still exists before returning a handle for it.
|
||||
let req = self
|
||||
.client
|
||||
.post(&format!("/v1/table/{}/describe/", identifier))
|
||||
.json(&serde_json::json!({}));
|
||||
let (request_id, rsp) = self.client.send_with_retry(req, None, true).await?;
|
||||
let rsp =
|
||||
RemoteTable::<S>::handle_table_not_found(&request.name, rsp, &request_id).await?;
|
||||
self.client.check_response(&request_id, rsp).await?;
|
||||
|
||||
Ok(Arc::new(RemoteTable::new_with_sql_client(
|
||||
self.client.clone(),
|
||||
request.name,
|
||||
@@ -2557,6 +2568,37 @@ mod tests {
|
||||
assert_eq!(table.name(), "table1");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_open_table_checks_that_a_cached_table_still_exists() {
|
||||
let describe_calls = Arc::new(AtomicUsize::new(0));
|
||||
let calls = describe_calls.clone();
|
||||
let mut db = super::RemoteDatabase::new_mock(move |request| {
|
||||
assert_eq!(request.url().path(), "/v1/table/table1/describe/");
|
||||
match calls.fetch_add(1, Ordering::SeqCst) {
|
||||
0 => http::Response::builder()
|
||||
.status(200)
|
||||
.body(r#"{"version": 1}"#.to_string())
|
||||
.unwrap(),
|
||||
1 => http::Response::builder()
|
||||
.status(404)
|
||||
.body("table not found".to_string())
|
||||
.unwrap(),
|
||||
call => panic!("unexpected describe call: {call}"),
|
||||
}
|
||||
});
|
||||
db.table_cache = moka::future::Cache::new(10);
|
||||
let conn = Connection::new(
|
||||
Arc::new(db),
|
||||
Arc::new(crate::embeddings::MemoryRegistry::new()),
|
||||
);
|
||||
|
||||
conn.open_table("table1").execute().await.unwrap();
|
||||
let result = conn.open_table("table1").execute().await;
|
||||
|
||||
assert!(matches!(result, Err(Error::TableNotFound { .. })));
|
||||
assert_eq!(describe_calls.load(Ordering::SeqCst), 2);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_open_table_checkout_is_independent_per_handle() {
|
||||
let latest = Arc::new(AtomicUsize::new(2));
|
||||
|
||||
Reference in New Issue
Block a user