From 6b6d55e6c72e1ab18c7cf52a3526f2b41e3a2b82 Mon Sep 17 00:00:00 2001 From: "lancedb-gatefixer[bot]" <313497061+lancedb-gatefixer[bot]@users.noreply.github.com> Date: Tue, 6 Oct 2026 16:14:57 -0700 Subject: [PATCH] fix: accept updated_rows in remote update responses (#4444) Remote update responses report their count as `updated_rows`, but the client only recognized `rows_updated` and silently defaulted the count to zero. Python sync/async and TypeScript both use this Rust result. Accept `updated_rows` as a Serde alias while retaining `rows_updated` and compatibility with older responses. Extend the remote mock test to cover both keys, zero and missing counts, returned versions, and empty responses, with and without a predicate. Validation: - Both nonzero `updated_rows` regression cases failed before the fix (0 instead of 5); all 11 focused remote tests pass after it. - All 3 native update tests pass. - `cargo fmt --all` - `cargo clippy --quiet --profile ci --features remote -p lancedb --tests --examples` Fixes #4443 --------- Co-authored-by: Gatefixer <313497061+lancedb-gatefixer[bot]@users.noreply.github.com> --- rust/lancedb/src/remote/table.rs | 45 ++++++++++++++++++-------------- rust/lancedb/src/table/update.rs | 2 +- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/rust/lancedb/src/remote/table.rs b/rust/lancedb/src/remote/table.rs index 35114cded..1ea72bf14 100644 --- a/rust/lancedb/src/remote/table.rs +++ b/rust/lancedb/src/remote/table.rs @@ -4620,10 +4620,18 @@ mod tests { } #[rstest] - #[case(true)] - #[case(false)] + #[case::old_server("", 0, 0)] + #[case::rows_updated(r#"{"rows_updated": 5, "version": 43}"#, 5, 43)] + #[case::updated_rows(r#"{"updated_rows": 5, "version": 43}"#, 5, 43)] + #[case::zero_updated_rows(r#"{"updated_rows": 0, "version": 43}"#, 0, 43)] + #[case::missing_row_count(r#"{"version": 43}"#, 0, 43)] #[tokio::test] - async fn test_update(#[case] old_server: bool) { + async fn test_update( + #[case] response_body: &'static str, + #[case] expected_rows_updated: u64, + #[case] expected_version: u64, + #[values(true, false)] filtered: bool, + ) { let table = Table::new_with_handler("my_table", move |request| { if request.url().path() == "/v1/table/my_table/update/" { assert_eq!(request.method(), "POST"); @@ -4648,32 +4656,29 @@ mod tests { assert_eq!(col_name, "b"); assert_eq!(expression, "b - 1"); - let only_if = value.get("predicate").unwrap().as_str().unwrap(); - assert_eq!(only_if, "`B` > 10"); + assert_eq!( + value.get("predicate").unwrap(), + &serde_json::json!(if filtered { Some("`B` > 10") } else { None }) + ); } - if old_server { - http::Response::builder().status(200).body("").unwrap() - } else { - http::Response::builder() - .status(200) - .body(r#"{"rows_updated": 5, "version": 43}"#) - .unwrap() - } + http::Response::builder() + .status(200) + .body(response_body) + .unwrap() } else { panic!("Unexpected request path: {}", request.url().path()); } }); - let update = table - .update() - .column("a", "a + 1") - .column("b", "b - 1") - .only_if(r#""B" > 10"#); + let mut update = table.update().column("a", "a + 1").column("b", "b - 1"); + if filtered { + update = update.only_if(r#""B" > 10"#); + } let result = table.base_table().update(update).await.unwrap(); - assert_eq!(result.version, if old_server { 0 } else { 43 }); - assert_eq!(result.rows_updated, if old_server { 0 } else { 5 }); + assert_eq!(result.version, expected_version); + assert_eq!(result.rows_updated, expected_rows_updated); } #[tokio::test] diff --git a/rust/lancedb/src/table/update.rs b/rust/lancedb/src/table/update.rs index f10594f23..5ffcb74a9 100644 --- a/rust/lancedb/src/table/update.rs +++ b/rust/lancedb/src/table/update.rs @@ -13,7 +13,7 @@ use crate::Result; /// The result of an update operation #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)] pub struct UpdateResult { - #[serde(default)] + #[serde(default, alias = "updated_rows")] pub rows_updated: u64, /// The commit version associated with the operation. #[serde(default)]