From 40238d240ac877479b1ab4c772a5ed1410fa7db1 Mon Sep 17 00:00:00 2001 From: kid <19265318+u70b3@users.noreply.github.com> Date: Tue, 14 Jul 2026 14:44:35 +0800 Subject: [PATCH] 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. --- python/python/lancedb/query.py | 34 ++++++----- python/python/tests/test_fts.py | 78 ++++++++++++++++++++++++ python/python/tests/test_hybrid_query.py | 19 ++++++ 3 files changed, 116 insertions(+), 15 deletions(-) diff --git a/python/python/lancedb/query.py b/python/python/lancedb/query.py index 992463f0c..1a4588808 100644 --- a/python/python/lancedb/query.py +++ b/python/python/lancedb/query.py @@ -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 ------- diff --git a/python/python/tests/test_fts.py b/python/python/tests/test_fts.py index db83cb678..134b27980 100644 --- a/python/python/tests/test_fts.py +++ b/python/python/tests/test_fts.py @@ -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") diff --git a/python/python/tests/test_hybrid_query.py b/python/python/tests/test_hybrid_query.py index f2bbf3180..4ba61a4b8 100644 --- a/python/python/tests/test_hybrid_query.py +++ b/python/python/tests/test_hybrid_query.py @@ -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"))