mirror of
https://github.com/lancedb/lancedb.git
synced 2026-09-22 13:05:48 +00:00
fix: accept all-null batches and plain JSON strings for json columns (#4067)
Two ways of writing to a `json` column failed or silently corrupted data. **All-null batches were rejected.** `add()` refused a batch whose values for a `json` column were all null, while every plain Arrow type accepted the same batch. This bites row-at-a-time inserts hardest: a one-row batch with no value for an optional column is trivially all-null, so most such writes failed. pyarrow infers `null` as the column's type, and the write path had no handling for it — casting to the table's type dropped the field metadata that identifies the column as `lance.json`, so lance rejected the batch (`` `val` should have type json but type was large_binary ``). A null-typed input column now becomes typed nulls matching the table's field exactly, metadata included. **Unlabelled JSON text was stored raw.** JSON supplied as plain strings (what pyarrow infers for a column of `str`) was cast to the column's `LargeBinary` storage type and relabelled `lance.json`, putting unparsed text where JSONB was expected. Reads returned the text unnormalized and `json_extract` failed with `InvalidJsonb`. Lance-core does the JSONB encoding, but only for input labelled `arrow.json`, so string input is now labelled rather than cast — at the top level and inside structs. Both fixes are in the shared Rust write path, so they apply to any binding, including hand-built Arrow tables that never pass through Python's list-of-dicts type inference. `_align_field` gets the same JSON-string fix for the legacy Python `_sanitize_data` path, which `on_bad_vectors` and embedding functions still route through. The blob v2 half of the issue landed separately in #4065, which added a `DataType::Null` arm to blob coercion. This PR keeps that implementation and adds end-to-end add-path coverage for it. The tests from #4066 are included here and pass, so that PR's Python-layer inference changes are no longer needed to close the issue. Fixes #3759 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
95055c4c54
commit
1d2a5d084b
@@ -634,26 +634,43 @@ def _align_field_types(
|
||||
return new_fields
|
||||
|
||||
|
||||
def _align_list_value_field(
|
||||
value_field: pa.Field, target_value_field: pa.Field
|
||||
) -> pa.Field:
|
||||
# A list has exactly one child, so the inferred child name ("item") aligns
|
||||
# positionally and adopts the table's child name; pa.Table.cast renames it.
|
||||
return _align_field(value_field, target_value_field).with_name(
|
||||
target_value_field.name
|
||||
)
|
||||
def _align_container_child(child: pa.Field, target_child: pa.Field) -> pa.Field:
|
||||
# A list has one child, a map one key and one item, so an inferred child name
|
||||
# ("item") aligns positionally and adopts the table's; pa.Table.cast renames it.
|
||||
return _align_field(child, target_child).with_name(target_child.name)
|
||||
|
||||
|
||||
def _arrow_json_storage_type(input_type: pa.DataType) -> Optional[pa.DataType]:
|
||||
"""The storage type arrow.json would use for ``input_type``.
|
||||
|
||||
Returns None if the type cannot hold JSON text.
|
||||
"""
|
||||
if pa.types.is_string(input_type) or pa.types.is_string_view(input_type):
|
||||
return pa.string()
|
||||
if pa.types.is_large_string(input_type):
|
||||
return pa.large_string()
|
||||
return None
|
||||
|
||||
|
||||
def _align_field(field: pa.Field, target_field: pa.Field) -> pa.Field:
|
||||
# Preserve arrow.json input until it reaches Lance. LanceDB exposes stored
|
||||
# JSON columns as lance.json (JSONB-backed LargeBinary), but casting the
|
||||
# input to that storage type here merely relabels the raw JSON bytes as
|
||||
# LanceDB exposes stored JSON columns as lance.json (JSONB-backed LargeBinary), but
|
||||
# casting the input to that storage type here merely relabels the raw JSON bytes as
|
||||
# JSONB. Lance must see arrow.json so it can perform the JSONB encoding.
|
||||
if (
|
||||
_field_extension_name(field) == "arrow.json"
|
||||
and _field_extension_name(target_field) == "lance.json"
|
||||
):
|
||||
return field
|
||||
if _field_extension_name(target_field) == "lance.json":
|
||||
if _field_extension_name(field) == "arrow.json":
|
||||
return field
|
||||
# Plain JSON text, which is what pyarrow infers for a column of `str`, only
|
||||
# needs the arrow.json label.
|
||||
json_storage = _arrow_json_storage_type(field.type)
|
||||
if json_storage is not None:
|
||||
# Labelled through metadata rather than pa.json_(), which only exists on
|
||||
# newer PyArrow; Lance reads the extension name off the field either way.
|
||||
return pa.field(
|
||||
field.name,
|
||||
json_storage,
|
||||
field.nullable,
|
||||
{"ARROW:extension:name": "arrow.json"},
|
||||
)
|
||||
if pa.types.is_struct(target_field.type):
|
||||
if pa.types.is_struct(field.type):
|
||||
new_type = pa.struct(
|
||||
@@ -667,7 +684,7 @@ def _align_field(field: pa.Field, target_field: pa.Field) -> pa.Field:
|
||||
elif pa.types.is_list(target_field.type):
|
||||
if _is_list_like(field.type):
|
||||
new_type = pa.list_(
|
||||
_align_list_value_field(
|
||||
_align_container_child(
|
||||
field.type.value_field, target_field.type.value_field
|
||||
)
|
||||
)
|
||||
@@ -676,7 +693,7 @@ def _align_field(field: pa.Field, target_field: pa.Field) -> pa.Field:
|
||||
elif pa.types.is_large_list(target_field.type):
|
||||
if _is_list_like(field.type):
|
||||
new_type = pa.large_list(
|
||||
_align_list_value_field(
|
||||
_align_container_child(
|
||||
field.type.value_field, target_field.type.value_field
|
||||
)
|
||||
)
|
||||
@@ -685,13 +702,28 @@ def _align_field(field: pa.Field, target_field: pa.Field) -> pa.Field:
|
||||
elif pa.types.is_fixed_size_list(target_field.type):
|
||||
if _is_list_like(field.type):
|
||||
new_type = pa.list_(
|
||||
_align_list_value_field(
|
||||
_align_container_child(
|
||||
field.type.value_field, target_field.type.value_field
|
||||
),
|
||||
target_field.type.list_size,
|
||||
)
|
||||
else:
|
||||
new_type = target_field.type
|
||||
elif pa.types.is_map(target_field.type):
|
||||
if pa.types.is_map(field.type):
|
||||
# A map has exactly one key and one item field, so like a list's child they
|
||||
# align positionally and adopt the table's names.
|
||||
new_type = pa.map_(
|
||||
_align_container_child(
|
||||
field.type.key_field, target_field.type.key_field
|
||||
),
|
||||
_align_container_child(
|
||||
field.type.item_field, target_field.type.item_field
|
||||
),
|
||||
keys_sorted=target_field.type.keys_sorted,
|
||||
)
|
||||
else:
|
||||
new_type = target_field.type
|
||||
else:
|
||||
new_type = target_field.type
|
||||
return pa.field(field.name, new_type, field.nullable, target_field.metadata)
|
||||
|
||||
@@ -751,6 +751,80 @@ def test_fetch_blobs_preserves_null_and_empty_values():
|
||||
assert blobs[3].as_py() == b"present"
|
||||
|
||||
|
||||
def test_add_all_null_list_to_blob_column():
|
||||
table = _blob_table("all_null_add", [{"id": 1, "image": None}])
|
||||
|
||||
hits = table.search().to_arrow()
|
||||
blobs = table.fetch_blobs("image", hits)
|
||||
assert len(blobs) == 1
|
||||
assert blobs[0].as_py() is None
|
||||
|
||||
|
||||
def test_add_all_null_list_to_blob_column_with_sanitizer():
|
||||
db = lancedb.connect("memory:///")
|
||||
schema = pa.schema([pa.field("id", pa.int64()), lancedb.blob("image")])
|
||||
table = db.create_table("all_null_sanitized_add", schema=schema)
|
||||
|
||||
table.add([{"id": 1, "image": None}], on_bad_vectors="fill")
|
||||
|
||||
hits = table.search().to_arrow()
|
||||
blobs = table.fetch_blobs("image", hits)
|
||||
assert len(blobs) == 1
|
||||
assert blobs[0].as_py() is None
|
||||
|
||||
|
||||
def test_add_all_null_list_to_nested_blob_column():
|
||||
db = lancedb.connect("memory:///")
|
||||
blob_field = lancedb.blob("image")
|
||||
info_field = pa.field("info", pa.struct([blob_field]))
|
||||
info = pa.StructArray.from_arrays(
|
||||
[_blob_array("image", [b"seed"])], fields=[blob_field]
|
||||
)
|
||||
seed = pa.Table.from_arrays(
|
||||
[pa.array([0], type=pa.int64()), info],
|
||||
schema=pa.schema([pa.field("id", pa.int64()), info_field]),
|
||||
)
|
||||
table = db.create_table("nested_null_add", data=seed)
|
||||
|
||||
table.add([{"id": 1, "info": {"image": None}}])
|
||||
table.add([{"id": 2, "info": {"image": None}}], on_bad_vectors="fill")
|
||||
|
||||
hits = table.search().where("id > 0").to_arrow()
|
||||
blobs = table.fetch_blobs("info.image", hits)
|
||||
assert len(blobs) == 2
|
||||
assert all(blob.as_py() is None for blob in blobs)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("large_list", [False, True], ids=["list", "large_list"])
|
||||
def test_add_list_of_dicts_to_blob_list_column(large_list):
|
||||
db = lancedb.connect("memory:///")
|
||||
blob_field = lancedb.blob("image")
|
||||
blob_values = _blob_array("image", [b"seed"])
|
||||
if large_list:
|
||||
items_field = pa.field("items", pa.large_list(blob_field))
|
||||
items = pa.LargeListArray.from_arrays(
|
||||
pa.array([0, 1], type=pa.int64()), blob_values
|
||||
)
|
||||
else:
|
||||
items_field = pa.field("items", pa.list_(blob_field))
|
||||
items = pa.ListArray.from_arrays(pa.array([0, 1], type=pa.int32()), blob_values)
|
||||
seed = pa.Table.from_arrays(
|
||||
[pa.array([0], type=pa.int64()), items],
|
||||
schema=pa.schema([pa.field("id", pa.int64()), items_field]),
|
||||
)
|
||||
table = db.create_table(f"blob_{large_list}_list_add", data=seed)
|
||||
|
||||
table.add([{"id": 1, "items": [None]}])
|
||||
table.add(
|
||||
[{"id": 2, "items": [b"a", None]}],
|
||||
on_bad_vectors="fill",
|
||||
)
|
||||
|
||||
ids = table.search().select(["id"]).to_arrow()["id"].to_pylist()
|
||||
assert sorted(ids) == [0, 1, 2]
|
||||
assert pa.types.is_large_list(table.schema.field("items").type) is large_list
|
||||
|
||||
|
||||
def test_fetch_blob_ranges_aligns_repeated_ranges_and_nulls():
|
||||
table = _blob_table(
|
||||
"range_alignment",
|
||||
|
||||
@@ -2979,28 +2979,29 @@ async def test_merge_insert_async(mem_db_async: AsyncConnection):
|
||||
assert (await table.to_arrow()).sort_by("a") == expected
|
||||
|
||||
|
||||
def _json_arrow_table(schema, rows):
|
||||
json_type = schema.field("j").type
|
||||
json_values = pa.ExtensionArray.from_storage(
|
||||
json_type,
|
||||
pa.array([value for _, value in rows], type=json_type.storage_type),
|
||||
)
|
||||
return pa.Table.from_arrays(
|
||||
[pa.array([row_id for row_id, _ in rows]), json_values], schema=schema
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.skipif(not hasattr(pa, "json_"), reason="requires PyArrow JSON type")
|
||||
@pytest.mark.asyncio
|
||||
async def test_merge_insert_encodes_json(mem_db_async: AsyncConnection):
|
||||
json_type = pa.json_()
|
||||
schema = pa.schema([pa.field("id", pa.string()), pa.field("j", json_type)])
|
||||
|
||||
def json_table(rows):
|
||||
json_values = pa.ExtensionArray.from_storage(
|
||||
json_type,
|
||||
pa.array([value for _, value in rows], type=json_type.storage_type),
|
||||
)
|
||||
return pa.Table.from_arrays(
|
||||
[pa.array([row_id for row_id, _ in rows]), json_values], schema=schema
|
||||
)
|
||||
schema = pa.schema([pa.field("id", pa.string()), pa.field("j", pa.json_())])
|
||||
|
||||
table = await mem_db_async.create_table("json_merge", schema=schema)
|
||||
await table.add(json_table([("a", '{"k": 1}'), ("b", '{"k": 9}')]))
|
||||
await table.add(_json_arrow_table(schema, [("a", '{"k": 1}'), ("b", '{"k": 9}')]))
|
||||
|
||||
await (
|
||||
table.merge_insert("id")
|
||||
.when_matched_update_all()
|
||||
.execute(json_table([("a", '{"k": 2}')]))
|
||||
.execute(_json_arrow_table(schema, [("a", '{"k": 2}')]))
|
||||
)
|
||||
|
||||
rows = sorted(await table.query().to_list(), key=lambda row: row["id"])
|
||||
@@ -3015,20 +3016,176 @@ async def test_merge_insert_encodes_json(mem_db_async: AsyncConnection):
|
||||
@pytest.mark.skipif(not hasattr(pa, "json_"), reason="requires PyArrow JSON type")
|
||||
@pytest.mark.asyncio
|
||||
async def test_add_sanitization_encodes_json(mem_db_async: AsyncConnection):
|
||||
json_type = pa.json_()
|
||||
schema = pa.schema([pa.field("id", pa.string()), pa.field("j", json_type)])
|
||||
json_values = pa.ExtensionArray.from_storage(
|
||||
json_type, pa.array(['{"k": 3}'], type=json_type.storage_type)
|
||||
)
|
||||
data = pa.Table.from_arrays([pa.array(["c"]), json_values], schema=schema)
|
||||
schema = pa.schema([pa.field("id", pa.string()), pa.field("j", pa.json_())])
|
||||
|
||||
table = await mem_db_async.create_table("json_add", schema=schema)
|
||||
await table.add(data, on_bad_vectors="fill")
|
||||
await table.add(
|
||||
_json_arrow_table(schema, [("c", '{"k": 3}')]), on_bad_vectors="fill"
|
||||
)
|
||||
|
||||
rows = await table.query().where("json_extract(j, '$.k') = '3'").to_list()
|
||||
assert rows == [{"id": "c", "j": '{"k":3}'}]
|
||||
|
||||
|
||||
@pytest.mark.skipif(not hasattr(pa, "json_"), reason="requires PyArrow JSON type")
|
||||
@pytest.mark.asyncio
|
||||
async def test_add_all_null_json_batch(mem_db_async: AsyncConnection):
|
||||
"""A batch of dicts whose json values are all None infers as pa.null(), which used
|
||||
to fail with a `json` vs `large_binary` schema mismatch. A row-at-a-time insert of
|
||||
an optional json column always looks like this."""
|
||||
schema = pa.schema([pa.field("id", pa.string()), pa.field("j", pa.json_())])
|
||||
table = await mem_db_async.create_table("json_nulls", schema=schema)
|
||||
|
||||
await table.add([{"id": "a", "j": None}])
|
||||
assert await table.count_rows() == 1
|
||||
|
||||
# ... and again once real JSON has been written.
|
||||
await table.add(_json_arrow_table(schema, [("b", '{"k": 9}')]))
|
||||
await table.add([{"id": "c", "j": None}])
|
||||
|
||||
rows = sorted(await table.query().to_list(), key=lambda row: row["id"])
|
||||
assert rows == [
|
||||
{"id": "a", "j": None},
|
||||
{"id": "b", "j": '{"k":9}'},
|
||||
{"id": "c", "j": None},
|
||||
]
|
||||
|
||||
# The nulls must not disturb reads of the column.
|
||||
filtered = await table.query().where("json_extract(j, '$.k') = '9'").to_list()
|
||||
assert filtered == [{"id": "b", "j": '{"k":9}'}]
|
||||
assert len(await table.query().where("j IS NULL").to_list()) == 2
|
||||
|
||||
|
||||
@pytest.mark.skipif(not hasattr(pa, "json_"), reason="requires PyArrow JSON type")
|
||||
def test_add_all_null_json_batch_sync(mem_db: DBConnection):
|
||||
schema = pa.schema([pa.field("id", pa.string()), pa.field("j", pa.json_())])
|
||||
table = mem_db.create_table("json_nulls_sync", schema=schema)
|
||||
|
||||
table.add([{"id": "a", "j": None}])
|
||||
|
||||
assert table.count_rows() == 1
|
||||
assert table.to_arrow()["j"].to_pylist() == [None]
|
||||
|
||||
|
||||
@pytest.mark.skipif(not hasattr(pa, "json_"), reason="requires PyArrow JSON type")
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
("values", "expected"),
|
||||
[
|
||||
([None], [None]),
|
||||
([None, '{"k": 1}'], [None, '{"k":1}']),
|
||||
(['{"k": 2}'], ['{"k":2}']),
|
||||
],
|
||||
)
|
||||
async def test_add_list_of_dicts_to_json_column(
|
||||
mem_db_async: AsyncConnection, values, expected
|
||||
):
|
||||
schema = pa.schema([pa.field("id", pa.int64()), pa.field("value", pa.json_())])
|
||||
table = await mem_db_async.create_table("json_list_add", schema=schema)
|
||||
|
||||
await table.add([{"id": idx, "value": value} for idx, value in enumerate(values)])
|
||||
|
||||
rows = (await table.to_arrow()).sort_by("id").to_pylist()
|
||||
assert [row["value"] for row in rows] == expected
|
||||
|
||||
|
||||
@pytest.mark.skipif(not hasattr(pa, "json_"), reason="requires PyArrow JSON type")
|
||||
@pytest.mark.asyncio
|
||||
async def test_add_list_of_dicts_to_nested_json_column(
|
||||
mem_db_async: AsyncConnection,
|
||||
):
|
||||
json_field = pa.field("value", pa.json_())
|
||||
info_field = pa.field("info", pa.struct([json_field]))
|
||||
info = pa.StructArray.from_arrays(
|
||||
[pa.array(['{"seed": 0}'], type=pa.json_())], fields=[json_field]
|
||||
)
|
||||
seed = pa.Table.from_arrays(
|
||||
[pa.array([0], type=pa.int64()), info],
|
||||
schema=pa.schema([pa.field("id", pa.int64()), info_field]),
|
||||
)
|
||||
table = await mem_db_async.create_table("nested_json_list_add", data=seed)
|
||||
|
||||
await table.add([{"id": 1, "info": {"value": '{"k": 1}'}}])
|
||||
await table.add([{"id": 2, "info": {"value": '{"k": 2}'}}], on_bad_vectors="fill")
|
||||
|
||||
rows = (await table.to_arrow()).sort_by("id").to_pylist()
|
||||
assert rows == [
|
||||
{"id": 0, "info": {"value": '{"seed":0}'}},
|
||||
{"id": 1, "info": {"value": '{"k":1}'}},
|
||||
{"id": 2, "info": {"value": '{"k":2}'}},
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.skipif(not hasattr(pa, "json_"), reason="requires PyArrow JSON type")
|
||||
@pytest.mark.asyncio
|
||||
async def test_add_list_of_dicts_to_json_list_column(mem_db_async: AsyncConnection):
|
||||
"""JSON inside a list must be JSONB-encoded, not stored as the raw text.
|
||||
|
||||
Storing raw text appends without error but leaves the column unreadable, so the
|
||||
round trip is checked with a filter as well as by value.
|
||||
"""
|
||||
schema = pa.schema(
|
||||
[
|
||||
pa.field("id", pa.int64()),
|
||||
pa.field("docs", pa.list_(pa.field("item", pa.json_()))),
|
||||
]
|
||||
)
|
||||
table = await mem_db_async.create_table("json_list_add", schema=schema)
|
||||
|
||||
await table.add([{"id": 1, "docs": ['{"k": 1}', '{"k": 2}']}])
|
||||
await table.add([{"id": 2, "docs": ['{"k": 3}']}], on_bad_vectors="fill")
|
||||
|
||||
rows = (await table.to_arrow()).sort_by("id").to_pylist()
|
||||
assert rows == [
|
||||
{"id": 1, "docs": ['{"k":1}', '{"k":2}']},
|
||||
{"id": 2, "docs": ['{"k":3}']},
|
||||
]
|
||||
|
||||
matched = await table.query().where("json_extract(docs[1], '$.k') = 3").to_arrow()
|
||||
assert matched.column("id").to_pylist() == [2]
|
||||
|
||||
|
||||
@pytest.mark.skipif(not hasattr(pa, "json_"), reason="requires PyArrow JSON type")
|
||||
@pytest.mark.asyncio
|
||||
async def test_add_map_of_json_values(mem_db_async: AsyncConnection):
|
||||
"""JSON in a map's values needs the same encoding a list's items do."""
|
||||
schema = pa.schema(
|
||||
[
|
||||
pa.field("id", pa.int64()),
|
||||
pa.field("m", pa.map_(pa.string(), pa.json_())),
|
||||
]
|
||||
)
|
||||
table = await mem_db_async.create_table(
|
||||
"json_map_add",
|
||||
schema=schema,
|
||||
storage_options={"new_table_data_storage_version": "2.2"},
|
||||
)
|
||||
|
||||
def batch(row_id: int, text: str) -> pa.Table:
|
||||
return pa.table(
|
||||
{
|
||||
"id": pa.array([row_id], type=pa.int64()),
|
||||
"m": pa.array([[("k", text)]], type=pa.map_(pa.string(), pa.string())),
|
||||
}
|
||||
)
|
||||
|
||||
await table.add(batch(1, '{"x": 1}'))
|
||||
await table.add(batch(2, '{"x": 2}'), on_bad_vectors="fill")
|
||||
|
||||
rows = (await table.to_arrow()).sort_by("id").to_pylist()
|
||||
assert rows == [
|
||||
{"id": 1, "m": [("k", '{"x":1}')]},
|
||||
{"id": 2, "m": [("k", '{"x":2}')]},
|
||||
]
|
||||
|
||||
matched = (
|
||||
await table.query()
|
||||
.where("json_extract(element_at(m, 'k')[1], '$.x') = '2'")
|
||||
.to_arrow()
|
||||
)
|
||||
assert matched.column("id").to_pylist() == [2]
|
||||
|
||||
|
||||
def test_create_with_embedding_function(mem_db: DBConnection):
|
||||
class MyTable(LanceModel):
|
||||
text: str
|
||||
|
||||
Reference in New Issue
Block a user