mirror of
https://github.com/lancedb/lancedb.git
synced 2026-11-03 01:06:52 +00:00
refactor(query): learn the hybrid fusion key from a refusal, not a probe
Asking `get_lsm_write_spec` up front to decide how to fuse taxed every hybrid query, on every table, to learn something almost no table needs -- and that endpoint is not cheap: it opens the table and does a `checkout_latest()` manifest reload so a just-set spec is visible. Ask for `_rowid` optimistically instead and fall back to the primary key when the server refuses. Only a MemWAL table refuses; the refusal is raised before the query is planned, so it costs nothing server-side; and it had to be handled regardless, because a write spec installed elsewhere can arrive between any two queries. The proactive probe was belt and braces over a mechanism that already had to exist. A base table now sends nothing beyond its two legs, covered by a test that counts request paths. A MemWAL table pays one refused attempt, then remembers -- `BaseTable::hybrid_pk_fusion_learned` / `note_hybrid_pk_fusion`, seeded by the refusal and never fetched, so reading it is free. Deletes the `lsm_enabled` accessor from `BaseTable`, `Table`, the PyO3 and Python tables, and the type stubs, along with the cache that backed it. A caller who explicitly asked for `_rowid` still gets the reason rather than a bare 400: the refusal is translated into the same message the anticipated path raises. `return_score="all"` turns row ids on for the reranker rather than the caller, so it falls back like any other query -- an e2e run caught both of these. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
959645f1c7
commit
25176778a3
@@ -422,7 +422,8 @@ class Table:
|
||||
async def set_lsm_write_spec(self, spec: LsmWriteSpec) -> None: ...
|
||||
async def unset_lsm_write_spec(self) -> None: ...
|
||||
async def get_lsm_write_spec(self) -> Optional[LsmWriteSpec]: ...
|
||||
async def lsm_enabled(self) -> bool: ...
|
||||
def hybrid_pk_fusion_learned(self) -> bool: ...
|
||||
def note_hybrid_pk_fusion(self) -> None: ...
|
||||
async def checkpoint_lsm(self) -> None: ...
|
||||
async def flush_lsm(self) -> None: ...
|
||||
async def compact_lsm(self) -> None: ...
|
||||
|
||||
@@ -2208,6 +2208,29 @@ def _pk_columns(schema: pa.Schema) -> List[str]:
|
||||
return [name for *_, name in sorted(keyed)]
|
||||
|
||||
|
||||
# Both layers raise this same sentence verbatim -- sophon's
|
||||
# `LSM_WITH_ROW_ID_UNSUPPORTED` says so explicitly, and the OSS LSM scanner
|
||||
# builds it from the same words -- so this substring survives either path and
|
||||
# the wrapping each adds. It is a coupling to a message rather than a code,
|
||||
# because the refusal arrives as a generic 400; if the server ever rewords it
|
||||
# the fallback stops firing and hybrid on a MemWAL table errors visibly again,
|
||||
# which is loud rather than silently wrong.
|
||||
_WAL_ROW_ID_REFUSAL = "does not support with_row_id"
|
||||
|
||||
_WAL_ROW_ID_UNSUPPORTED = (
|
||||
"hybrid search on a MemWAL table cannot return _rowid: the fresh tier has "
|
||||
"no stable row id, and the ids the fusion joins on are synthesized from "
|
||||
"the primary key. Set use_lsm(False) to read the base table only (results "
|
||||
"will exclude un-compacted MemWAL data)"
|
||||
)
|
||||
|
||||
|
||||
def _is_wal_row_id_refusal(error: BaseException) -> bool:
|
||||
"""Whether `error` is the server declining `_rowid` because the table is
|
||||
MemWAL-backed, as opposed to any other failure the query might hit."""
|
||||
return _WAL_ROW_ID_REFUSAL in str(error)
|
||||
|
||||
|
||||
def _with_pk_columns(columns, pk_columns: List[str]):
|
||||
"""Add the key columns a projection is missing.
|
||||
|
||||
@@ -2352,6 +2375,28 @@ class LanceHybridQueryBuilder(LanceQueryBuilder):
|
||||
raise NotImplementedError("to_query_object not yet supported on a hybrid query")
|
||||
|
||||
def to_arrow(self, *, timeout: Optional[timedelta] = None) -> pa.Table:
|
||||
"""Ask for `_rowid` first, falling back to the primary key only when the
|
||||
server refuses, rather than paying a round trip up front to find out.
|
||||
|
||||
Only a MemWAL table refuses, and the refusal has to be handled anyway
|
||||
-- a write spec installed elsewhere can arrive between any two queries.
|
||||
It costs nothing server-side: it is raised before the query is planned.
|
||||
"""
|
||||
try:
|
||||
return self._run_hybrid(timeout=timeout)
|
||||
except Exception as e:
|
||||
if self._fusion_pk is not None or not _is_wal_row_id_refusal(e):
|
||||
raise
|
||||
# `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:
|
||||
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()
|
||||
return self._run_hybrid(timeout=timeout)
|
||||
|
||||
def _run_hybrid(self, *, timeout: Optional[timedelta] = None) -> pa.Table:
|
||||
self._create_query_builders()
|
||||
fts_query, vector_query = self._fts_query, self._vector_query
|
||||
if self._fusion_pk is None:
|
||||
@@ -2379,21 +2424,20 @@ class LanceHybridQueryBuilder(LanceQueryBuilder):
|
||||
def _resolve_fusion_pk(self) -> Optional[List[str]]:
|
||||
"""The key columns the two legs must be fused on, or None for `_rowid`.
|
||||
|
||||
A MemWAL table rejects `with_row_id` before it plans, because the fresh
|
||||
tier has no stable row id, so there the legs are joined on a surrogate
|
||||
built from the primary key instead.
|
||||
`_rowid` until a MemWAL table refuses it, after which the legs are
|
||||
joined on a surrogate built from the primary key. The refusal is what
|
||||
teaches us, rather than a round trip up front that every table would
|
||||
pay to learn something almost none of them need.
|
||||
"""
|
||||
# Only an unambiguous True forks to PK mode: a table type that does not
|
||||
# answer the question properly keeps today's `_rowid` behavior.
|
||||
if self._use_lsm is False or self._table.lsm_enabled() is not True:
|
||||
# Only an unambiguous True forks to the key: a table type that answers
|
||||
# the question loosely keeps the `_rowid` path.
|
||||
if (
|
||||
self._use_lsm is False
|
||||
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:
|
||||
raise NotImplementedError(
|
||||
"hybrid search on a MemWAL table cannot return _rowid: the fresh "
|
||||
"tier has no stable row id, and the ids the fusion joins on are "
|
||||
"synthesized from the primary key. Set use_lsm(False) to read the "
|
||||
"base table only (results will exclude un-compacted MemWAL data)"
|
||||
)
|
||||
raise NotImplementedError(_WAL_ROW_ID_UNSUPPORTED)
|
||||
if self._blob_auto_row_id_enabled():
|
||||
raise NotImplementedError(
|
||||
"hybrid search cannot project a blob column on a MemWAL table: "
|
||||
@@ -4061,25 +4105,20 @@ class AsyncHybridQuery(AsyncStandardQuery, AsyncVectorQueryBase):
|
||||
async def _resolve_fusion_pk(self) -> Optional[List[str]]:
|
||||
"""The key columns the two legs must be fused on, or None for `_rowid`.
|
||||
|
||||
A MemWAL table rejects `with_row_id` before it plans, because the fresh
|
||||
tier has no stable row id, so there the legs are joined on a surrogate
|
||||
built from the primary key instead.
|
||||
`_rowid` until a MemWAL table refuses it, after which the legs are
|
||||
joined on a surrogate built from the primary key. The refusal is what
|
||||
teaches us, rather than a round trip up front that every table would
|
||||
pay to learn something almost none of them need.
|
||||
"""
|
||||
if self._table is None:
|
||||
return None
|
||||
if self._inner.to_query_request().use_lsm is False:
|
||||
return None
|
||||
# Only an unambiguous True forks to PK mode: a table type that does not
|
||||
# answer the question properly keeps today's `_rowid` behavior.
|
||||
if (await self._table.lsm_enabled()) is not True:
|
||||
# Only an unambiguous True forks to the key; see the sync resolver.
|
||||
if self._table._hybrid_pk_fusion_learned() is not True:
|
||||
return None
|
||||
if self._user_requested_row_id():
|
||||
raise NotImplementedError(
|
||||
"hybrid search on a MemWAL table cannot return _rowid: the fresh "
|
||||
"tier has no stable row id, and the ids the fusion joins on are "
|
||||
"synthesized from the primary key. Set use_lsm(False) to read the "
|
||||
"base table only (results will exclude un-compacted MemWAL data)"
|
||||
)
|
||||
raise NotImplementedError(_WAL_ROW_ID_UNSUPPORTED)
|
||||
schema = await self._table.schema()
|
||||
if blob_auto_row_id_for_scan(
|
||||
schema,
|
||||
@@ -4154,6 +4193,28 @@ class AsyncHybridQuery(AsyncStandardQuery, AsyncVectorQueryBase):
|
||||
*,
|
||||
max_batch_length: Optional[int] = None,
|
||||
timeout: Optional[timedelta] = None,
|
||||
) -> AsyncRecordBatchReader:
|
||||
"""See `LanceHybridQueryBuilder.to_arrow` for why the key is learned
|
||||
from a refusal rather than probed for."""
|
||||
try:
|
||||
return await self._run_hybrid(
|
||||
max_batch_length=max_batch_length, timeout=timeout
|
||||
)
|
||||
except Exception as e:
|
||||
if self._fusion_pk is not None or not _is_wal_row_id_refusal(e):
|
||||
raise
|
||||
if self._user_requested_row_id():
|
||||
raise NotImplementedError(_WAL_ROW_ID_UNSUPPORTED) from e
|
||||
self._table._note_hybrid_pk_fusion()
|
||||
return await self._run_hybrid(
|
||||
max_batch_length=max_batch_length, timeout=timeout
|
||||
)
|
||||
|
||||
async def _run_hybrid(
|
||||
self,
|
||||
*,
|
||||
max_batch_length: Optional[int] = None,
|
||||
timeout: Optional[timedelta] = None,
|
||||
) -> AsyncRecordBatchReader:
|
||||
fts_query, vec_query, limit, offset = await self._create_child_queries()
|
||||
|
||||
|
||||
@@ -1023,9 +1023,12 @@ class RemoteTable(Table):
|
||||
"""Read the installed LsmWriteSpec, or ``None``."""
|
||||
return LOOP.run(self._table.get_lsm_write_spec())
|
||||
|
||||
def lsm_enabled(self) -> bool:
|
||||
"""Whether reads route through the MemWAL. Cached by the client."""
|
||||
return LOOP.run(self._table.lsm_enabled())
|
||||
def _hybrid_pk_fusion_learned(self) -> bool:
|
||||
# `self._table` is the async table, which owns the Rust handle.
|
||||
return self._table._hybrid_pk_fusion_learned()
|
||||
|
||||
def _note_hybrid_pk_fusion(self) -> None:
|
||||
self._table._note_hybrid_pk_fusion()
|
||||
|
||||
def checkpoint_lsm(self) -> None:
|
||||
"""Synchronous version of
|
||||
|
||||
@@ -2467,14 +2467,17 @@ class Table(ABC):
|
||||
to check if the table is already using the new path style.
|
||||
"""
|
||||
|
||||
def lsm_enabled(self) -> bool:
|
||||
"""Whether reads on this table route through the MemWAL.
|
||||
def _hybrid_pk_fusion_learned(self) -> bool:
|
||||
"""Whether a hybrid query here has already been refused ``_rowid``.
|
||||
|
||||
``False`` for table types that have no MemWAL write path, which is what
|
||||
lets query paths ask unconditionally.
|
||||
Learned from a refusal, never probed, so asking is free. ``False`` for
|
||||
table types that never refuse.
|
||||
"""
|
||||
return False
|
||||
|
||||
def _note_hybrid_pk_fusion(self) -> None:
|
||||
"""Remember a ``_rowid`` refusal, so later hybrid queries skip it."""
|
||||
|
||||
|
||||
class LanceTable(Table):
|
||||
"""
|
||||
@@ -4370,10 +4373,12 @@ class LanceTable(Table):
|
||||
[`AsyncTable.get_lsm_write_spec`][lancedb.AsyncTable.get_lsm_write_spec]."""
|
||||
return LOOP.run(self._table.get_lsm_write_spec())
|
||||
|
||||
def lsm_enabled(self) -> bool:
|
||||
"""Whether reads route through the MemWAL. See
|
||||
[`AsyncTable.lsm_enabled`][lancedb.AsyncTable.lsm_enabled]."""
|
||||
return LOOP.run(self._table.lsm_enabled())
|
||||
def _hybrid_pk_fusion_learned(self) -> bool:
|
||||
# `self._table` is the async table, which owns the Rust handle.
|
||||
return self._table._hybrid_pk_fusion_learned()
|
||||
|
||||
def _note_hybrid_pk_fusion(self) -> None:
|
||||
self._table._note_hybrid_pk_fusion()
|
||||
|
||||
def checkpoint_lsm(self) -> None:
|
||||
"""Synchronous version of
|
||||
@@ -5114,13 +5119,13 @@ class AsyncTable:
|
||||
"""
|
||||
return await self._inner.get_lsm_write_spec()
|
||||
|
||||
async def lsm_enabled(self) -> bool:
|
||||
"""Whether reads on this table route through the MemWAL.
|
||||
def _hybrid_pk_fusion_learned(self) -> bool:
|
||||
"""See [`Table._hybrid_pk_fusion_learned`][lancedb.table.Table]."""
|
||||
return self._inner.hybrid_pk_fusion_learned()
|
||||
|
||||
Equivalent to testing `get_lsm_write_spec` for ``None``, but cached by
|
||||
the remote table, so query paths can ask on every call.
|
||||
"""
|
||||
return await self._inner.lsm_enabled()
|
||||
def _note_hybrid_pk_fusion(self) -> None:
|
||||
"""See [`Table._note_hybrid_pk_fusion`][lancedb.table.Table]."""
|
||||
self._inner.note_hybrid_pk_fusion()
|
||||
|
||||
async def checkpoint_lsm(self) -> None:
|
||||
"""Converge this table's LSM write path into its base table.
|
||||
|
||||
@@ -498,9 +498,10 @@ def test_normalize_scores():
|
||||
|
||||
|
||||
def _wal_table(schema: pa.Schema) -> mock.Mock:
|
||||
"""A stand-in for a MemWAL-backed table: the fusion cannot use `_rowid`."""
|
||||
"""A MemWAL-backed table a previous query has already been refused
|
||||
`_rowid` on, so the fusion goes straight to the primary key."""
|
||||
table = mock.Mock()
|
||||
table.lsm_enabled.return_value = True
|
||||
table._hybrid_pk_fusion_learned.return_value = True
|
||||
table.schema = schema
|
||||
return table
|
||||
|
||||
@@ -554,7 +555,7 @@ def test_wal_hybrid_without_a_projection_leaves_it_alone():
|
||||
|
||||
def test_base_table_hybrid_still_joins_on_row_ids():
|
||||
table = mock.Mock()
|
||||
table.lsm_enabled.return_value = False
|
||||
table._hybrid_pk_fusion_learned.return_value = False
|
||||
builder = LanceHybridQueryBuilder(table).vector([0.1, 0.2]).text("puppy")
|
||||
builder._create_query_builders()
|
||||
|
||||
|
||||
+11
-7
@@ -1481,13 +1481,17 @@ impl Table {
|
||||
})
|
||||
}
|
||||
|
||||
/// Whether reads on this table route through the MemWAL. Cached by the
|
||||
/// remote table, so the hybrid query path can ask on every call.
|
||||
pub fn lsm_enabled(self_: PyRef<'_, Self>) -> PyResult<Bound<'_, PyAny>> {
|
||||
let inner = self_.inner_ref()?.clone();
|
||||
future_into_py(self_.py(), async move {
|
||||
inner.lsm_enabled().await.infer_error()
|
||||
})
|
||||
/// Whether a hybrid query on this table has already been refused `_rowid`.
|
||||
/// Learned from a previous refusal, never probed, so this is free.
|
||||
pub fn hybrid_pk_fusion_learned(self_: PyRef<'_, Self>) -> PyResult<bool> {
|
||||
Ok(self_.inner_ref()?.base_table().hybrid_pk_fusion_learned())
|
||||
}
|
||||
|
||||
/// Remember that this table refused `_rowid`, so later hybrid queries skip
|
||||
/// straight to the primary-key fusion.
|
||||
pub fn note_hybrid_pk_fusion(self_: PyRef<'_, Self>) -> PyResult<()> {
|
||||
self_.inner_ref()?.base_table().note_hybrid_pk_fusion();
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Converge the table's LSM write path into its base table.
|
||||
|
||||
Reference in New Issue
Block a user