mirror of
https://github.com/lancedb/lancedb.git
synced 2026-08-18 12:08:35 +00:00
fix: hybrid search silently ignores .offset() (#3769)
## Summary `LanceHybridQueryBuilder` (sync hybrid search, `table.search(query_type="hybrid")`) silently ignored `.offset()`. `self._offset` was never forwarded to the vector/FTS sub-queries and never applied when slicing the final combined/reranked result, so `.offset(N)` behaved identically to `.offset(0)` — no error, just wrong pagination. Fixes #3765 ## Changes - `_create_query_builders()`: each sub-query now fetches `limit + offset` rows so there's enough data to slice the correct window out of after combining/reranking. - `_combine_hybrid_results()` / `to_arrow()`: the final table is sliced with `offset=self._offset` instead of always starting at 0. ## Test plan - [x] New regression test `test_hybrid_query_offset` in `python/python/tests/test_hybrid_query.py` - [x] `uv run --extra tests pytest python/tests/test_hybrid_query.py -vv` — 13 passed - [x] `uv run --extra dev ruff format` / `ruff check` — clean Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Will Jones <willjones127@gmail.com>
This commit is contained in:
@@ -2235,6 +2235,7 @@ class LanceHybridQueryBuilder(LanceQueryBuilder):
|
||||
reranker=self._reranker,
|
||||
limit=self._limit,
|
||||
with_row_ids=True,
|
||||
offset=self._offset,
|
||||
)
|
||||
return self._finish_hybrid_results(results)
|
||||
|
||||
@@ -2256,6 +2257,7 @@ class LanceHybridQueryBuilder(LanceQueryBuilder):
|
||||
reranker,
|
||||
limit: int,
|
||||
with_row_ids: bool,
|
||||
offset: Optional[int] = None,
|
||||
) -> pa.Table:
|
||||
if norm == "rank":
|
||||
vector_results = LanceHybridQueryBuilder._rank(vector_results, "_distance")
|
||||
@@ -2332,7 +2334,7 @@ class LanceHybridQueryBuilder(LanceQueryBuilder):
|
||||
score_i = results.column_names.index("_score")
|
||||
results = results.set_column(score_i, "_score", original_scores)
|
||||
|
||||
results = results.slice(length=limit)
|
||||
results = results.slice(offset=offset or 0, length=limit)
|
||||
|
||||
if not with_row_ids:
|
||||
results = results.drop(["_rowid"])
|
||||
@@ -2679,8 +2681,12 @@ class LanceHybridQueryBuilder(LanceQueryBuilder):
|
||||
|
||||
# Apply common configurations
|
||||
if self._limit:
|
||||
self._vector_query.limit(self._limit)
|
||||
self._fts_query.limit(self._limit)
|
||||
# The final offset/limit window is sliced out of the combined,
|
||||
# reranked results, so each sub-query must fetch enough rows to
|
||||
# cover the skipped prefix as well as the window itself.
|
||||
sub_query_limit = self._limit + (self._offset or 0)
|
||||
self._vector_query.limit(sub_query_limit)
|
||||
self._fts_query.limit(sub_query_limit)
|
||||
if self._columns:
|
||||
self._vector_query.select(self._columns)
|
||||
self._fts_query.select(self._columns)
|
||||
|
||||
@@ -203,6 +203,31 @@ async def test_async_hybrid_query_default_limit(table: AsyncTable):
|
||||
assert texts.count("a") == 1
|
||||
|
||||
|
||||
def test_hybrid_query_offset(sync_table: Table):
|
||||
# The offset window of a hybrid query must be a suffix of the same query
|
||||
# run without an offset -- it must not be silently ignored.
|
||||
full = (
|
||||
sync_table.search(query_type="hybrid")
|
||||
.vector([0.0, 0.4])
|
||||
.text("dog")
|
||||
.limit(4)
|
||||
.with_row_id(True)
|
||||
.to_arrow()
|
||||
)
|
||||
assert len(full) == 4
|
||||
|
||||
offset_result = (
|
||||
sync_table.search(query_type="hybrid")
|
||||
.vector([0.0, 0.4])
|
||||
.text("dog")
|
||||
.offset(2)
|
||||
.limit(2)
|
||||
.with_row_id(True)
|
||||
.to_arrow()
|
||||
)
|
||||
assert offset_result["_rowid"].to_pylist() == full["_rowid"].to_pylist()[2:]
|
||||
|
||||
|
||||
def test_hybrid_query_minimum_nprobes_zero_raises(sync_table: Table):
|
||||
# minimum_nprobes(0) must raise the same validation error a plain vector
|
||||
# query raises, not silently no-op because 0 is falsy.
|
||||
|
||||
Reference in New Issue
Block a user