Skip to content

fix(python): reject empty fts queries - #640

Open
HosniBelfeki wants to merge 1 commit into
alibaba:mainfrom
HosniBelfeki:fix-python-fts-empty-validation
Open

fix(python): reject empty fts queries#640
HosniBelfeki wants to merge 1 commit into
alibaba:mainfrom
HosniBelfeki:fix-python-fts-empty-validation

Conversation

@HosniBelfeki

Copy link
Copy Markdown
Contributor

Summary

This hardens Python FTS query validation by rejecting Fts objects that do not provide a non-empty query_string or match_string. Previously, empty or whitespace-only FTS values could pass Python-side validation and reach the native query path.

The executor now forwards stripped FTS text to the native _Fts object, keeping validation and execution behavior consistent.

Tests

  • python -m ruff check python/zvec/model/param/query.py python/zvec/executor/query_executor.py python/tests/test_fts_query.py python/tests/test_query_executor.py
  • python -m py_compile python/zvec/model/param/query.py python/zvec/executor/query_executor.py python/tests/test_fts_query.py python/tests/test_query_executor.py

Note: focused pytest against local source is blocked in this workspace because zvec._zvec is not built locally.

@HosniBelfeki
HosniBelfeki requested a review from Cuiyus as a code owner August 2, 2026 14:55
Copilot AI review requested due to automatic review settings August 2, 2026 14:55

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens Python-side validation and execution for FTS (full-text search) queries by rejecting empty/whitespace-only Fts inputs and ensuring stripped FTS text is forwarded to the native _Fts binding.

Changes:

  • Treat whitespace-only fts.query_string / fts.match_string as empty via centralized stripping helpers and update has_fts() accordingly.
  • Add validation to reject Fts objects that provide neither a non-empty query_string nor match_string.
  • Update the executor to forward stripped FTS strings to native _Fts, and add tests covering empty inputs and stripping behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
python/zvec/model/param/query.py Adds stripped/normalized FTS accessors and strengthens _validate() to reject empty FTS.
python/zvec/executor/query_executor.py Forwards stripped FTS strings into the native _Fts query object.
python/tests/test_query_executor.py Adds a regression test ensuring FTS text is stripped before reaching native query construction.
python/tests/test_fts_query.py Adds parameterized tests asserting empty/whitespace-only FTS inputs are rejected.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread python/zvec/model/param/query.py Outdated
Comment on lines +117 to +121
def _fts_query_string(self) -> str:
return self.fts.query_string.strip() if self.fts and self.fts.query_string else ""

def _fts_match_string(self) -> str:
return self.fts.match_string.strip() if self.fts and self.fts.match_string else ""
Copilot AI review requested due to automatic review settings August 2, 2026 14:58
@HosniBelfeki
HosniBelfeki force-pushed the fix-python-fts-empty-validation branch from 44847cc to ccf8325 Compare August 2, 2026 14:58

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (3)

python/zvec/model/param/query.py:125

  • _fts_match_string() has the same issue as _fts_query_string(): calling .strip() on a non-string will raise at runtime, and could also pass an unexpected type into the native query layer. Add a runtime type check and raise ValueError with a clear message.
    def _fts_match_string(self) -> str:
        return (
            self.fts.match_string.strip() if self.fts and self.fts.match_string else ""
        )

python/zvec/model/param/query.py:120

  • _fts_query_string() assumes self.fts.query_string is a str and calls .strip() unconditionally when truthy. Since dataclasses don't enforce types at runtime, passing a non-string (e.g., int/bytes) will raise AttributeError (or propagate an unexpected type into the native _Fts binding). Add an explicit runtime type check and raise a clear ValueError instead.

This issue also appears on line 122 of the same file.

    def _fts_query_string(self) -> str:
        return (
            self.fts.query_string.strip() if self.fts and self.fts.query_string else ""
        )

python/zvec/executor/query_executor.py:223

  • QueryExecutor is calling Query methods that are underscore-prefixed (_fts_query_string(), _fts_match_string()), which implies they are private implementation details. Since this behavior is now relied on across modules, consider making these helpers part of the public Query API (e.g., fts_query_string() / fts_match_string() or fts_query_string_stripped properties) and update callers accordingly to avoid coupling to internals.
    def _apply_fts(self, query: Query, search_query: _SearchQuery) -> None:
        """Set FTS query on search_query if the query has FTS parameters."""
        if query.has_fts():
            fts = _Fts()
            fts.query_string = query._fts_query_string()
            fts.match_string = query._fts_match_string()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants