Skip to content

fix(python): validate query field names#612

Open
HosniBelfeki wants to merge 2 commits into
alibaba:mainfrom
HosniBelfeki:fix-python-query-field-name-validation
Open

fix(python): validate query field names#612
HosniBelfeki wants to merge 2 commits into
alibaba:mainfrom
HosniBelfeki:fix-python-query-field-name-validation

Conversation

@HosniBelfeki

Copy link
Copy Markdown
Contributor

Summary

Tighten Python Query validation so field_name must be a non-empty string.

Previously, Query._validate() only rejected None, so an empty string or non-string value could pass Python-side validation and fail later with a less direct executor/schema error.

This adds a focused regression test for None, empty string, and non-string field names.

Tests

  • python -m ruff check python\zvec\model\param\query.py python\tests\test_params.py
  • python -m py_compile python\zvec\model\param\query.py python\tests\test_params.py

Copilot AI review requested due to automatic review settings July 22, 2026 01:03
@HosniBelfeki
HosniBelfeki requested a review from Cuiyus as a code owner July 22, 2026 01:03

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

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Tightens Python-side Query validation to ensure field_name is a non-empty string, preventing later executor/schema failures from invalid field names.

Changes:

  • Update Query._validate() to reject non-str and empty field_name values with a clearer error message.
  • Add a parametrized regression test covering None, empty string, and non-string field names.

Reviewed changes

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

File Description
python/zvec/model/param/query.py Strengthens field_name validation and improves error message clarity.
python/tests/test_params.py Adds parametrized tests to ensure invalid field_name values 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 +118 to +119
if not isinstance(self.field_name, str) or not self.field_name:
raise ValueError("Field name must be a non-empty string")
Copilot AI review requested due to automatic review settings July 23, 2026 22:14
@HosniBelfeki
HosniBelfeki force-pushed the fix-python-query-field-name-validation branch from f8ec25c to c088569 Compare July 23, 2026 22:14
@HosniBelfeki

Copy link
Copy Markdown
Contributor Author

Addressed Copilot review feedback: whitespace-only field_name values are now rejected with field_name.strip(), and the regression test covers whitespace-only input.

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 2 out of 2 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings July 25, 2026 18:48

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 2 out of 2 changed files in this pull request and generated no new comments.

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