mirror of
https://github.com/lancedb/lancedb.git
synced 2026-08-18 20:18:37 +00:00
fix(python): preserve phrase semantics in sync queries (#3654)
## Summary - serialize sync phrase queries consistently for execution and query plans - restore the documented no-argument hybrid `phrase_query()` behavior - keep reranker input as the original user text without mutating the builder Fixes #3653. ## Testing - `python/.venv/bin/python -m pytest <8 focused test nodes> -q` (`8 passed`) - `python/.venv/bin/python -m ruff format --check python/python/lancedb/query.py python/python/tests/test_fts.py python/python/tests/test_hybrid_query.py` - `python/.venv/bin/python -m ruff check .` - `git diff --check origin/main...HEAD` The complete hybrid module and the real native FTS phrase test were not completed in the current PyO3 runtime environment: both stalled in the native `lancedb.connect()` fixture and were interrupted without an assertion failure.
This commit is contained in:
@@ -1951,8 +1951,7 @@ class LanceFtsQueryBuilder(LanceQueryBuilder):
|
||||
Parameters
|
||||
----------
|
||||
phrase_query: bool, default True
|
||||
If True, then the query will be wrapped in quotes and
|
||||
double quotes replaced by single quotes.
|
||||
If True, then an unquoted string query will be wrapped in quotes.
|
||||
|
||||
Returns
|
||||
-------
|
||||
@@ -1962,6 +1961,21 @@ class LanceFtsQueryBuilder(LanceQueryBuilder):
|
||||
self._phrase_query = phrase_query
|
||||
return self
|
||||
|
||||
def _query_with_phrase_semantics(self) -> str | FullTextQuery:
|
||||
query = self._query
|
||||
if not self._phrase_query:
|
||||
return query
|
||||
if isinstance(query, str):
|
||||
if not query.startswith('"') or not query.endswith('"'):
|
||||
return f'"{query}"'
|
||||
return query
|
||||
if isinstance(query, PhraseQuery):
|
||||
return query
|
||||
raise TypeError(
|
||||
"phrase_query() requires a string or PhraseQuery, "
|
||||
f"got {type(query).__name__}"
|
||||
)
|
||||
|
||||
def fast_search(self) -> LanceFtsQueryBuilder:
|
||||
"""
|
||||
Skip a flat search of unindexed data. This will improve
|
||||
@@ -1986,7 +2000,7 @@ class LanceFtsQueryBuilder(LanceQueryBuilder):
|
||||
fragments=self._fragments,
|
||||
fragment_ids=self._fragment_ids,
|
||||
full_text_query=FullTextSearchQuery(
|
||||
query=self._query, columns=self._fts_columns
|
||||
query=self._query_with_phrase_semantics(), columns=self._fts_columns
|
||||
),
|
||||
offset=self._offset,
|
||||
fast_search=self._fast_search,
|
||||
@@ -2004,15 +2018,6 @@ class LanceFtsQueryBuilder(LanceQueryBuilder):
|
||||
def to_arrow(self, *, timeout: Optional[timedelta] = None) -> pa.Table:
|
||||
self._table._ensure_no_legacy_fts_index()
|
||||
|
||||
query = self._query
|
||||
if self._phrase_query:
|
||||
if isinstance(query, str):
|
||||
if not query.startswith('"') or not query.endswith('"'):
|
||||
self._query = f'"{query}"'
|
||||
elif isinstance(query, FullTextQuery) and not isinstance(
|
||||
query, PhraseQuery
|
||||
):
|
||||
raise TypeError("Please use PhraseQuery for phrase queries.")
|
||||
query = self._query_for_scan()
|
||||
results = self._table._execute_query(query, timeout=timeout)
|
||||
results = results.read_all()
|
||||
@@ -2143,14 +2148,13 @@ class LanceHybridQueryBuilder(LanceQueryBuilder):
|
||||
|
||||
return vector_query, text_query
|
||||
|
||||
def phrase_query(self, phrase_query: bool = None) -> LanceHybridQueryBuilder:
|
||||
def phrase_query(self, phrase_query: bool = True) -> LanceHybridQueryBuilder:
|
||||
"""Set whether to use phrase query.
|
||||
|
||||
Parameters
|
||||
----------
|
||||
phrase_query: bool, default True
|
||||
If True, then the query will be wrapped in quotes and
|
||||
double quotes replaced by single quotes.
|
||||
If True, then an unquoted string query will be wrapped in quotes.
|
||||
|
||||
Returns
|
||||
-------
|
||||
|
||||
@@ -1084,6 +1084,84 @@ def test_fts_query_to_json():
|
||||
assert json_str == expected
|
||||
|
||||
|
||||
def test_fts_phrase_query_is_preserved_in_query_object():
|
||||
query = LanceFtsQueryBuilder(mock.Mock(), "puppy runs").phrase_query()
|
||||
|
||||
query_object = query.to_query_object()
|
||||
|
||||
assert query_object.full_text_query.query == '"puppy runs"'
|
||||
|
||||
|
||||
def test_fts_phrase_query_execution_preserves_user_text():
|
||||
table = mock.Mock()
|
||||
table.schema = pa.schema([])
|
||||
table._execute_query.return_value = pa.table({"text": ["result"]}).to_reader()
|
||||
|
||||
class CapturingReranker:
|
||||
score = "relevance"
|
||||
|
||||
def __init__(self):
|
||||
self.queries = []
|
||||
|
||||
def rerank_fts(self, query, results):
|
||||
self.queries.append(query)
|
||||
return results.append_column("_relevance_score", [[1.0]])
|
||||
|
||||
reranker = CapturingReranker()
|
||||
query = (
|
||||
LanceFtsQueryBuilder(table, "puppy runs")
|
||||
.phrase_query()
|
||||
.with_row_id(False)
|
||||
.rerank(reranker)
|
||||
)
|
||||
|
||||
query.to_arrow()
|
||||
|
||||
backend_query = table._execute_query.call_args.args[0]
|
||||
assert (
|
||||
backend_query.full_text_query.query,
|
||||
reranker.queries,
|
||||
query._query,
|
||||
) == ('"puppy runs"', ["puppy runs"], "puppy runs")
|
||||
|
||||
|
||||
def test_fts_phrase_query_false_preserves_string():
|
||||
query = LanceFtsQueryBuilder(mock.Mock(), "puppy runs").phrase_query(False)
|
||||
|
||||
query_object = query.to_query_object()
|
||||
|
||||
assert query_object.full_text_query.query == "puppy runs"
|
||||
|
||||
|
||||
def test_fts_phrase_query_preserves_fully_quoted_string():
|
||||
query = LanceFtsQueryBuilder(mock.Mock(), '"puppy runs"').phrase_query()
|
||||
|
||||
query_object = query.to_query_object()
|
||||
|
||||
assert query_object.full_text_query.query == '"puppy runs"'
|
||||
|
||||
|
||||
def test_fts_phrase_query_preserves_structured_phrase_query():
|
||||
phrase_query = PhraseQuery("puppy runs", "text")
|
||||
query = LanceFtsQueryBuilder(mock.Mock(), phrase_query).phrase_query()
|
||||
|
||||
query_object = query.to_query_object()
|
||||
|
||||
assert query_object.full_text_query.query == phrase_query
|
||||
|
||||
|
||||
def test_fts_phrase_query_rejects_other_structured_queries():
|
||||
query = LanceFtsQueryBuilder(
|
||||
mock.Mock(), MatchQuery("puppy", "text")
|
||||
).phrase_query()
|
||||
|
||||
with pytest.raises(
|
||||
TypeError,
|
||||
match=r"phrase_query\(\) requires a string or PhraseQuery, got MatchQuery",
|
||||
):
|
||||
query.to_query_object()
|
||||
|
||||
|
||||
def test_fts_fast_search(table):
|
||||
table.create_fts_index("text")
|
||||
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
# SPDX-License-Identifier: Apache-2.0
|
||||
# SPDX-FileCopyrightText: Copyright The LanceDB Authors
|
||||
|
||||
from unittest import mock
|
||||
|
||||
import lancedb
|
||||
|
||||
from lancedb.query import LanceHybridQueryBuilder
|
||||
@@ -191,6 +193,23 @@ async def test_analyze_plan(table: AsyncTable):
|
||||
assert "metrics=" in res
|
||||
|
||||
|
||||
def test_hybrid_phrase_query_is_preserved_in_analyze_plan():
|
||||
table = mock.Mock()
|
||||
analyzed_queries = []
|
||||
table._analyze_plan.side_effect = lambda query: analyzed_queries.append(query) or ""
|
||||
|
||||
(
|
||||
LanceHybridQueryBuilder(table)
|
||||
.vector([0.1, 0.2])
|
||||
.text("puppy runs")
|
||||
.phrase_query()
|
||||
.analyze_plan()
|
||||
)
|
||||
|
||||
assert len(analyzed_queries) == 2
|
||||
assert analyzed_queries[1].full_text_query.query == '"puppy runs"'
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def table_with_id(tmpdir_factory) -> Table:
|
||||
tmp_path = str(tmpdir_factory.mktemp("data"))
|
||||
|
||||
Reference in New Issue
Block a user