Compare commits

...

5 Commits

Author SHA1 Message Date
Gatefixer 238028767a docs(node): clarify full-text search filtering 2026-08-05 22:27:18 +00:00
lancedb-gatefixer[bot] 7357d63e87 fix(python): guard concurrent table deletes (#3787)
<!-- lance-gatekeeper-fix:v1 agent=5c80c44c083b3b8ad0da595419d468fc
generation=1 -->

## Root cause

The legacy synchronous Python table called `delete` on a shared, mutable
`lance.Dataset`. Concurrent table operations could hold a PyO3 borrow
while delete requested an exclusive borrow, producing `RuntimeError:
Already borrowed`. The current async-backed binding fixes this by
cloning its thread-safe Rust table handle before awaiting, but that
concurrency contract had no regression coverage.

## Fix

- Document why delete must clone the Rust table handle before entering
its async future.
- Add a barrier-synchronized regression test that deletes distinct rows
through one shared table from eight Python threads.
- Verify every delete commits exactly one row, every commit gets a
distinct version, and no rows remain.

## Validation

- `cargo check --quiet --features remote --tests --examples`
- `cargo fmt --all -- --check`
- `uv run --extra tests --extra dev ruff format --check
python/tests/test_table.py`
- `uv run --extra tests --extra dev ruff check
python/tests/test_table.py`
- `uv run --extra tests --extra dev pytest
python/tests/test_table.py::test_concurrent_deletes_are_thread_safe
python/tests/test_table.py::test_delete
python/tests/test_table.py::test_delete_expr
python/tests/test_table.py::test_delete_expr_async -q` (4 passed)
- Manual stress reproduction: 100 concurrent deletes on one table
completed at versions 2–101 with zero rows remaining.

Fixes #530

Co-authored-by: Gatefixer <313497061+lancedb-gatefixer[bot]@users.noreply.github.com>
2026-08-05 15:17:04 -07:00
lancedb-gatefixer[bot] 624a75edf7 fix(python): avoid debugger deadlock during connection inspection (#3788)
## Summary

- cache the immutable read consistency interval on synchronous
connection wrappers
- keep debugger property expansion from dispatching to the background
event loop
- cover direct connections and wrappers reconstructed from native
connections

## Root cause

The debugger expands connection variables by evaluating properties after
suspending all Python threads.
`LanceDBConnection.read_consistency_interval` dispatched a coroutine to
`LanceDBBackgroundEventLoop` and synchronously waited for it, but that
loop thread was also suspended, causing a deadlock.

## Validation

- `uv run --no-sync pytest python/tests/test_db.py -q` (48 passed)
- `ruff format --check python/python/lancedb/db.py
python/python/tests/test_db.py`
- `ruff check .`
- `git diff --check`

Fixes #3773

<!-- lance-gatekeeper-fix:v1 agent=e2e612236d722d926f64245d3f682bbc
generation=1 -->

---------

Co-authored-by: Gatefixer <313497061+lancedb-gatefixer[bot]@users.noreply.github.com>
2026-08-05 15:15:49 -07:00
Justin Miller c7ea91f3ea test: cover blob null/empty preservation across Table::optimize (#3774)
## Description

`Table::optimize()` compacts through
`lance::dataset::optimize::compact_files`
(`rust/lancedb/src/table/optimize.rs:155`). Until
lance-format/lance#7965 that rewrite corrupted blob columns holding null
or empty values, which is what #3744 reports:

- **storage 2.0** (legacy v1 `lance-encoding:blob` descriptors): every
payload following a null or empty row in the same fragment was rewritten
as `{position: 0, size: 0}`, so it read back as `b""` and the new
fragment no longer referenced the bytes — silent payload loss,
unrecoverable once the pre-optimize versions are pruned.
- **storage 2.2** (blob v2): a valid empty value was rewritten as null,
destroying the null-vs-empty distinction.

Both manifestations share one root cause: `is_inline_null_blob`
classified any inline blob with `position == 0 && size == 0` as null,
which is also exactly what a *valid empty value* looks like. Such rows
were dropped from `blob_read_addrs`, misaligning every payload that
followed.

The behaviour is already correct on `main`: the vendored lance crate
first carried the fix at `v10.0.0-beta.3` (#3710) and is now
`v10.1.0-beta.1` (#3757). What was missing is coverage — nothing in this
repo exercised a blob column containing a null or empty value through
`optimize()`, which is why this shipped unnoticed. This PR adds that
guard.

## Tests

Two tests in `rust/lancedb/tests/blob_integration.rs`, reusing the
file's existing 64 KiB dedicated-blob helpers and a delete-triggered
fragment rewrite. After `id IN (1, 4)` is deleted the surviving rows are
`2` (null), `3` (valid empty), `5` and `6` (payloads) — payloads sit
immediately after the null/empty, which is where the misalignment
landed.

- `optimize_preserves_v1_blob_payloads_with_null_and_empty` — storage
2.0; asserts the **payload bytes** are unchanged across
`OptimizeAction::All` (what the Python/Node `optimize()` bindings
invoke). Payloads are read through `lance::Dataset::take_blobs`, since
`Table::fetch_blobs` rejects legacy v1 columns. The before/after
descriptors are reported on failure but deliberately *not* asserted:
compaction repacks the blob file, so they shift legitimately (id 5
`(131072, 65536)` → `(0, 65536)`, id 6 `(196608, 65536)` → `(65536,
65536)`). Note that a post-compaction `position: 0` is both the
legitimate first-payload offset and the bug's signature, so asserting
descriptors would be actively misleading.
- `optimize_preserves_blob_v2_null_and_empty_distinction` — storage >=
2.2; asserts a null stays null and a valid empty value stays non-null
empty.

Both assert the pre-optimize state first, so a setup change that stops
producing the null/empty/payload mix fails loudly instead of passing
vacuously.

Both also assert the returned `CompactionMetrics` show a fragment was
actually rewritten. These tests depend on `delete("id IN (1, 4)")`
pushing the fragment past lance's `materialize_deletions_threshold` (0.1
by default; 2 of 6 rows here). That coupling is invisible and unasserted
otherwise: against a forced no-op (`materialize_deletions_threshold:
1.5`) the metrics come back all zeroes and *every payload assertion
still passes*. Since the whole point of these tests is to survive
dependency changes, they check that the rewrite happened rather than
trusting the planner to keep selecting the fragment.

Guard verified against a pre-fix lance: with the published
`lancedb==0.36.0` wheel (vendors lance 9.0.0), `Table.optimize()` on the
same data rewrites the descriptors of the two rows following the
null/empty from `(131072, 65536)` and `(196608, 65536)` to `(0, 0)`, and
the payloads read back empty. Against the pinned `v10.1.0-beta.1`, all
39 tests in the file pass, adding roughly 10–20 ms to the file's
runtime.

## Not addressed here

- **No released artifact has the fix yet.** PyPI `lancedb` 0.36.0
(2026-07-29) vendors lance 9.0.0; npm `@lancedb/lancedb` 0.37.1-beta.0
predates the bump. No 9.x lance tag carries the fix: `v10.0.0-beta.3` is
the first tag containing it, every `v9.1.0-beta.1`…`beta.8` is behind
it, and `v9.0.0` / `v9.0.1-rc.1` sit on a diverged branch without it. A
stable lancedb release needs a stable lance >= 10.
- **The version skew #3744 flagged is still live.**
`python/pyproject.toml` pins `pylance==9.0.0rc1` for the `tests` extra
against a vendored `10.1.0-beta.1`, so Python CI still cannot observe
this class of divergence.
- **Only the single-fragment rewrite shape is covered.** Both tests
rewrite one fragment by materializing deletions. lance's own
`test_compact_blob_v1/v2_preserves_null_empty_and_payload_order` cover
the multi-fragment merge shape (3 fragments → 1) at unit level, so this
PR is complementary rather than redundant — it covers the binding-level
path through `Table::optimize` — but it would not catch a regression
that only appears when *merging* fragments.
`multi_fragment_dedicated_blob_table` in the same file makes that a
cheap follow-up.

Closes #3744

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-08-04 12:23:03 -07:00
Wyatt Alt 8e24dd3828 feat(rust)!: make add_columns a builder (#3778)
Table::add_columns now takes no arguments and returns AddColumnsBuilder,
so calls become .add_columns().transform(t).execute().

read_columns was the second positional argument but reaches only one of
the five transform variants. In lance's add_columns_to_fragments only
BatchUDF receives the caller's value: SqlExpressions replaces it with
the columns its expressions reference, Stream and Reader pass None, and
AllNulls reads nothing. So it was mandatory on every call -- all
eighteen call sites here passed None -- and silently discarded four
times out of five. As a builder method it is optional, and setting it
where lance would discard it is now an error, which does reject a call
that previously succeeded while ignoring the argument.

Matches the builders add, update, and merge_insert already use.
2026-08-04 11:18:22 -07:00
19 changed files with 677 additions and 95 deletions
Generated
+8 -8
View File
@@ -7583,7 +7583,7 @@ version = "0.14.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "343d3bd7056eda839b03204e68deff7d1b13aba7af2b2fd16890697274262ee7"
dependencies = [
"heck 0.5.0",
"heck 0.4.1",
"itertools 0.14.0",
"log",
"multimap",
@@ -8498,9 +8498,9 @@ dependencies = [
[[package]]
name = "rkyv"
version = "0.8.16"
version = "0.8.17"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "73389e0c99e664f919275ab5b5b0471391fe9a8de61e1dff9b1eaf56a90f16e3"
checksum = "815cc8a37159a463064825246cadb07961e25cd9885908606f6d08a98d8f8874"
dependencies = [
"bytecheck",
"bytes",
@@ -8517,9 +8517,9 @@ dependencies = [
[[package]]
name = "rkyv_derive"
version = "0.8.16"
version = "0.8.17"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5d2ed0b54125315fb36bd021e82d314d1c126548f871634b483f46b31d13cac6"
checksum = "c0ed1a78a1b19d184b0daa629dd9a024573173ec7d485b287cb369fb3607cc1c"
dependencies = [
"proc-macro2",
"quote",
@@ -9277,7 +9277,7 @@ version = "0.8.9"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c1c97747dbf44bb1ca44a561ece23508e99cb592e862f22222dcf42f51d1e451"
dependencies = [
"heck 0.5.0",
"heck 0.4.1",
"proc-macro2",
"quote",
"syn 2.0.117",
@@ -9289,7 +9289,7 @@ version = "0.9.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "54254b8531cafa275c5e096f62d48c81435d1015405a91198ddb11e967301d40"
dependencies = [
"heck 0.5.0",
"heck 0.4.1",
"proc-macro2",
"quote",
"syn 2.0.117",
@@ -9730,7 +9730,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd"
dependencies = [
"fastrand",
"getrandom 0.4.2",
"getrandom 0.3.4",
"once_cell",
"rustix",
"windows-sys 0.61.2",
+22 -10
View File
@@ -541,7 +541,17 @@ where(predicate): this
A filter statement to be applied to this query.
The filter should be supplied as an SQL query string. For example:
Filters are applied before full-text and vector searches by default; no
separate prefilter call is needed. For vector searches only, use
[VectorQuery#postfilter](VectorQuery.md#postfilter) to apply the filter after the search.
The filter should be supplied as an SQL query string.
Filtering performance can often be improved by creating a scalar index
on the filter column(s).
Calling this multiple times combines the filters with a logical AND rather
than replacing the previous filter.
#### Parameters
@@ -551,18 +561,20 @@ The filter should be supplied as an SQL query string. For example:
`this`
#### Example
#### Examples
```ts
x > 10
y > 0 AND y < 100
x > 5 OR y = 'test'
const results = await table
.search("puppy", "fts")
.where("meta = 'foo'")
.limit(10)
.toArray();
```
Filtering performance can often be improved by creating a scalar index
on the filter column(s).
Calling this multiple times combines the filters with a logical AND rather
than replacing the previous filter.
```ts
query.where("x > 10");
query.where("y > 0 AND y < 100");
query.where("x > 5 OR y = 'test'");
```
#### Inherited from
+25 -10
View File
@@ -560,6 +560,9 @@ postfilter(): VectorQuery
If this is called then filtering will happen after the vector search instead of
before.
This method is only available for vector search queries. Full-text search
filters are always applied before the search.
By default filtering will be performed before the vector search. This is how
filtering is typically understood to work. This prefilter step does add some
additional latency. Creating a scalar index on the filter column(s) can
@@ -790,7 +793,17 @@ where(predicate): this
A filter statement to be applied to this query.
The filter should be supplied as an SQL query string. For example:
Filters are applied before full-text and vector searches by default; no
separate prefilter call is needed. For vector searches only, use
[VectorQuery#postfilter](VectorQuery.md#postfilter) to apply the filter after the search.
The filter should be supplied as an SQL query string.
Filtering performance can often be improved by creating a scalar index
on the filter column(s).
Calling this multiple times combines the filters with a logical AND rather
than replacing the previous filter.
#### Parameters
@@ -800,18 +813,20 @@ The filter should be supplied as an SQL query string. For example:
`this`
#### Example
#### Examples
```ts
x > 10
y > 0 AND y < 100
x > 5 OR y = 'test'
const results = await table
.search("puppy", "fts")
.where("meta = 'foo'")
.limit(10)
.toArray();
```
Filtering performance can often be improved by creating a scalar index
on the filter column(s).
Calling this multiple times combines the filters with a logical AND rather
than replacing the previous filter.
```ts
query.where("x > 10");
query.where("y > 0 AND y < 100");
query.where("x > 5 OR y = 'test'");
```
#### Inherited from
+17
View File
@@ -47,5 +47,22 @@ test("filtering examples", async () => {
.limit(5)
.toArray();
// --8<-- [end:orderby_search]
const ftsTable = await db.createTable("myFts", [
{ text: "Frodo was a happy puppy", category: "pet" },
{ text: "A puppy training guide", category: "guide" },
{ text: "There are several kittens playing", category: "pet" },
]);
await ftsTable.createIndex("text", { config: lancedb.Index.fts() });
// --8<-- [start:fts_prefilter]
const ftsResults = await ftsTable
.search("puppy", "fts")
.where("category = 'pet'")
.limit(10)
.toArray();
// --8<-- [end:fts_prefilter]
expect(ftsResults).toHaveLength(1);
expect(ftsResults[0].category).toBe("pet");
});
});
+24 -5
View File
@@ -363,17 +363,33 @@ export class StandardQueryBase<
/**
* A filter statement to be applied to this query.
*
* The filter should be supplied as an SQL query string. For example:
* @example
* x > 10
* y > 0 AND y < 100
* x > 5 OR y = 'test'
* Filters are applied before full-text and vector searches by default; no
* separate prefilter call is needed. For vector searches only, use
* {@link VectorQuery#postfilter} to apply the filter after the search.
*
* The filter should be supplied as an SQL query string.
*
* Filtering performance can often be improved by creating a scalar index
* on the filter column(s).
*
* Calling this multiple times combines the filters with a logical AND rather
* than replacing the previous filter.
*
* @example Filter a full-text search
* ```ts
* const results = await table
* .search("puppy", "fts")
* .where("meta = 'foo'")
* .limit(10)
* .toArray();
* ```
*
* @example SQL filter expressions
* ```ts
* query.where("x > 10");
* query.where("y > 0 AND y < 100");
* query.where("x > 5 OR y = 'test'");
* ```
*/
where(predicate: string): this {
this.doCall((inner: NativeQueryType) => inner.onlyIf(predicate));
@@ -669,6 +685,9 @@ export class VectorQuery extends StandardQueryBase<NativeVectorQuery> {
* If this is called then filtering will happen after the vector search instead of
* before.
*
* This method is only available for vector search queries. Full-text search
* filters are always applied before the search.
*
* By default filtering will be performed before the vector search. This is how
* filtering is typically understood to work. This prefilter step does add some
* additional latency. Creating a scalar index on the filter column(s) can
+6 -2
View File
@@ -339,7 +339,9 @@ impl Table {
let transforms = NewColumnTransform::SqlExpressions(transforms);
let res = self
.inner_ref()?
.add_columns(transforms, None)
.add_columns()
.transform(transforms)
.execute()
.await
.default_error()?;
Ok(res.into())
@@ -356,7 +358,9 @@ impl Table {
let transforms = NewColumnTransform::AllNulls(schema);
let res = self
.inner_ref()?
.add_columns(transforms, None)
.add_columns()
.transform(transforms)
.execute()
.await
.default_error()?;
Ok(res.into())
+17 -3
View File
@@ -707,6 +707,9 @@ class LanceDBConnection(DBConnection):
self._namespace_client_properties = namespace_client_properties
if _inner is not None:
self._conn = _inner
# Native-derived wrappers resolve this in their async reconstruction
# path so construction never synchronously re-enters LOOP.
self._read_consistency_interval = read_consistency_interval
self._cached_namespace_client = None
return
@@ -756,11 +759,14 @@ class LanceDBConnection(DBConnection):
# storage_options. Also, this class really shouldn't be holding any state
# beyond _conn.
self._conn = AsyncConnection(LOOP.run(do_connect()))
# Keep property access synchronous so debugger introspection cannot wait on
# the background loop while that thread is suspended at a breakpoint.
self._read_consistency_interval = read_consistency_interval
self._cached_namespace_client: Optional[LanceNamespace] = None
@property
def read_consistency_interval(self) -> Optional[timedelta]:
return LOOP.run(self._conn.get_read_consistency_interval())
return self._read_consistency_interval
@property
def session(self) -> Optional[Session]:
@@ -771,8 +777,16 @@ class LanceDBConnection(DBConnection):
return self._conn.uri
@classmethod
def from_inner(cls, inner: LanceDbConnection):
return cls(None, _inner=inner)
def from_inner(
cls,
inner: LanceDbConnection,
read_consistency_interval: Optional[timedelta],
):
return cls(
None,
read_consistency_interval=read_consistency_interval,
_inner=inner,
)
def __repr__(self) -> str:
return f"{self.__class__.__name__}(uri={self._conn.uri!r})"
+1 -1
View File
@@ -226,7 +226,7 @@ class PermutationBuilder:
async def do_execute():
inner_tbl = await self._async.execute()
return LanceTable.from_inner(inner_tbl)
return await LanceTable.from_inner(inner_tbl)
return LOOP.run(do_execute())
+7 -3
View File
@@ -2182,11 +2182,15 @@ class LanceTable(Table):
return self.name
@classmethod
def from_inner(cls, tbl: LanceDBTable):
from .db import LanceDBConnection
async def from_inner(cls, tbl: LanceDBTable):
from .db import AsyncConnection, LanceDBConnection
async_tbl = AsyncTable(tbl)
conn = LanceDBConnection.from_inner(tbl.database())
inner_conn = tbl.database()
read_consistency_interval = await AsyncConnection(
inner_conn
).get_read_consistency_interval()
conn = LanceDBConnection.from_inner(inner_conn, read_consistency_interval)
return cls(
conn,
async_tbl.name,
+17
View File
@@ -77,6 +77,23 @@ def test_sync_repr_does_not_use_background_loop(tmp_path, monkeypatch):
assert repr(table) == f"LanceTable(name='test', _conn={db!r})"
def test_read_consistency_interval_does_not_use_background_loop(tmp_path, monkeypatch):
from lancedb.background_loop import LOOP
from lancedb.db import LanceDBConnection
consistency_interval = timedelta(seconds=5)
db = lancedb.connect(tmp_path, read_consistency_interval=consistency_interval)
db_from_inner = LanceDBConnection.from_inner(db._inner, consistency_interval)
def fail_run(*args, **kwargs):
raise AssertionError("properties should not use the Python background loop")
monkeypatch.setattr(LOOP, "run", fail_run)
assert db.read_consistency_interval == consistency_interval
assert db_from_inner.read_consistency_interval == consistency_interval
def test_ingest_pd(tmp_path):
db = lancedb.connect(tmp_path)
+20
View File
@@ -6,6 +6,7 @@ import math
import pytest
from lancedb import DBConnection, Table, connect
from lancedb.background_loop import LOOP
from lancedb.permutation import Permutation, Permutations, permutation_builder
@@ -31,6 +32,25 @@ def test_split_random_ratios(mem_db):
assert 65 <= split_1_count <= 75 # ~70% ± tolerance
def test_execute_does_not_reenter_background_loop(tmp_path, monkeypatch):
import threading
db = connect(tmp_path)
tbl = db.create_table("test_table", pa.table({"x": range(10)}))
original_run = LOOP.run
def fail_on_reentry(future):
assert threading.current_thread() is not LOOP.thread
return original_run(future)
monkeypatch.setattr(LOOP, "run", fail_on_reentry)
permutation_tbl = permutation_builder(tbl).execute()
assert permutation_tbl.count_rows() == 10
assert permutation_tbl._conn.read_consistency_interval is None
def test_split_random_counts(mem_db):
"""Test random splitting with absolute counts."""
tbl = mem_db.create_table(
+22
View File
@@ -6,6 +6,7 @@ import os
import sys
import threading
import warnings
from concurrent.futures import ThreadPoolExecutor
from datetime import date, datetime, timedelta
from time import sleep
from typing import List
@@ -2124,6 +2125,27 @@ def test_delete(mem_db: DBConnection):
assert table.to_arrow()["id"].to_pylist() == [1]
def test_concurrent_deletes_are_thread_safe(mem_db: DBConnection):
num_workers = 8
table = mem_db.create_table(
"my_table", data=[{"id": row_id} for row_id in range(num_workers)]
)
barrier = threading.Barrier(num_workers)
def delete(row_id: int):
barrier.wait()
return table.delete(f"id = {row_id}")
with ThreadPoolExecutor(max_workers=num_workers) as pool:
results = list(pool.map(delete, range(num_workers)))
assert all(result.num_deleted_rows == 1 for result in results)
assert sorted(result.version for result in results) == list(
range(2, num_workers + 2)
)
assert table.count_rows() == 0
def test_delete_expr(mem_db: DBConnection):
table = mem_db.create_table(
"my_table",
+15 -2
View File
@@ -745,6 +745,9 @@ impl Table {
#[allow(private_interfaces)]
pub fn delete(self_: PyRef<'_, Self>, condition: PredicateArg) -> PyResult<Bound<'_, PyAny>> {
// Do not hold the Python borrow across the await. The cloned Rust table
// handle is thread-safe and allows deletes on the same Python table to
// run concurrently without PyO3 reporting "Already borrowed".
let inner = self_.inner_ref()?.clone();
future_into_py(self_.py(), async move {
let result = match &condition {
@@ -1375,7 +1378,12 @@ impl Table {
let inner = self_.inner_ref()?.clone();
future_into_py(self_.py(), async move {
let result = inner.add_columns(definitions, None).await.infer_error()?;
let result = inner
.add_columns()
.transform(definitions)
.execute()
.await
.infer_error()?;
Ok(AddColumnsResult::from(result))
})
}
@@ -1389,7 +1397,12 @@ impl Table {
let inner = self_.inner_ref()?.clone();
future_into_py(self_.py(), async move {
let result = inner.add_columns(transform, None).await.infer_error()?;
let result = inner
.add_columns()
.transform(transform)
.execute()
.await
.infer_error()?;
Ok(AddColumnsResult::from(result))
})
}
+24 -19
View File
@@ -3089,10 +3089,12 @@ mod tests {
Box::pin(table.delete("false").map_ok(|_| ())),
Box::pin(
table
.add_columns(
NewColumnTransform::SqlExpressions(vec![("x".into(), "y".into())]),
None,
)
.add_columns()
.transform(NewColumnTransform::SqlExpressions(vec![(
"x".into(),
"y".into(),
)]))
.execute()
.map_ok(|_| ()),
),
Box::pin(async {
@@ -6388,13 +6390,12 @@ mod tests {
});
let result = table
.add_columns(
NewColumnTransform::SqlExpressions(vec![
("b".into(), "a + 1".into()),
("x".into(), "cast(NULL as int32)".into()),
]),
None,
)
.add_columns()
.transform(NewColumnTransform::SqlExpressions(vec![
("b".into(), "a + 1".into()),
("x".into(), "cast(NULL as int32)".into()),
]))
.execute()
.await
.unwrap();
@@ -7119,10 +7120,12 @@ mod tests {
}
"add_columns" => {
let _ = table
.add_columns(
NewColumnTransform::SqlExpressions(vec![("c".into(), "a + 1".into())]),
None,
)
.add_columns()
.transform(NewColumnTransform::SqlExpressions(vec![(
"c".into(),
"a + 1".into(),
)]))
.execute()
.await;
}
"drop_columns" => {
@@ -9880,10 +9883,12 @@ mod tests {
.await
.unwrap();
branch
.add_columns(
NewColumnTransform::SqlExpressions(vec![("b".into(), "a + 1".into())]),
None,
)
.add_columns()
.transform(NewColumnTransform::SqlExpressions(vec![(
"b".into(),
"a + 1".into(),
)]))
.execute()
.await
.unwrap();
branch
+4 -6
View File
@@ -65,6 +65,7 @@ use crate::utils::{PatchReadParam, PatchWriteParam, resolve_arrow_field_path};
use self::dataset::DatasetConsistencyWrapper;
use self::merge::MergeInsertBuilder;
pub mod add_columns;
mod add_data;
pub mod branch_merge;
mod create_index;
@@ -79,6 +80,7 @@ pub mod schema_evolution;
pub mod update;
pub mod write_progress;
use crate::index::waiter::wait_for_index;
pub use add_columns::AddColumnsBuilder;
#[cfg(feature = "remote")]
pub(crate) use add_data::PreprocessingOutput;
pub use add_data::{AddDataBuilder, AddDataMode, AddResult, NaNVectorBehavior};
@@ -1620,12 +1622,8 @@ impl Table {
}
/// Add new columns to the table, providing values to fill in.
pub async fn add_columns(
&self,
transforms: NewColumnTransform,
read_columns: Option<Vec<String>>,
) -> Result<AddColumnsResult> {
self.inner.add_columns(transforms, read_columns).await
pub fn add_columns(&self) -> AddColumnsBuilder {
AddColumnsBuilder::new(self.inner.clone())
}
/// Change a column's name or nullability.
+161
View File
@@ -0,0 +1,161 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: Copyright The LanceDB Authors
//! Builder for adding columns to a table.
use std::sync::Arc;
use lance::dataset::NewColumnTransform;
use super::BaseTable;
use super::schema_evolution::AddColumnsResult;
use crate::{Error, Result};
/// Adds columns to a table. See [`Table::add_columns`](super::Table::add_columns).
pub struct AddColumnsBuilder {
parent: Arc<dyn BaseTable>,
transform: Option<NewColumnTransform>,
read_columns: Option<Vec<String>>,
}
impl std::fmt::Debug for AddColumnsBuilder {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("AddColumnsBuilder")
.field("parent", &self.parent)
.field("has_transform", &self.transform.is_some())
.field("read_columns", &self.read_columns)
.finish()
}
}
impl AddColumnsBuilder {
pub(crate) fn new(parent: Arc<dyn BaseTable>) -> Self {
Self {
parent,
transform: None,
read_columns: None,
}
}
/// Set how the new columns' values are produced. Required.
pub fn transform(mut self, transform: NewColumnTransform) -> Self {
self.transform = Some(transform);
self
}
/// Limit which existing columns a [`NewColumnTransform::BatchUDF`] mapper
/// receives. Every other transform determines what it reads, so setting
/// this alongside one is an error rather than a silent no-op.
pub fn read_columns(mut self, columns: impl IntoIterator<Item = impl Into<String>>) -> Self {
self.read_columns = Some(columns.into_iter().map(Into::into).collect());
self
}
/// Add the columns.
pub async fn execute(self) -> Result<AddColumnsResult> {
let Self {
parent,
transform,
read_columns,
} = self;
let Some(transform) = transform else {
return Err(Error::InvalidInput {
message: "add_columns requires a transform".into(),
});
};
if read_columns.is_some() && !matches!(transform, NewColumnTransform::BatchUDF(_)) {
return Err(Error::InvalidInput {
message: "read_columns applies only to a BatchUDF transform; \
every other transform determines what it reads"
.into(),
});
}
parent.add_columns(transform, read_columns).await
}
}
#[cfg(test)]
mod tests {
use std::sync::Arc;
use arrow_array::{Int32Array, RecordBatch, record_batch};
use arrow_schema::{DataType, Field, Schema};
use lance::dataset::{BatchUDF, NewColumnTransform};
use crate::Table;
use crate::connect;
async fn table_with_two_columns(name: &str) -> Table {
let conn = connect("memory://").execute().await.unwrap();
let batch = record_batch!(("x", Int32, [1, 2, 3]), ("y", Int32, [10, 20, 30])).unwrap();
conn.create_table(name, batch).execute().await.unwrap()
}
#[tokio::test]
async fn test_requires_a_transform() {
let table = table_with_two_columns("no_transform").await;
let err = table.add_columns().execute().await.unwrap_err();
assert!(
err.to_string().contains("requires a transform"),
"got: {err}"
);
}
#[tokio::test]
async fn test_read_columns_with_sql_expressions_is_rejected() {
let table = table_with_two_columns("read_cols_sql").await;
let err = table
.add_columns()
.transform(NewColumnTransform::SqlExpressions(vec![(
"doubled".into(),
"x * 2".into(),
)]))
.read_columns(["x"])
.execute()
.await
.unwrap_err();
assert!(err.to_string().contains("BatchUDF"), "got: {err}");
let schema = table.schema().await.unwrap();
assert!(
schema.field_with_name("doubled").is_err(),
"a rejected call must not commit"
);
}
#[tokio::test]
async fn test_read_columns_limits_what_a_batch_udf_sees() {
let table = table_with_two_columns("read_cols_udf").await;
let output_schema = Arc::new(Schema::new(vec![Field::new("sum", DataType::Int32, true)]));
let mapper_schema = output_schema.clone();
let udf = BatchUDF {
mapper: Box::new(move |batch: &RecordBatch| {
assert!(batch.column_by_name("x").is_some());
assert!(batch.column_by_name("y").is_none(), "y was not requested");
let x = batch["x"].as_any().downcast_ref::<Int32Array>().unwrap();
let doubled: Int32Array = x.iter().map(|v| v.map(|v| v * 2)).collect();
Ok(RecordBatch::try_new(
mapper_schema.clone(),
vec![Arc::new(doubled)],
)?)
}),
output_schema,
result_checkpoint: None,
};
table
.add_columns()
.transform(NewColumnTransform::BatchUDF(udf))
.read_columns(["x"])
.execute()
.await
.unwrap();
let schema = table.schema().await.unwrap();
assert!(schema.field_with_name("sum").is_ok());
}
}
+9 -5
View File
@@ -576,10 +576,12 @@ mod tests {
// Add a new physical column AFTER the embedding column.
table
.add_columns(
NewColumnTransform::SqlExpressions(vec![("score".into(), "42.0".into())]),
None,
)
.add_columns()
.transform(NewColumnTransform::SqlExpressions(vec![(
"score".into(),
"42.0".into(),
)]))
.execute()
.await
.unwrap();
@@ -683,7 +685,9 @@ mod tests {
true,
)]));
table
.add_columns(NewColumnTransform::AllNulls(nested_schema), None)
.add_columns()
.transform(NewColumnTransform::AllNulls(nested_schema))
.execute()
.await
.unwrap();
+24 -19
View File
@@ -193,10 +193,12 @@ mod tests {
// Add a computed column
let result = table
.add_columns(
NewColumnTransform::SqlExpressions(vec![("doubled".into(), "id * 2".into())]),
None,
)
.add_columns()
.transform(NewColumnTransform::SqlExpressions(vec![(
"doubled".into(),
"id * 2".into(),
)]))
.execute()
.await
.unwrap();
@@ -251,13 +253,12 @@ mod tests {
// Add multiple columns at once
table
.add_columns(
NewColumnTransform::SqlExpressions(vec![
("y".into(), "x + 1".into()),
("z".into(), "x * x".into()),
]),
None,
)
.add_columns()
.transform(NewColumnTransform::SqlExpressions(vec![
("y".into(), "x + 1".into()),
("z".into(), "x * x".into()),
]))
.execute()
.await
.unwrap();
@@ -283,10 +284,12 @@ mod tests {
// Add a column with a constant value
table
.add_columns(
NewColumnTransform::SqlExpressions(vec![("constant".into(), "42".into())]),
None,
)
.add_columns()
.transform(NewColumnTransform::SqlExpressions(vec![(
"constant".into(),
"42".into(),
)]))
.execute()
.await
.unwrap();
@@ -659,10 +662,12 @@ mod tests {
// Add column increments version
let add_result = table
.add_columns(
NewColumnTransform::SqlExpressions(vec![("c".into(), "a + b".into())]),
None,
)
.add_columns()
.transform(NewColumnTransform::SqlExpressions(vec![(
"c".into(),
"a + b".into(),
)]))
.execute()
.await
.unwrap();
assert!(add_result.version > v1);
+254 -2
View File
@@ -9,14 +9,17 @@ use arrow_array::{
};
use arrow_schema::{DataType, Field, Fields, Schema};
use futures::TryStreamExt;
use lance::Dataset;
use lance_encoding::version::LanceFileVersion;
use lancedb::{
Connection, Error, Result, Table,
blob::{BlobRangeRequest, blob},
connect, connect_namespace,
database::listing::OPT_NEW_TABLE_ENABLE_STABLE_ROW_IDS,
database::listing::{
ListingDatabaseOptions, NewTableConfig, OPT_NEW_TABLE_ENABLE_STABLE_ROW_IDS,
},
query::{ExecutableQuery, QueryBase},
table::{AddDataMode, CompactionOptions, OptimizeAction},
table::{AddDataMode, CompactionOptions, OptimizeAction, OptimizeStats},
};
use tempfile::tempdir;
@@ -1075,3 +1078,252 @@ async fn fetch_blob_files_aligns_across_fragments_with_nulls_and_dups() -> Resul
}
Ok(())
}
/// Rows exercising the null/empty interleavings from
/// <https://github.com/lancedb/lancedb/issues/3744>: a payload, a null, a valid
/// empty value, then payloads whose descriptors a fragment rewrite used to zero.
fn null_empty_input_batch() -> RecordBatch {
let owned = [
Some(dedicated_blob_bytes(1)),
None,
Some(Vec::new()),
Some(dedicated_blob_bytes(4)),
Some(dedicated_blob_bytes(5)),
Some(dedicated_blob_bytes(6)),
];
let payloads: Vec<Option<&[u8]>> = owned.iter().map(|payload| payload.as_deref()).collect();
binary_input_batch(&[1, 2, 3, 4, 5, 6], &payloads)
}
/// One `(id, Some((payload length, first byte)))` per live row, or `(id, None)`
/// for a null blob. Comparing lengths and first bytes keeps failure output
/// readable where comparing whole payloads would not.
type BlobSummary = Vec<(i64, Option<(usize, Option<u8>)>)>;
/// The rows [`null_empty_input_batch`] leaves behind after `id IN (1, 4)` is
/// deleted: a null, a valid empty value, and the two payloads that follow them.
fn expected_null_empty_survivors() -> BlobSummary {
vec![
(2, None),
(3, Some((0, None))),
(5, Some((DEDICATED_BLOB_LEN, Some(5)))),
(6, Some((DEDICATED_BLOB_LEN, Some(6)))),
]
}
/// `optimize()` only rewrites a fragment when lance's compaction planner selects
/// it — here because the delete pushes the fragment past
/// `materialize_deletions_threshold` (0.1 by default; these tests delete 2 of 6
/// rows). Without this check, a planner or threshold change upstream would leave
/// both regression tests green while no rewrite happened at all.
fn assert_compacted(stats: &OptimizeStats) {
let metrics = stats
.compaction
.as_ref()
.expect("OptimizeAction::All runs compaction");
assert!(
metrics.fragments_removed >= 1,
"optimize() rewrote no fragment, so this test proves nothing: {metrics:?}"
);
}
fn summarize(rows: &[(i64, Option<Vec<u8>>)]) -> BlobSummary {
rows.iter()
.map(|(id, payload)| {
(
*id,
payload
.as_ref()
.map(|bytes| (bytes.len(), bytes.first().copied())),
)
})
.collect()
}
async fn sorted_id_rowid(table: &Table) -> Result<Vec<(i64, u64)>> {
let mut pairs = collect_id_rowid(table).await?;
pairs.sort_by_key(|(id, _)| *id);
Ok(pairs)
}
/// `{position, size}` descriptors of a legacy v1 blob column, keyed by `id`.
async fn v1_blob_descriptors(table: &Table) -> Result<Vec<(i64, Option<(u64, u64)>)>> {
let batches = table
.query()
.execute()
.await?
.try_collect::<Vec<_>>()
.await?;
let batch = arrow_select::concat::concat_batches(&batches[0].schema(), &batches).unwrap();
let ids = batch
.column_by_name("id")
.unwrap()
.as_any()
.downcast_ref::<Int64Array>()
.unwrap();
let descriptors = batch
.column_by_name("image")
.unwrap()
.as_any()
.downcast_ref::<StructArray>()
.expect("v1 blob column reads back as a descriptor struct");
let position = descriptors
.column_by_name("position")
.unwrap()
.as_any()
.downcast_ref::<UInt64Array>()
.unwrap();
let size = descriptors
.column_by_name("size")
.unwrap()
.as_any()
.downcast_ref::<UInt64Array>()
.unwrap();
let mut rows: Vec<(i64, Option<(u64, u64)>)> = (0..batch.num_rows())
.map(|row| {
let descriptor =
(!descriptors.is_null(row)).then(|| (position.value(row), size.value(row)));
(ids.value(row), descriptor)
})
.collect();
rows.sort_by_key(|(id, _)| *id);
Ok(rows)
}
/// Payload bytes of every live row of a legacy v1 blob column, keyed by `id`.
/// [`Table::fetch_blobs`] rejects v1 columns, so read them through lance.
async fn v1_blob_payloads(dataset_uri: &str, table: &Table) -> Result<Vec<(i64, Option<Vec<u8>>)>> {
let pairs = sorted_id_rowid(table).await?;
let row_ids: Vec<u64> = pairs.iter().map(|(_, row_id)| *row_id).collect();
let dataset = Arc::new(Dataset::open(dataset_uri).await?);
let files = dataset.take_blobs(&row_ids, "image").await?;
assert_eq!(
files.len(),
pairs.len(),
"take_blobs returned {} handles for {} live rows",
files.len(),
pairs.len()
);
let mut rows = Vec::with_capacity(pairs.len());
for ((id, _), file) in pairs.iter().zip(files) {
let payload = match file {
Some(file) => Some(file.read().await?.to_vec()),
None => None,
};
rows.push((*id, payload));
}
Ok(rows)
}
/// Length and first byte of every live blob v2 value, keyed by `id`.
async fn blob_v2_values(table: &Table) -> Result<BlobSummary> {
let pairs = sorted_id_rowid(table).await?;
let row_ids: Vec<u64> = pairs.iter().map(|(_, row_id)| *row_id).collect();
let bytes = table.fetch_blobs("image", &row_ids).await?;
Ok(pairs
.iter()
.enumerate()
.map(|(slot, (id, _))| {
let value = (!bytes.is_null(slot))
.then(|| (bytes.value(slot).len(), bytes.value(slot).first().copied()));
(*id, value)
})
.collect())
}
/// Regression test for [#3744]: on storage 2.0 (legacy v1 descriptors),
/// compaction rewrote every payload following a null or empty value in the same
/// fragment as `{position: 0, size: 0}`, so the payload bytes read back as `b""`
/// and the new fragment no longer referenced them at all.
///
/// [#3744]: https://github.com/lancedb/lancedb/issues/3744
#[tokio::test]
async fn optimize_preserves_v1_blob_payloads_with_null_and_empty() -> Result<()> {
let tmp = tempdir().unwrap();
let db_uri = tmp.path().to_str().unwrap().to_string();
let db = connect(&db_uri)
.database_options(&ListingDatabaseOptions {
new_table_config: NewTableConfig {
data_storage_version: Some(LanceFileVersion::V2_0),
..Default::default()
},
..Default::default()
})
.execute()
.await?;
let legacy = Field::new("image", DataType::LargeBinary, true).with_metadata(
std::collections::HashMap::from([("lance-encoding:blob".to_string(), "true".to_string())]),
);
let schema = Arc::new(Schema::new(vec![
Field::new("id", DataType::Int64, false),
legacy,
]));
let table = db.create_empty_table("t", schema).execute().await?;
table.add(null_empty_input_batch()).execute().await?;
assert_eq!(
storage_format_version(&table).await,
LanceFileVersion::V2_0.resolve(),
"v1 blob descriptors only exist below storage 2.2"
);
let dataset_uri = table.uri().await?;
// Any rewrite triggers it; deleting rows is the shape from the issue.
table.delete("id IN (1, 4)").await?;
let descriptors_before = v1_blob_descriptors(&table).await?;
let before = v1_blob_payloads(&dataset_uri, &table).await?;
assert_eq!(
summarize(&before),
expected_null_empty_survivors(),
"test setup no longer produces the null/empty/payload mix"
);
let stats = table.optimize(OptimizeAction::All).await?;
assert_compacted(&stats);
let descriptors_after = v1_blob_descriptors(&table).await?;
let after = v1_blob_payloads(&dataset_uri, &table).await?;
assert_eq!(
summarize(&after),
summarize(&before),
"optimize() lost blob payloads; descriptors before={descriptors_before:?} after={descriptors_after:?}"
);
assert!(after == before, "optimize() changed blob payload bytes");
Ok(())
}
/// Regression test for the blob v2 half of [#3744]: compaction rewrote a valid
/// empty value as null, destroying the null-vs-empty distinction.
///
/// [#3744]: https://github.com/lancedb/lancedb/issues/3744
#[tokio::test]
async fn optimize_preserves_blob_v2_null_and_empty_distinction() -> Result<()> {
let tmp = tempdir().unwrap();
let db = connect(tmp.path().to_str().unwrap()).execute().await?;
let table = db
.create_empty_table("t", blob_table_schema())
.execute()
.await?;
table.add(null_empty_input_batch()).execute().await?;
assert!(
storage_format_version(&table).await >= LanceFileVersion::V2_2,
"blob v2 columns require storage >= 2.2"
);
table.delete("id IN (1, 4)").await?;
let before = blob_v2_values(&table).await?;
assert_eq!(
before,
expected_null_empty_survivors(),
"test setup no longer produces the null/empty/payload mix"
);
let stats = table.optimize(OptimizeAction::All).await?;
assert_compacted(&stats);
assert_eq!(
blob_v2_values(&table).await?,
before,
"optimize() changed blob v2 values"
);
Ok(())
}