fix(query): restore the empty-result and real-row-id contracts

Three defects from review, each reproduced before it was touched.

A hybrid query matching nothing errored instead of returning nothing. When a
leg comes back with no batches, `query_schemas` synthesizes a schema that
already carries `_rowid`, and stamping then tried to append a second column of
that name. The column is ours to define, so replace it rather than append.

The unit test covering that case stripped `_rowid` by hand first, claiming to
do "what the caller does" -- the caller does no such thing, so the test
asserted a shape `run_hybrid` never produces. It now runs the synthesized
schemas untouched, and a mock-server test covers a no-match query end to end.

An explicit `with_row_id` combined with `return_score="all"` was accepted and
then silently stripped, in either call order, because both set the same
`_with_row_id` flag and the guard inferred caller intent from it. Record the
caller's own request instead of inferring it: `with_row_id` on the hybrid
builder sets `_caller_requested_row_id`, and `rerank` sets `_with_row_id`
directly without going through it.

A projected Blob v2 column slipped the same way. `blob_auto_row_id_for_scan`
reports whether a row id would be added *automatically*, so it answers False
once anything else has turned `_with_row_id` on -- including that reranker --
while the blob still needs a real row id to fetch through. Check the projected
sources directly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Daniel Rammer
2026-09-23 14:20:56 -05:00
co-authored by Claude Opus 5
parent 90b99af20f
commit 83cfcb84d2
4 changed files with 119 additions and 20 deletions
+22 -7
View File
@@ -2332,8 +2332,9 @@ class LanceHybridQueryBuilder(LanceQueryBuilder):
# to make that possible.
self._fusion_pk = None
self._injected_pk = []
# `rerank(return_score="all")` turns `_with_row_id` on for its own use.
self._reranker_requested_row_id = False
# `_with_row_id` is also turned on by `rerank(return_score="all")` for
# the reranker's own use, so it cannot answer "did the caller ask?".
self._caller_requested_row_id = False
def _validate_query(self, query, vector=None, text=None):
if query is not None and (vector is not None or text is not None):
@@ -2355,6 +2356,16 @@ class LanceHybridQueryBuilder(LanceQueryBuilder):
return vector_query, text_query
def with_row_id(self, with_row_id: bool) -> Self:
"""Set whether to return row ids.
Recorded separately from `_with_row_id`, which `rerank(return_score=
"all")` also sets for its own use — on a MemWAL table only a caller who
asked is refused, and the reranker still falls back to the primary key.
"""
self._caller_requested_row_id = with_row_id
return super().with_row_id(with_row_id)
def phrase_query(self, phrase_query: bool = True) -> LanceHybridQueryBuilder:
"""Set whether to use phrase query.
@@ -2390,7 +2401,7 @@ class LanceHybridQueryBuilder(LanceQueryBuilder):
# `return_score="all"` turns row ids on for the reranker, not the
# caller, so it still falls back; only a caller who asked for them
# is refused, and with the reason rather than a bare 400.
if self._user_requested_row_id() and not self._reranker_requested_row_id:
if self._caller_requested_row_id:
raise NotImplementedError(_WAL_ROW_ID_UNSUPPORTED) from e
# Learned, so later queries on this table skip straight to it.
self._table._note_hybrid_pk_fusion()
@@ -2436,9 +2447,13 @@ class LanceHybridQueryBuilder(LanceQueryBuilder):
or self._table._hybrid_pk_fusion_learned() is not True
):
return None
if self._user_requested_row_id() and not self._reranker_requested_row_id:
if self._caller_requested_row_id:
raise NotImplementedError(_WAL_ROW_ID_UNSUPPORTED)
if self._blob_auto_row_id_enabled():
# Not `_blob_auto_row_id_enabled`: that reports whether a row id would
# be added *automatically*, so it goes quiet once anything else has
# turned `_with_row_id` on — including `return_score="all"`. A projected
# blob needs a real row id to fetch through either way.
if blob_v2_projection_sources(self._table.schema, self._columns):
raise NotImplementedError(
"hybrid search cannot project a blob column on a MemWAL table: "
"fetching blobs needs a real _rowid, and the fresh tier has no "
@@ -2640,8 +2655,8 @@ class LanceHybridQueryBuilder(LanceQueryBuilder):
# The reranker needs the join column to carry the per-leg scores
# across; the caller did not ask for row ids and must not be
# refused on a MemWAL table because of it.
self._reranker_requested_row_id = True
self.with_row_id(True)
# Deliberately not through `with_row_id`, which records caller intent.
self._with_row_id = True
return self
+43
View File
@@ -608,3 +608,46 @@ def test_wal_hybrid_fuses_on_the_key_and_drops_it_again():
assert "_rowid" not in results.column_names, "the surrogate is internal"
assert "id" not in results.column_names, "the caller selected only `text`"
assert sorted(results.column("text").to_pylist()) == ["a", "b", "c"]
@pytest.mark.parametrize(
"row_id_first", [True, False], ids=["row_id_first", "rerank_first"]
)
def test_wal_hybrid_refuses_row_id_even_with_return_score_all(row_id_first: bool):
"""`return_score="all"` sets the same flag the caller's request does, so
asking for both must still be refused — in either call order."""
builder = _wal_hybrid()
reranker = RRFReranker(return_score="all")
if row_id_first:
builder = builder.with_row_id(True).rerank(reranker)
else:
builder = builder.rerank(reranker).with_row_id(True)
with pytest.raises(NotImplementedError, match="use_lsm"):
builder._create_query_builders()
def test_wal_hybrid_refuses_a_blob_projection_with_return_score_all():
"""A projected blob needs a real row id to fetch through, which the
reranker turning `_with_row_id` on does not provide."""
schema = pa.schema(
[
pa.field(
"id",
pa.string(),
metadata={b"lance-schema:unenforced-primary-key:position": b"1"},
),
pa.field("text", pa.string()),
lancedb.blob("blob"),
]
)
builder = (
LanceHybridQueryBuilder(_wal_table(schema))
.vector([0.1, 0.2])
.text("puppy")
.select(["blob"])
.rerank(RRFReranker(return_score="all"))
)
with pytest.raises(NotImplementedError, match="blob"):
builder._create_query_builders()