From ca32647f2342e0e65005d40e57e484ed6638964b Mon Sep 17 00:00:00 2001 From: Gatefixer <313497061+lancedb-gatefixer[bot]@users.noreply.github.com> Date: Wed, 23 Sep 2026 19:58:58 +0000 Subject: [PATCH] fix: omit unfetchable list blobs from column discovery --- docs/src/js/classes/Table.md | 4 +- docs/src/js/functions/blob.md | 2 + docs/src/python/python.md | 4 + nodejs/__test__/table.test.ts | 14 ++- nodejs/lancedb/blob.ts | 2 + nodejs/lancedb/table.ts | 4 +- python/python/lancedb/table.py | 8 +- python/python/tests/test_blob.py | 10 ++- rust/lancedb/src/blob.rs | 115 ++++++++++++++++++++++--- rust/lancedb/src/remote/table.rs | 64 ++++++++++++-- rust/lancedb/src/remote/table/blobs.rs | 40 ++++++++- rust/lancedb/src/table.rs | 14 +-- rust/lancedb/tests/blob_integration.rs | 41 +++++++++ 13 files changed, 288 insertions(+), 34 deletions(-) diff --git a/docs/src/js/classes/Table.md b/docs/src/js/classes/Table.md index a8c382c27..e4b2a89c6 100644 --- a/docs/src/js/classes/Table.md +++ b/docs/src/js/classes/Table.md @@ -143,7 +143,8 @@ containing the new version number of the table after altering the columns. abstract blobColumns(): Promise ``` -Blob v2 columns, including nested dotted paths. +Blob v2 columns addressable by row ID, including struct-nested dotted paths. +Blobs inside lists are omitted because a row ID does not identify an element. #### Returns @@ -548,6 +549,7 @@ Bytes for `column` at row IDs from [Query.withRowId](Query.md#withrowid). Reads the table's current checkout. IDs from another version can fail after compaction unless stable row ids are enabled. Results keep input order and duplicates. Null blobs are `null`. Empty blobs are empty buffers. +Blobs inside lists cannot be fetched by row ID. #### Parameters diff --git a/docs/src/js/functions/blob.md b/docs/src/js/functions/blob.md index 20a734cd4..1a65b5133 100644 --- a/docs/src/js/functions/blob.md +++ b/docs/src/js/functions/blob.md @@ -14,6 +14,8 @@ Declares a `lance.blob.v2` column. Query results are descriptors, not payload bytes. Use [Table.fetchBlobs](../classes/Table.md#fetchblobs) or [Table.fetchBlobFiles](../classes/Table.md#fetchblobfiles) to read bytes. +Blobs inside lists can be written and queried as descriptors, but cannot +currently be fetched because a row ID does not identify a list element. ## Parameters diff --git a/docs/src/python/python.md b/docs/src/python/python.md index 68cd306c4..06747d362 100644 --- a/docs/src/python/python.md +++ b/docs/src/python/python.md @@ -323,6 +323,10 @@ still work. Queries return descriptors. Call [`fetch_blob_files`][lancedb.table.Table.fetch_blob_files] for lazy reads or [`fetch_blobs`][lancedb.table.Table.fetch_blobs] for eager bytes. +Blobs inside lists can be written and queried as descriptors. They are omitted +from `blob_columns()` because row ids do not identify individual list elements, +so the fetch APIs cannot read their bytes yet. + ::: lancedb.blob ::: lancedb._blob.BlobFile diff --git a/nodejs/__test__/table.test.ts b/nodejs/__test__/table.test.ts index 4142af696..6f90b6033 100644 --- a/nodejs/__test__/table.test.ts +++ b/nodejs/__test__/table.test.ts @@ -2500,7 +2500,7 @@ describe("when dealing with blob columns", () => { expect(bytes[0]!.equals(payload)).toBe(true); }); - it("creates and adds list blob columns", async () => { + it("creates list blobs without advertising unfetchable elements", async () => { const db = await connect(tmpDir.name); const schema = new Schema([ new Field("id", new Int64(), true), @@ -2519,7 +2519,16 @@ describe("when dealing with blob columns", () => { { id: 3n, images: [gamma, null] }, { id: 4n, images: [] }, ]); - expect(await table.blobColumns()).toEqual(["images.image"]); + expect(await table.blobColumns()).toEqual([]); + const [first] = await table.query().withRowId().toArray(); + for (const column of ["images", "images.image"]) { + await expect( + table.fetchBlobs(column, [first._rowid as bigint]), + ).rejects.toThrow(/blobs inside lists cannot be fetched/); + await expect( + table.fetchBlobFiles(column, [first._rowid as bigint]), + ).rejects.toThrow(/blobs inside lists cannot be fetched/); + } const rows = await table.query().toArray(); const byId = new Map(rows.map((row) => [Number(row.id), row])); expect(descriptorSizes(byId.get(1)!.images)).toEqual([ @@ -2565,6 +2574,7 @@ describe("when dealing with blob columns", () => { ], }, ]); + expect(await table.blobColumns()).toEqual([]); const rows = await table.query().toArray(); const byId = new Map(rows.map((row) => [Number(row.id), row])); expect( diff --git a/nodejs/lancedb/blob.ts b/nodejs/lancedb/blob.ts index 244faac09..049d302e1 100644 --- a/nodejs/lancedb/blob.ts +++ b/nodejs/lancedb/blob.ts @@ -42,6 +42,8 @@ export type BlobOptions = { * * Query results are descriptors, not payload bytes. Use {@link Table.fetchBlobs} * or {@link Table.fetchBlobFiles} to read bytes. + * Blobs inside lists can be written and queried as descriptors, but cannot + * currently be fetched because a row ID does not identify a list element. * * @example * ```ts diff --git a/nodejs/lancedb/table.ts b/nodejs/lancedb/table.ts index a098322a9..e60c09267 100644 --- a/nodejs/lancedb/table.ts +++ b/nodejs/lancedb/table.ts @@ -516,7 +516,8 @@ export abstract class Table { abstract takeRowIds(rowIds: readonly (bigint | number)[]): TakeQuery; /** - * Blob v2 columns, including nested dotted paths. + * Blob v2 columns addressable by row ID, including struct-nested dotted paths. + * Blobs inside lists are omitted because a row ID does not identify an element. */ abstract blobColumns(): Promise; @@ -526,6 +527,7 @@ export abstract class Table { * Reads the table's current checkout. IDs from another version can fail after * compaction unless stable row ids are enabled. Results keep input order and * duplicates. Null blobs are `null`. Empty blobs are empty buffers. + * Blobs inside lists cannot be fetched by row ID. */ abstract fetchBlobs( column: string, diff --git a/python/python/lancedb/table.py b/python/python/lancedb/table.py index 297b4237f..95ff79914 100644 --- a/python/python/lancedb/table.py +++ b/python/python/lancedb/table.py @@ -1920,7 +1920,11 @@ class Table(ABC): @abstractmethod def blob_columns(self) -> list[str]: - """Names of the blob v2 columns declared on this table.""" + """Names of blob v2 columns that can be fetched by row id. + + Struct-nested blobs use dotted paths. Blobs inside lists are omitted + because a row id does not identify an individual list element. + """ @abstractmethod def fetch_blobs( @@ -1936,6 +1940,8 @@ class Table(ABC): Convenience for small payloads. For large values use :meth:`fetch_blob_files`. + + Blobs inside lists cannot be fetched by row id yet. """ @abstractmethod diff --git a/python/python/tests/test_blob.py b/python/python/tests/test_blob.py index 0d3a89c21..4dbf2bd49 100644 --- a/python/python/tests/test_blob.py +++ b/python/python/tests/test_blob.py @@ -660,12 +660,18 @@ def test_merge_insert_list_blob_dicts(): def test_list_blob_column_queries_as_raw_descriptors(): table = _list_blob_table("list_query") + assert table.blob_columns() == [] hits = table.search().limit(10).to_arrow() element = hits.schema.field("images").type.value_type assert pa.types.is_struct(element) assert "_lance_row_id" not in element.names - with pytest.raises(ValueError, match="expected struct before segment"): - table.fetch_blobs("images.image", [0]) + for column in ("images", "images.image"): + with pytest.raises(ValueError, match="blobs inside lists cannot be fetched"): + table.fetch_blobs(column, [0]) + with pytest.raises(ValueError, match="blobs inside lists cannot be fetched"): + table.fetch_blob_files(column, [0]) + with pytest.raises(ValueError, match="blobs inside lists cannot be fetched"): + table.fetch_blob_ranges(column, [(0, 0, 1)]) def test_row_addressable_paths_exclude_list_children(): diff --git a/rust/lancedb/src/blob.rs b/rust/lancedb/src/blob.rs index cbc0724c6..b0c38bac9 100644 --- a/rust/lancedb/src/blob.rs +++ b/rust/lancedb/src/blob.rs @@ -247,8 +247,10 @@ impl BlobRangeRequest { /// `Struct` with the `lance.blob.v2` marker. Same layout Lance /// expects on write. /// -/// A blob column may be top-level or nested inside a struct or list. Nested -/// blobs are addressed by a dotted path (e.g. `info.blob`) in the read APIs. +/// A blob column may be top-level or nested inside a struct or list. Blobs +/// nested inside structs use dotted paths (e.g. `info.blob`) in the read APIs. +/// Blobs inside lists can be written and queried as descriptors, but cannot +/// currently be fetched by row id. /// /// ``` /// use arrow_schema::{DataType, Field, Schema}; @@ -286,7 +288,7 @@ fn field_tree_has_blob_v2(field: &Field) -> bool { } } -/// Collects the dotted paths of blob v2 columns under `field`, into `paths`. +/// Collects the dotted paths of row-addressable blob v2 columns under `field`. fn collect_blob_paths(field: &Field, prefix: &str, paths: &mut Vec) { let path = if prefix.is_empty() { field.name().clone() @@ -297,26 +299,58 @@ fn collect_blob_paths(field: &Field, prefix: &str, paths: &mut Vec) { paths.push(path); return; } - match field.data_type() { - DataType::Struct(children) => { - for child in children { - collect_blob_paths(child, &path, paths); - } + if let DataType::Struct(children) = field.data_type() { + for child in children { + collect_blob_paths(child, &path, paths); } - DataType::List(child) | DataType::LargeList(child) | DataType::FixedSizeList(child, _) => { - collect_blob_paths(child, &path, paths) - } - _ => {} } } +fn is_list_blob_path(field: &Field, prefix: &str, column: &str, inside_list: bool) -> bool { + let path = if prefix.is_empty() { + field.name().clone() + } else { + format!("{prefix}.{}", field.name()) + }; + if field.is_blob_v2() { + return inside_list && column == path; + } + match field.data_type() { + DataType::Struct(children) => children + .iter() + .any(|child| is_list_blob_path(child, &path, column, inside_list)), + DataType::List(child) | DataType::LargeList(child) | DataType::FixedSizeList(child, _) => { + (column == path && field_tree_has_blob_v2(child)) + || is_list_blob_path(child, &path, column, true) + } + _ => false, + } +} + +/// Give list blob paths a useful error before they reach the row-based reader. +pub(crate) fn reject_list_blob_column(schema: &Schema, column: &str) -> Result<()> { + if schema + .fields() + .iter() + .any(|field| is_list_blob_path(field, "", column, false)) + { + return Err(Error::InvalidInput { + message: format!( + "column '{column}' contains a blob inside a list; blobs inside lists cannot be fetched by row id yet" + ), + }); + } + Ok(()) +} + /// Returns true if `schema` declares any blob v2 column, including nested ones. pub(crate) fn has_blob_columns(schema: &Schema) -> bool { schema.fields().iter().any(|f| field_tree_has_blob_v2(f)) } -/// Blob v2 column paths in `schema`, declaration order preserved. Nested blobs -/// are dotted paths (e.g. `info.blob`). +/// Row-addressable blob v2 column paths in `schema`, in declaration order. +/// Struct-nested blobs use dotted paths (e.g. `info.blob`); list-nested blobs +/// are omitted because a row id cannot identify an individual list element. pub(crate) fn blob_column_names(schema: &Schema) -> Vec { let mut paths = Vec::new(); for field in schema.fields() { @@ -351,6 +385,11 @@ pub(crate) fn ensure_blob_v2_column( schema: &lance_core::datatypes::Schema, column: &str, ) -> Result<()> { + // A top-level blob cannot have a list ancestor. Keep the common read path + // from converting the entire schema just to validate that condition. + if column.contains('.') || schema.field(column).is_none_or(|field| !field.is_blob_v2()) { + reject_list_blob_column(&Schema::from(schema), column)?; + } match schema.field(column) { Some(field) if field.is_blob_v2() => Ok(()), Some(field) if field.is_blob() => Err(Error::InvalidInput { @@ -619,6 +658,54 @@ mod tests { assert_eq!(blob_column_names(&schema), vec!["info.blob"]); } + #[test] + fn list_blob_paths_are_not_row_addressable() { + let schema = Schema::new(vec![ + blob("image", true), + Field::new( + "info", + DataType::Struct(vec![blob("thumbnail", true)].into()), + true, + ), + Field::new("images", DataType::List(Arc::new(blob("item", true))), true), + Field::new( + "groups", + DataType::LargeList(Arc::new(Field::new( + "group", + DataType::Struct(vec![blob("photo", true)].into()), + true, + ))), + true, + ), + Field::new( + "frames", + DataType::FixedSizeList(Arc::new(blob("frame", true)), 2), + true, + ), + ]); + assert!(has_blob_columns(&schema)); + assert_eq!(blob_column_names(&schema), vec!["image", "info.thumbnail"]); + + for column in [ + "images", + "images.item", + "groups", + "groups.group.photo", + "frames", + "frames.frame", + ] { + let err = reject_list_blob_column(&schema, column).unwrap_err(); + assert!(matches!(err, Error::InvalidInput { .. })); + assert!( + err.to_string() + .contains("blobs inside lists cannot be fetched") + ); + } + for column in ["image", "info.thumbnail", "info"] { + reject_list_blob_column(&schema, column).unwrap(); + } + } + #[test] fn storage_version_noop_without_blob_columns() { let schema = Schema::new(vec![Field::new("id", DataType::Int64, false)]); diff --git a/rust/lancedb/src/remote/table.rs b/rust/lancedb/src/remote/table.rs index 6bbb49adf..8d655a35c 100644 --- a/rust/lancedb/src/remote/table.rs +++ b/rust/lancedb/src/remote/table.rs @@ -5047,6 +5047,14 @@ mod tests { assert_eq!(data[0].as_ref().unwrap(), &expected_data); } + fn schema_describe_response(schema: &Schema) -> http::Response { + let json_schema = JsonSchema::try_from(schema).unwrap(); + http::Response::builder() + .status(200) + .body(serde_json::json!({ "version": 1, "schema": json_schema }).to_string()) + .unwrap() + } + fn blob_describe_response() -> http::Response { let schema = Schema::new(vec![ Field::new("id", DataType::Int64, false), @@ -5054,11 +5062,7 @@ mod tests { Field::new("caption", DataType::Utf8, true), crate::blob("thumbnail", true), ]); - let json_schema = JsonSchema::try_from(&schema).unwrap(); - http::Response::builder() - .status(200) - .body(serde_json::json!({ "version": 1, "schema": json_schema }).to_string()) - .unwrap() + schema_describe_response(&schema) } #[rstest] @@ -5077,6 +5081,48 @@ mod tests { assert_eq!(columns, vec!["image".to_string(), "thumbnail".to_string()]); } + #[tokio::test] + async fn test_list_blob_paths_are_not_advertised_or_fetchable() { + let schema = Schema::new(vec![ + crate::blob("thumbnail", true), + Field::new( + "imgs", + DataType::List(Arc::new(crate::blob("item", true))), + true, + ), + ]); + let table = Table::new_with_handler_version( + "my_table", + semver::Version::new(0, 5, 0), + move |request| match request.url().path() { + "/v1/table/my_table/describe/" => schema_describe_response(&schema), + "/v1/table/my_table/fetch_blobs/" => http::Response::builder() + .status(400) + .body("Invalid input, column 'imgs' is not a blob column".to_string()) + .unwrap(), + path if path.starts_with("/v1/table/my_table/blob/") => http::Response::builder() + .status(400) + .body("Invalid input, column 'imgs' is not a blob column".to_string()) + .unwrap(), + path => panic!("unexpected request: {path}"), + }, + ); + + assert_eq!(table.blob_columns().await.unwrap(), vec!["thumbnail"]); + for column in ["imgs", "imgs.item"] { + let err = table.fetch_blobs(column, &[10]).await.unwrap_err(); + assert!( + err.to_string() + .contains("blobs inside lists cannot be fetched") + ); + let err = table.fetch_blob_files(column, &[10]).await.unwrap_err(); + assert!( + err.to_string() + .contains("blobs inside lists cannot be fetched") + ); + } + } + #[tokio::test] async fn test_fetch_blobs_decodes_null_aligned_bytes() { let mut builder = LargeBinaryBuilder::new(); @@ -5322,10 +5368,18 @@ mod tests { #[tokio::test] async fn test_fetch_blobs_sends_a_nested_column_as_a_dotted_path() { let ipc = one_row_blob_ipc_stream("info.blob"); + let schema = Schema::new(vec![Field::new( + "info", + DataType::Struct(vec![crate::blob("blob", true)].into()), + true, + )]); let table = Table::new_with_handler_version( "my_table", semver::Version::new(0, 5, 0), move |request| { + if request.url().path() == "/v1/table/my_table/describe/" { + return schema_describe_response(&schema).map(|body| body.into_bytes()); + } let body = request_body_json(&request); assert_eq!(body["column"], "info.blob"); http::Response::builder() diff --git a/rust/lancedb/src/remote/table/blobs.rs b/rust/lancedb/src/remote/table/blobs.rs index 597b41782..1e3411de1 100644 --- a/rust/lancedb/src/remote/table/blobs.rs +++ b/rust/lancedb/src/remote/table/blobs.rs @@ -348,6 +348,34 @@ fn validate_partial_response( } impl RemoteTable { + async fn reject_list_blob_path_if_known(&self, column: &str) -> Result<()> { + // A dotted path needs client-side validation because the server may + // report an internal type mismatch when the path crosses a list. Use + // an already cached schema for top-level list fields as well. + if column.contains('.') || self.schema_cache.try_get().is_some() { + let schema = self.schema().await?; + crate::blob::reject_list_blob_column(schema.as_ref(), column)?; + } + Ok(()) + } + + async fn explain_list_blob_bad_request(&self, column: &str, error: Error) -> Error { + let is_bad_request = match &error { + Error::Http { + status_code: Some(code), + .. + } => *code == StatusCode::BAD_REQUEST || *code == StatusCode::UNPROCESSABLE_ENTITY, + _ => false, + }; + if is_bad_request + && let Ok(schema) = self.schema().await + && let Err(list_error) = crate::blob::reject_list_blob_column(schema.as_ref(), column) + { + return list_error; + } + error + } + /// Blob v2 columns are marked in field metadata, which `describe` returns. Reading /// them from the cached schema needs no route of its own and no version gate. pub(super) async fn blob_columns_impl(&self) -> Result> { @@ -360,6 +388,7 @@ impl RemoteTable { column: &str, row_ids: &[u64], ) -> Result { + self.reject_list_blob_path_if_known(column).await?; // Empty requests do not require blob-route support. if row_ids.is_empty() { return Ok(LargeBinaryArray::from(Vec::>::new())); @@ -384,7 +413,10 @@ impl RemoteTable { let (request_id, response) = self .send_with_freshness(request, true, read_snapshot.freshness) .await?; - let mut stream = self.read_arrow_response(&request_id, response).await?; + let mut stream = match self.read_arrow_response(&request_id, response).await { + Ok(stream) => stream, + Err(error) => return Err(self.explain_list_blob_bad_request(column, error).await), + }; let mut blob_chunks: Vec> = Vec::new(); while let Some(batch) = stream.try_next().await? { @@ -449,6 +481,7 @@ impl RemoteTable { column: &str, row_ids: &[u64], ) -> Result>> { + self.reject_list_blob_path_if_known(column).await?; // Empty requests do not require blob-route support. if row_ids.is_empty() { return Ok(Vec::new()); @@ -481,7 +514,10 @@ impl RemoteTable { requester }) .collect(); - probe_blob_files(requesters).await + match probe_blob_files(requesters).await { + Ok(files) => Ok(files), + Err(error) => Err(self.explain_list_blob_bad_request(column, error).await), + } } } diff --git a/rust/lancedb/src/table.rs b/rust/lancedb/src/table.rs index fbec73dfb..31a9937f6 100644 --- a/rust/lancedb/src/table.rs +++ b/rust/lancedb/src/table.rs @@ -1215,10 +1215,11 @@ impl Table { self.inner.count_rows(filter).await } - /// Names of the blob v2 columns in this table, in declaration order. + /// Names of the row-addressable blob v2 columns, in declaration order. /// - /// Nested blobs use dotted paths (e.g. `info.blob`). Returns - /// [`Error::NotSupported`] on table types without blob support. + /// Blobs nested in structs use dotted paths (e.g. `info.blob`). Blobs + /// inside lists are omitted because row ids cannot identify their elements. + /// Returns [`Error::NotSupported`] on table types without blob support. pub async fn blob_columns(&self) -> Result> { self.inner.blob_columns().await } @@ -1254,9 +1255,10 @@ impl Table { /// # } /// ``` /// - /// Returns [`Error::InvalidInput`] when the column does not exist or is - /// not a blob v2 column, and [`Error::NotSupported`] on table types - /// without blob support. + /// Blobs inside lists cannot be fetched by row id. Returns + /// [`Error::InvalidInput`] for those paths, when the column does not exist, + /// or when it is not a blob v2 column. Returns [`Error::NotSupported`] on + /// table types without blob support. pub async fn fetch_blobs( &self, column: impl AsRef, diff --git a/rust/lancedb/tests/blob_integration.rs b/rust/lancedb/tests/blob_integration.rs index dd4051657..79782fca1 100644 --- a/rust/lancedb/tests/blob_integration.rs +++ b/rust/lancedb/tests/blob_integration.rs @@ -611,6 +611,47 @@ async fn blob_columns_lists_nested_dotted_paths() -> Result<()> { Ok(()) } +#[tokio::test] +async fn blob_columns_excludes_list_elements_and_fetches_explain_limitation() -> Result<()> { + let tmp = tempdir().unwrap(); + let db = connect(tmp.path().to_str().unwrap()).execute().await?; + let schema = Arc::new(Schema::new(vec![ + Field::new("id", DataType::Int64, false), + blob("thumbnail", true), + Field::new("imgs", DataType::List(Arc::new(blob("item", true))), true), + ])); + let table = db + .create_empty_table("list_blobs", schema) + .execute() + .await?; + + assert_eq!(table.blob_columns().await?, vec!["thumbnail"]); + for column in ["imgs", "imgs.item"] { + let err = table.fetch_blobs(column, &[0]).await.unwrap_err(); + assert!(matches!(err, Error::InvalidInput { .. })); + assert!( + err.to_string() + .contains("blobs inside lists cannot be fetched") + ); + + let err = table.fetch_blob_files(column, &[0]).await.unwrap_err(); + assert!( + err.to_string() + .contains("blobs inside lists cannot be fetched") + ); + + let err = table + .fetch_blob_ranges(column, [BlobRangeRequest::new(0, 0, 1)]) + .await + .unwrap_err(); + assert!( + err.to_string() + .contains("blobs inside lists cannot be fetched") + ); + } + Ok(()) +} + #[tokio::test] async fn blob_columns_lists_blob_fields_in_order() -> Result<()> { let tmp = tempdir().unwrap();