Feature/allow pass configuration for to ts query#4
Feature/allow pass configuration for to ts query#4
Conversation
There was a problem hiding this comment.
Pull Request Overview
Introduces an optional config_name parameter for PostgreSQL full-text search functions and ensures the chosen configuration is propagated in search filters.
- Adds
DEFAULT_TEXT_SEARCH_CONFIGand extendsToTsVector,ToTsQuery, andPlainToTsQueryto acceptconfig_name. - Updates
SearchCriterionto extract and forward theconfig_namefrom the query term. - Applies a blanket
# type: ignoreto the updatedSearchCriterion.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tortoise/contrib/postgres/functions.py | Added default config constant and updated search function classes to accept config_name. |
| tortoise/contrib/postgres/search.py | Modified SearchCriterion to pass the extracted config_name into ToTsVector. |
Comments suppressed due to low confidence (4)
tortoise/contrib/postgres/functions.py:11
- [nitpick] The docstring for
ToTsVectorshould be updated to describe the newconfig_nameparameter, its default value, and how it affects the generated SQL.
def __init__(self, field: Term, config_name: str = DEFAULT_TEXT_SEARCH_CONFIG) -> None:
tortoise/contrib/postgres/functions.py:20
- [nitpick] The docstring for
ToTsQueryshould be updated to explain theconfig_nameparameter and its default behavior in the SQL query.
def __init__(self, field: Term, config_name: str = DEFAULT_TEXT_SEARCH_CONFIG) -> None:
tortoise/contrib/postgres/functions.py:29
- [nitpick] The docstring for
PlainToTsQueryneeds an update to include information on the newly addedconfig_nameparameter and its effect.
def __init__(self, field: Term, config_name: str = DEFAULT_TEXT_SEARCH_CONFIG) -> None:
tortoise/contrib/postgres/search.py:15
- Consider adding unit tests to verify that custom
config_namevalues are correctly propagated throughSearchCriterionand reflected in the generated SQL.
if not isinstance(expr, Function):
| class SearchCriterion(BasicCriterion): # type: ignore | ||
| def __init__(self, field: Term, expr: Union[Term, Function]) -> None: | ||
| if isinstance(expr, Function): | ||
| _expr = expr | ||
| else: | ||
| _expr = ToTsQuery(expr) | ||
| super().__init__(Comp.search, ToTsVector(field), _expr) | ||
| if not isinstance(expr, Function): | ||
| expr = ToTsQuery(expr) | ||
| super().__init__(Comp.search, ToTsVector(config_name=expr.args[0].value, field=field), expr) |
There was a problem hiding this comment.
[nitpick] The # type: ignore suppresses all type errors and may hide real issues; consider resolving the underlying type mismatch or narrowing the ignore to a specific expression instead of a blanket ignore.
| super().__init__(Comp.search, ToTsVector(field), _expr) | ||
| if not isinstance(expr, Function): | ||
| expr = ToTsQuery(expr) | ||
| super().__init__(Comp.search, ToTsVector(config_name=expr.args[0].value, field=field), expr) |
There was a problem hiding this comment.
Accessing expr.args[0].value assumes the first argument is always a literal config name; this is brittle. Consider adding a dedicated config_name attribute or accessor on your Function subclasses to make this extraction safer.
There was a problem hiding this comment.
quizá tendría sentido validarlo antes de intentar desempaquetarlo
quizá también tendría sentido mejorar la readability manteniendo el orden original y la opcionalidad de la config
XaviTorello
left a comment
XaviTorello
left a comment
There was a problem hiding this comment.
Si lo acabamos de fortificar seguramente será fácilmente promocionable al repo original
Tiene sentido añadir algún test mínimo para blindarlo?
Versionamos de alguna manera? Agrupamos features?
| super().__init__(Comp.search, ToTsVector(field), _expr) | ||
| if not isinstance(expr, Function): | ||
| expr = ToTsQuery(expr) | ||
| super().__init__(Comp.search, ToTsVector(config_name=expr.args[0].value, field=field), expr) |
There was a problem hiding this comment.
quizá tendría sentido validarlo antes de intentar desempaquetarlo
quizá también tendría sentido mejorar la readability manteniendo el orden original y la opcionalidad de la config
fullonic
force-pushed
the
feature/allow-pass-configuration-for-ToTsQuery
branch
from
76cbfe8 to
02a7f93
Compare
fullonic
force-pushed
the
feature/allow-pass-configuration-for-ToTsQuery
branch
from
b2e03a1 to
52d6aaa
Compare
2 participants
Supersede #2. This is version contains the latest version of Tortoise-ORM, v0.25
Description
Updates search-related functions and the
SearchCriterionobject to receive a new configuration parameter:config_name. By default, we use the PostgreSQL default text search config,pg_catalog.simple.Motivation and Context
When using PostgreSQL search, the user might need to change the default configuration. This PR makes it possible by allowing the user to specify which config he/she wants to use. For example, using the PostgreSQL extension
unaccent, we can now query as:Tortoise will generate the below SQL query
'SELECT "id", "name" FROM "cities" WHERE TO_TSVECTOR(\'public.unaccent\',"name") @@ TO_TSQUERY(\'public.unaccent\',\'paris\')'Because of the current implementation of the
__searchfilter, inside theSearchCriterion.__init__, we extract theconfig_namefrom the function in use, in the above exampleToTsQuery, to pass it to theToTsVectorfunction.How Has This Been Tested?
Checklist: