From eeabb60200ea75ae9d9f8111b242320f96c71ff0 Mon Sep 17 00:00:00 2001 From: collerek Date: Sat, 6 Mar 2021 13:07:22 +0100 Subject: [PATCH 01/16] add is null filter, add complex fiters (and_ & or_) and basic tests for them --- docs/queries/filter-and-sort.md | 2 + docs/releases.md | 10 ++ ormar/__init__.py | 6 +- ormar/fields/base.py | 1 - ormar/fields/foreign_key.py | 1 - ormar/queryset/__init__.py | 3 + ormar/queryset/actions/filter_action.py | 6 +- ormar/queryset/clause.py | 133 +++++++++++++++++++-- ormar/queryset/prefetch_query.py | 2 +- ormar/queryset/queryset.py | 21 +++- ormar/relations/querysetproxy.py | 2 + tests/test_filter_groups.py | 149 ++++++++++++++++++++++++ tests/test_isnull_filter.py | 76 ++++++++++++ tests/test_or_filters.py | 118 +++++++++++++++++++ 14 files changed, 509 insertions(+), 21 deletions(-) create mode 100644 tests/test_filter_groups.py create mode 100644 tests/test_isnull_filter.py create mode 100644 tests/test_or_filters.py diff --git a/docs/queries/filter-and-sort.md b/docs/queries/filter-and-sort.md index 2e451e3..8de1550 100644 --- a/docs/queries/filter-and-sort.md +++ b/docs/queries/filter-and-sort.md @@ -70,6 +70,8 @@ You can use special filter suffix to change the filter operands: * contains - like `album__name__contains='Mal'` (sql like) * icontains - like `album__name__icontains='mal'` (sql like case insensitive) * in - like `album__name__in=['Malibu', 'Barclay']` (sql in) +* isnull - like `album__name__isnull=True` (sql is null) + (isnotnull `album__name__isnull=False` (sql is not null)) * gt - like `position__gt=3` (sql >) * gte - like `position__gte=3` (sql >=) * lt - like `position__lt=3` (sql <) diff --git a/docs/releases.md b/docs/releases.md index 5e61041..dca68fc 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -1,3 +1,13 @@ +# 0.9.7 + +## Features +* Add `isnull` operator to filter and exclude methods. + ```python + album__name__isnull=True #(sql: album.name is null) + album__name__isnull=False #(sql: album.name is not null)) + ``` + + # 0.9.6 ##Important diff --git a/ormar/__init__.py b/ormar/__init__.py index 5ed97f1..98f7e78 100644 --- a/ormar/__init__.py +++ b/ormar/__init__.py @@ -56,7 +56,7 @@ from ormar.fields import ( ) # noqa: I100 from ormar.models import ExcludableItems, Model from ormar.models.metaclass import ModelMeta -from ormar.queryset import OrderAction, QuerySet +from ormar.queryset import OrderAction, QuerySet, and_, or_ from ormar.relations import RelationType from ormar.signals import Signal @@ -68,7 +68,7 @@ class UndefinedType: # pragma no cover Undefined = UndefinedType() -__version__ = "0.9.6" +__version__ = "0.9.7" __all__ = [ "Integer", "BigInteger", @@ -108,4 +108,6 @@ __all__ = [ "ForeignKeyField", "OrderAction", "ExcludableItems", + "and_", + "or_", ] diff --git a/ormar/fields/base.py b/ormar/fields/base.py index 1b5f7a6..1fada90 100644 --- a/ormar/fields/base.py +++ b/ormar/fields/base.py @@ -1,4 +1,3 @@ -import inspect from typing import Any, List, Optional, TYPE_CHECKING, Type, Union import sqlalchemy diff --git a/ormar/fields/foreign_key.py b/ormar/fields/foreign_key.py index da20325..3c8d18b 100644 --- a/ormar/fields/foreign_key.py +++ b/ormar/fields/foreign_key.py @@ -75,7 +75,6 @@ def create_dummy_model( fields = {f"{pk_field.name}": (pk_field.__type__, None)} dummy_model = create_model( # type: ignore - f"PkOnly{base_model.get_name(lower=False)}{alias}", __module__=base_model.__module__, **fields, # type: ignore diff --git a/ormar/queryset/__init__.py b/ormar/queryset/__init__.py index 161d9bb..678e977 100644 --- a/ormar/queryset/__init__.py +++ b/ormar/queryset/__init__.py @@ -2,6 +2,7 @@ Contains QuerySet and different Query classes to allow for constructing of sql queries. """ from ormar.queryset.actions import FilterAction, OrderAction +from ormar.queryset.clause import and_, or_ from ormar.queryset.filter_query import FilterQuery from ormar.queryset.limit_query import LimitQuery from ormar.queryset.offset_query import OffsetQuery @@ -16,4 +17,6 @@ __all__ = [ "OrderQuery", "FilterAction", "OrderAction", + "and_", + "or_", ] diff --git a/ormar/queryset/actions/filter_action.py b/ormar/queryset/actions/filter_action.py index ed6277d..7b3bb9e 100644 --- a/ormar/queryset/actions/filter_action.py +++ b/ormar/queryset/actions/filter_action.py @@ -19,6 +19,7 @@ FILTER_OPERATORS = { "istartswith": "ilike", "endswith": "like", "iendswith": "ilike", + "isnull": "is_", "in": "in_", "gt": "__gt__", "gte": "__ge__", @@ -38,7 +39,7 @@ class FilterAction(QueryAction): Extracted in order to easily change table prefixes on complex relations. """ - def __init__(self, filter_str: str, value: Any, model_cls: Type["Model"]) -> None: + def __init__(self, filter_str: str, value: Any, model_cls: Type["Model"],) -> None: super().__init__(query_str=filter_str, model_cls=model_cls) self.filter_value = value self._escape_characters_in_clause() @@ -124,6 +125,9 @@ class FilterAction(QueryAction): self.filter_value = self.filter_value.pk op_attr = FILTER_OPERATORS[self.operator] + if self.operator == "isnull": + op_attr = "is_" if self.filter_value else "isnot" + self.filter_value = None clause = getattr(self.column, op_attr)(self.filter_value) clause = self._compile_clause( clause, modifiers={"escape": "\\" if self.has_escaped_character else None}, diff --git a/ormar/queryset/clause.py b/ormar/queryset/clause.py index b98616d..4a1ef97 100644 --- a/ormar/queryset/clause.py +++ b/ormar/queryset/clause.py @@ -1,6 +1,9 @@ import itertools from dataclasses import dataclass -from typing import Any, List, TYPE_CHECKING, Tuple, Type +from enum import Enum +from typing import Any, Generator, List, TYPE_CHECKING, Tuple, Type + +import sqlalchemy import ormar # noqa I100 from ormar.queryset.actions.filter_action import FilterAction @@ -10,6 +13,99 @@ if TYPE_CHECKING: # pragma no cover from ormar import Model +class FilterType(Enum): + AND = 1 + OR = 2 + + +class FilterGroup: + def __init__( + self, *args: Any, _filter_type: FilterType = FilterType.AND, **kwargs: Any, + ) -> None: + self.filter_type = _filter_type + self.exclude = False + self._nested_groups: List["FilterGroup"] = list(args) + self._resolved = False + self.is_source_model_filter = False + self._kwargs_dict = kwargs + self.actions: List[FilterAction] = [] + + def resolve( + self, + model_cls: Type["Model"], + select_related: List = None, + filter_clauses: List = None, + ) -> Tuple[List[FilterAction], List[str]]: + select_related = select_related if select_related is not None else [] + filter_clauses = filter_clauses if filter_clauses is not None else [] + qryclause = QueryClause( + model_cls=model_cls, + select_related=select_related, + filter_clauses=filter_clauses, + ) + own_filter_clauses, select_related = qryclause.prepare_filter( + _own_only=True, **self._kwargs_dict + ) + self.actions = own_filter_clauses + filter_clauses = filter_clauses + own_filter_clauses + self._resolved = True + if self._nested_groups: + for group in self._nested_groups: + if not group._resolved: + (filter_clauses, select_related) = group.resolve( + model_cls=model_cls, + select_related=select_related, + filter_clauses=filter_clauses, + ) + self._is_self_model_group() + return filter_clauses, select_related + + def _iter(self) -> Generator: + if not self._nested_groups: + yield from self.actions + return + for group in self._nested_groups: + yield from group._iter() + yield from self.actions + + def _is_self_model_group(self) -> None: + if self.actions and self._nested_groups: + if all([action.is_source_model_filter for action in self.actions]) and all( + group.is_source_model_filter for group in self._nested_groups + ): + self.is_source_model_filter = True + elif self.actions: + if all([action.is_source_model_filter for action in self.actions]): + self.is_source_model_filter = True + else: + if all(group.is_source_model_filter for group in self._nested_groups): + self.is_source_model_filter = True + + def _get_text_clauses(self) -> List[sqlalchemy.sql.expression.TextClause]: + return [x.get_text_clause() for x in self._nested_groups] + [ + x.get_text_clause() for x in self.actions + ] + + def get_text_clause(self) -> sqlalchemy.sql.expression.TextClause: + if self.filter_type == FilterType.AND: + clause = sqlalchemy.text( + "( " + str(sqlalchemy.sql.and_(*self._get_text_clauses())) + " )" + ) + else: + clause = sqlalchemy.text( + "( " + str(sqlalchemy.sql.or_(*self._get_text_clauses())) + " )" + ) + return clause + + +def or_(*args: Any, **kwargs: Any) -> FilterGroup: + return FilterGroup(_filter_type=FilterType.OR, *args, **kwargs) + + +def and_(*args: Any, **kwargs: Any) -> FilterGroup: + return FilterGroup(_filter_type=FilterType.AND, *args, **kwargs) + + @dataclass class Prefix: source_model: Type["Model"] @@ -40,13 +136,15 @@ class QueryClause: self.table = self.model_cls.Meta.table def prepare_filter( # noqa: A003 - self, **kwargs: Any + self, _own_only: bool = False, **kwargs: Any ) -> Tuple[List[FilterAction], List[str]]: """ Main external access point that processes the clauses into sqlalchemy text clauses and updates select_related list with implicit related tables mentioned in select_related strings but not included in select_related. + :param _own_only: + :type _own_only: :param kwargs: key, value pair with column names and values :type kwargs: Any :return: Tuple with list of where clauses and updated select_related list @@ -56,12 +154,14 @@ class QueryClause: pk_name = self.model_cls.get_column_alias(self.model_cls.Meta.pkname) kwargs[pk_name] = kwargs.pop("pk") - filter_clauses, select_related = self._populate_filter_clauses(**kwargs) + filter_clauses, select_related = self._populate_filter_clauses( + _own_only=_own_only, **kwargs + ) return filter_clauses, select_related def _populate_filter_clauses( - self, **kwargs: Any + self, _own_only: bool, **kwargs: Any ) -> Tuple[List[FilterAction], List[str]]: """ Iterates all clauses and extracts used operator and field from related @@ -74,6 +174,7 @@ class QueryClause: :rtype: Tuple[List[sqlalchemy.sql.elements.TextClause], List[str]] """ filter_clauses = self.filter_clauses + own_filter_clauses = [] select_related = list(self._select_related) for key, value in kwargs.items(): @@ -84,12 +185,14 @@ class QueryClause: select_related=select_related ) - filter_clauses.append(filter_action) + own_filter_clauses.append(filter_action) self._register_complex_duplicates(select_related) filter_clauses = self._switch_filter_action_prefixes( - filter_clauses=filter_clauses + filter_clauses=filter_clauses + own_filter_clauses ) + if _own_only: + return own_filter_clauses, select_related return filter_clauses, select_related def _register_complex_duplicates(self, select_related: List[str]) -> None: @@ -150,11 +253,17 @@ class QueryClause: :return: list of actions with aliases changed if needed :rtype: List[FilterAction] """ - manager = self.model_cls.Meta.alias_manager + for action in filter_clauses: - new_alias = manager.resolve_relation_alias( - self.model_cls, action.related_str - ) - if "__" in action.related_str and new_alias: - action.table_prefix = new_alias + if isinstance(action, FilterGroup): + for action2 in action._iter(): + self._verify_prefix_and_switch(action2) + else: + self._verify_prefix_and_switch(action) return filter_clauses + + def _verify_prefix_and_switch(self, action: "FilterAction") -> None: + manager = self.model_cls.Meta.alias_manager + new_alias = manager.resolve_relation_alias(self.model_cls, action.related_str) + if "__" in action.related_str and new_alias: + action.table_prefix = new_alias diff --git a/ormar/queryset/prefetch_query.py b/ormar/queryset/prefetch_query.py index d224c22..7d53535 100644 --- a/ormar/queryset/prefetch_query.py +++ b/ormar/queryset/prefetch_query.py @@ -266,7 +266,7 @@ class PrefetchQuery: model_cls=clause_target, select_related=[], filter_clauses=[], ) kwargs = {f"{filter_column}__in": ids} - filter_clauses, _ = qryclause.prepare_filter(**kwargs) + filter_clauses, _ = qryclause.prepare_filter(_own_only=False, **kwargs) return filter_clauses return [] diff --git a/ormar/queryset/queryset.py b/ormar/queryset/queryset.py index d0679f5..944550e 100644 --- a/ormar/queryset/queryset.py +++ b/ormar/queryset/queryset.py @@ -256,7 +256,9 @@ class QuerySet: # print("\n", exp.compile(compile_kwargs={"literal_binds": True})) return exp - def filter(self, _exclude: bool = False, **kwargs: Any) -> "QuerySet": # noqa: A003 + def filter( # noqa: A003 + self, *args: Any, _exclude: bool = False, **kwargs: Any + ) -> "QuerySet": """ Allows you to filter by any `Model` attribute/field as well as to fetch instances, with a filter across an FK relationship. @@ -268,6 +270,8 @@ class QuerySet: * contains - like `album__name__contains='Mal'` (sql like) * icontains - like `album__name__icontains='mal'` (sql like case insensitive) * in - like `album__name__in=['Malibu', 'Barclay']` (sql in) + * isnull - like `album__name__isnull=True` (sql is null) + (isnotnull `album__name__isnull=False` (sql is not null)) * gt - like `position__gt=3` (sql >) * gte - like `position__gte=3` (sql >=) * lt - like `position__lt=3` (sql <) @@ -284,12 +288,23 @@ class QuerySet: :return: filtered QuerySet :rtype: QuerySet """ + filter_groups = [] + if args: + for arg in args: + arg.resolve( + model_cls=self.model, + select_related=self._select_related, + filter_clauses=self.filter_clauses, + ) + filter_groups.append(arg) + qryclause = QueryClause( model_cls=self.model, select_related=self._select_related, filter_clauses=self.filter_clauses, ) filter_clauses, select_related = qryclause.prepare_filter(**kwargs) + filter_clauses = filter_clauses + filter_groups if _exclude: exclude_clauses = filter_clauses filter_clauses = self.filter_clauses @@ -303,7 +318,7 @@ class QuerySet: select_related=select_related, ) - def exclude(self, **kwargs: Any) -> "QuerySet": # noqa: A003 + def exclude(self, *args: Any, **kwargs: Any) -> "QuerySet": # noqa: A003 """ Works exactly the same as filter and all modifiers (suffixes) are the same, but returns a *not* condition. @@ -322,7 +337,7 @@ class QuerySet: :return: filtered QuerySet :rtype: QuerySet """ - return self.filter(_exclude=True, **kwargs) + return self.filter(_exclude=True, *args, **kwargs) def select_related(self, related: Union[List, str]) -> "QuerySet": """ diff --git a/ormar/relations/querysetproxy.py b/ormar/relations/querysetproxy.py index 190dd7b..69a0f54 100644 --- a/ormar/relations/querysetproxy.py +++ b/ormar/relations/querysetproxy.py @@ -386,6 +386,8 @@ class QuerysetProxy: * contains - like `album__name__contains='Mal'` (sql like) * icontains - like `album__name__icontains='mal'` (sql like case insensitive) * in - like `album__name__in=['Malibu', 'Barclay']` (sql in) + * isnull - like `album__name__isnull=True` (sql is null) + (isnotnull `album__name__isnull=False` (sql is not null)) * gt - like `position__gt=3` (sql >) * gte - like `position__gte=3` (sql >=) * lt - like `position__lt=3` (sql <) diff --git a/tests/test_filter_groups.py b/tests/test_filter_groups.py new file mode 100644 index 0000000..ec6c4a0 --- /dev/null +++ b/tests/test_filter_groups.py @@ -0,0 +1,149 @@ +from typing import Optional + +import databases +import sqlalchemy + +import ormar +from tests.settings import DATABASE_URL + +database = databases.Database(DATABASE_URL) +metadata = sqlalchemy.MetaData() + + +class BaseMeta(ormar.ModelMeta): + metadata = metadata + database = database + + +class Author(ormar.Model): + class Meta(BaseMeta): + tablename = "authors" + + id: int = ormar.Integer(primary_key=True) + name: str = ormar.String(max_length=100) + + +class Book(ormar.Model): + class Meta(BaseMeta): + tablename = "books" + + id: int = ormar.Integer(primary_key=True) + author: Optional[Author] = ormar.ForeignKey(Author) + title: str = ormar.String(max_length=100) + year: int = ormar.Integer(nullable=True) + + +def test_or_group(): + result = ormar.or_(name="aa", books__title="bb") + result.resolve(model_cls=Author) + assert len(result.actions) == 2 + assert result.actions[0].target_model == Author + assert result.actions[1].target_model == Book + assert ( + str(result.get_text_clause()) == f"( authors.name = 'aa' OR " + f"{result.actions[1].table_prefix}" + f"_books.title = 'bb' )" + ) + assert not result.is_source_model_filter + + +def test_and_group(): + result = ormar.and_(name="aa", books__title="bb") + result.resolve(model_cls=Author) + assert len(result.actions) == 2 + assert result.actions[0].target_model == Author + assert result.actions[1].target_model == Book + assert ( + str(result.get_text_clause()) == f"( authors.name = 'aa' AND " + f"{result.actions[1].table_prefix}" + f"_books.title = 'bb' )" + ) + assert not result.is_source_model_filter + + +def test_nested_and(): + result = ormar.and_( + ormar.or_(name="aa", books__title="bb"), ormar.or_(name="cc", books__title="dd") + ) + result.resolve(model_cls=Author) + assert len(result.actions) == 0 + assert len(result._nested_groups) == 2 + book_prefix = result._nested_groups[0].actions[1].table_prefix + assert ( + str(result.get_text_clause()) == f"( ( authors.name = 'aa' OR " + f"{book_prefix}" + f"_books.title = 'bb' ) AND " + f"( authors.name = 'cc' OR " + f"{book_prefix}" + f"_books.title = 'dd' ) )" + ) + assert not result.is_source_model_filter + + +def test_nested_group_and_action(): + result = ormar.and_(ormar.or_(name="aa", books__title="bb"), books__title="dd") + result.resolve(model_cls=Author) + assert len(result.actions) == 1 + assert len(result._nested_groups) == 1 + book_prefix = result._nested_groups[0].actions[1].table_prefix + assert ( + str(result.get_text_clause()) == f"( ( authors.name = 'aa' OR " + f"{book_prefix}" + f"_books.title = 'bb' ) AND " + f"{book_prefix}" + f"_books.title = 'dd' )" + ) + assert not result.is_source_model_filter + + +def test_deeply_nested_or(): + result = ormar.or_( + ormar.and_( + ormar.or_(name="aa", books__title="bb"), + ormar.or_(name="cc", books__title="dd"), + ), + ormar.and_( + ormar.or_(books__year__lt=1900, books__title="11"), + ormar.or_(books__year__gt="xx", books__title="22"), + ), + ) + result.resolve(model_cls=Author) + assert len(result.actions) == 0 + assert len(result._nested_groups) == 2 + assert len(result._nested_groups[0]._nested_groups) == 2 + book_prefix = result._nested_groups[0]._nested_groups[0].actions[1].table_prefix + result_qry = str(result.get_text_clause()) + expected_qry = ( + f"( ( ( authors.name = 'aa' OR {book_prefix}_books.title = 'bb' ) AND " + f"( authors.name = 'cc' OR {book_prefix}_books.title = 'dd' ) ) " + f"OR ( ( {book_prefix}_books.year < 1900 OR {book_prefix}_books.title = '11' ) AND " + f"( {book_prefix}_books.year > 'xx' OR {book_prefix}_books.title = '22' ) ) )" + ) + assert result_qry.replace("\n", "") == expected_qry.replace("\n", "") + assert not result.is_source_model_filter + + +def test_one_model_group(): + result = ormar.and_(year__gt=1900, title="bb") + result.resolve(model_cls=Book) + assert len(result.actions) == 2 + assert len(result._nested_groups) == 0 + assert result.is_source_model_filter + + +def test_one_model_nested_group(): + result = ormar.and_( + ormar.or_(year__gt=1900, title="bb"), ormar.or_(year__lt=1800, title="aa") + ) + result.resolve(model_cls=Book) + assert len(result.actions) == 0 + assert len(result._nested_groups) == 2 + assert result.is_source_model_filter + + +def test_one_model_with_group(): + result = ormar.or_(ormar.and_(year__gt=1900, title="bb"), title="uu") + result.resolve(model_cls=Book) + assert len(result.actions) == 1 + assert len(result._nested_groups) == 1 + assert result.is_source_model_filter diff --git a/tests/test_isnull_filter.py b/tests/test_isnull_filter.py new file mode 100644 index 0000000..2db0c9f --- /dev/null +++ b/tests/test_isnull_filter.py @@ -0,0 +1,76 @@ +from typing import Optional + +import databases +import pytest +import sqlalchemy + +import ormar +from tests.settings import DATABASE_URL + +database = databases.Database(DATABASE_URL) +metadata = sqlalchemy.MetaData() + + +class BaseMeta(ormar.ModelMeta): + metadata = metadata + database = database + + +class Author(ormar.Model): + class Meta(BaseMeta): + tablename = "authors" + + id: int = ormar.Integer(primary_key=True) + name: str = ormar.String(max_length=100) + + +class Book(ormar.Model): + class Meta(BaseMeta): + tablename = "books" + + id: int = ormar.Integer(primary_key=True) + author: Optional[Author] = ormar.ForeignKey(Author) + title: str = ormar.String(max_length=100) + year: int = ormar.Integer(nullable=True) + + +@pytest.fixture(autouse=True, scope="module") +def create_test_database(): + engine = sqlalchemy.create_engine(DATABASE_URL) + metadata.drop_all(engine) + metadata.create_all(engine) + yield + metadata.drop_all(engine) + + +@pytest.mark.asyncio +async def test_is_null(): + async with database: + tolkien = await Author.objects.create(name="J.R.R. Tolkien") + await Book.objects.create(author=tolkien, title="The Hobbit") + await Book.objects.create( + author=tolkien, title="The Lord of the Rings", year=1955 + ) + await Book.objects.create(author=tolkien, title="The Silmarillion", year=1977) + + books = await Book.objects.all(year__isnull=True) + assert len(books) == 1 + assert books[0].year is None + assert books[0].title == "The Hobbit" + + books = await Book.objects.all(year__isnull=False) + assert len(books) == 2 + + tolkien = await Author.objects.select_related("books").get( + books__year__isnull=True + ) + assert len(tolkien.books) == 1 + assert tolkien.books[0].year is None + assert tolkien.books[0].title == "The Hobbit" + + tolkien = await Author.objects.select_related("books").get( + books__year__isnull=False + ) + assert len(tolkien.books) == 2 + assert tolkien.books[0].year == 1955 + assert tolkien.books[0].title == "The Lord of the Rings" diff --git a/tests/test_or_filters.py b/tests/test_or_filters.py new file mode 100644 index 0000000..b3300d1 --- /dev/null +++ b/tests/test_or_filters.py @@ -0,0 +1,118 @@ +from typing import Optional + +import databases +import pytest +import sqlalchemy + +import ormar +from tests.settings import DATABASE_URL + +database = databases.Database(DATABASE_URL) +metadata = sqlalchemy.MetaData() + + +class BaseMeta(ormar.ModelMeta): + metadata = metadata + database = database + + +class Author(ormar.Model): + class Meta(BaseMeta): + tablename = "authors" + + id: int = ormar.Integer(primary_key=True) + name: str = ormar.String(max_length=100) + + +class Book(ormar.Model): + class Meta(BaseMeta): + tablename = "books" + + id: int = ormar.Integer(primary_key=True) + author: Optional[Author] = ormar.ForeignKey(Author) + title: str = ormar.String(max_length=100) + year: int = ormar.Integer(nullable=True) + + +@pytest.fixture(autouse=True, scope="module") +def create_test_database(): + engine = sqlalchemy.create_engine(DATABASE_URL) + metadata.drop_all(engine) + metadata.create_all(engine) + yield + metadata.drop_all(engine) + + +@pytest.mark.asyncio +async def test_or_filters(): + async with database: + tolkien = await Author(name="J.R.R. Tolkien").save() + await Book(author=tolkien, title="The Hobbit", year=1933).save() + await Book(author=tolkien, title="The Lord of the Rings", year=1955).save() + await Book(author=tolkien, title="The Silmarillion", year=1977).save() + sapkowski = await Author(name="Andrzej Sapkowski").save() + await Book(author=sapkowski, title="The Witcher", year=1990).save() + await Book(author=sapkowski, title="The Tower of Fools", year=2002).save() + + books = ( + await Book.objects.select_related("author") + .filter(ormar.or_(author__name="J.R.R. Tolkien", year__gt=1970)) + .all() + ) + assert len(books) == 5 + + books = ( + await Book.objects.select_related("author") + .filter(ormar.or_(author__name="J.R.R. Tolkien", year__lt=1995)) + .all() + ) + assert len(books) == 4 + assert not any([x.title == "The Tower of Fools" for x in books]) + + books = ( + await Book.objects.select_related("author") + .filter(ormar.or_(year__gt=1960, year__lt=1940)) + .filter(author__name="J.R.R. Tolkien") + .all() + ) + assert len(books) == 2 + assert books[0].title == "The Hobbit" + assert books[1].title == "The Silmarillion" + + books = ( + await Book.objects.select_related("author") + .filter( + ormar.or_( + ormar.and_(year__gt=1960, author__name="J.R.R. Tolkien"), + ormar.and_(year__lt=2000, author__name="Andrzej Sapkowski"), + ) + ) + .filter(title__startswith="The") + .all() + ) + assert len(books) == 2 + assert books[0].title == "The Silmarillion" + assert books[1].title == "The Witcher" + + books = ( + await Book.objects.select_related("author") + .exclude( + ormar.or_( + ormar.and_(year__gt=1960, author__name="J.R.R. Tolkien"), + ormar.and_(year__lt=2000, author__name="Andrzej Sapkowski"), + ) + ) + .filter(title__startswith="The") + .all() + ) + assert len(books) == 3 + assert not any([x.title in ["The Silmarillion", "The Witcher"] for x in books]) + + +# TODO: Check / modify +# process and and or into filter groups (V) +# check exclude queries working (V) + +# when limit and no sql do not allow main model and other models +# check complex prefixes properly resolved +# fix types for FilterAction and FilterGroup (?) From 79ad69e355543631e28452a03eaedc866fd31ea8 Mon Sep 17 00:00:00 2001 From: collerek Date: Sun, 7 Mar 2021 12:50:40 +0100 Subject: [PATCH 02/16] check complex prefixes in groups, refactor limit queries, finish docstrings, refactors and cleanup in long methods --- ormar/models/helpers/sqlalchemy.py | 53 +++++++- ormar/models/mixins/relation_mixin.py | 41 ++++++- ormar/models/model_row.py | 155 ++++++++++++++++++------ ormar/models/newbasemodel.py | 2 +- ormar/queryset/actions/filter_action.py | 3 - ormar/queryset/clause.py | 95 +++++++++++---- ormar/queryset/query.py | 48 +++----- ormar/queryset/queryset.py | 43 +++++-- ormar/queryset/utils.py | 58 +++++++-- tests/test_filter_groups.py | 8 -- tests/test_more_same_table_joins.py | 42 +++++++ tests/test_or_filters.py | 56 ++++++++- 12 files changed, 468 insertions(+), 136 deletions(-) diff --git a/ormar/models/helpers/sqlalchemy.py b/ormar/models/helpers/sqlalchemy.py index 3475c66..e40239d 100644 --- a/ormar/models/helpers/sqlalchemy.py +++ b/ormar/models/helpers/sqlalchemy.py @@ -150,19 +150,68 @@ def sqlalchemy_columns_from_model_fields( "Integer primary key named `id` created." ) validate_related_names_in_relations(model_fields, new_model) + return _process_fields(model_fields=model_fields, new_model=new_model) + + +def _process_fields( + model_fields: Dict, new_model: Type["Model"] +) -> Tuple[Optional[str], List[sqlalchemy.Column]]: + """ + Helper method. + + Populates pkname and columns. + Trigger validation of primary_key - only one and required pk can be set, + cannot be pydantic_only. + + Append fields to columns if it's not pydantic_only, + virtual ForeignKey or ManyToMany field. + + Sets `owner` on each model_field as reference to newly created Model. + + :raises ModelDefinitionError: if validation of related_names fail, + or pkname validation fails. + :param model_fields: dictionary of declared ormar model fields + :type model_fields: Dict[str, ormar.Field] + :param new_model: + :type new_model: Model class + :return: pkname, list of sqlalchemy columns + :rtype: Tuple[Optional[str], List[sqlalchemy.Column]] + """ columns = [] pkname = None for field_name, field in model_fields.items(): field.owner = new_model - if field.is_multi and not field.through: + if _is_through_model_not_set(field): field.create_default_through_model() if field.primary_key: pkname = check_pk_column_validity(field_name, field, pkname) - if not field.pydantic_only and not field.virtual and not field.is_multi: + if _is_db_field(field): columns.append(field.get_column(field.get_alias())) return pkname, columns +def _is_through_model_not_set(field: Type["BaseField"]) -> bool: + """ + Alias to if check that verifies if through model was created. + :param field: field to check + :type field: Type["BaseField"] + :return: result of the check + :rtype: bool + """ + return field.is_multi and not field.through + + +def _is_db_field(field: Type["BaseField"]) -> bool: + """ + Alias to if check that verifies if field should be included in database. + :param field: field to check + :type field: Type["BaseField"] + :return: result of the check + :rtype: bool + """ + return not field.pydantic_only and not field.virtual and not field.is_multi + + def populate_meta_tablename_columns_and_pk( name: str, new_model: Type["Model"] ) -> Type["Model"]: diff --git a/ormar/models/mixins/relation_mixin.py b/ormar/models/mixins/relation_mixin.py index 53fab3c..8955acf 100644 --- a/ormar/models/mixins/relation_mixin.py +++ b/ormar/models/mixins/relation_mixin.py @@ -129,7 +129,7 @@ class RelationMixin: return related_names @classmethod - def _iterate_related_models( + def _iterate_related_models( # noqa: CCR001 cls, visited: Set[str] = None, source_visited: Set[str] = None, @@ -149,14 +149,12 @@ class RelationMixin: :return: list of relation strings to be passed to select_related :rtype: List[str] """ - source_visited = source_visited or set() - if not source_model: - source_visited = cls._populate_source_model_prefixes() + source_visited = source_visited or cls._populate_source_model_prefixes() relations = cls.extract_related_names() processed_relations = [] for relation in relations: target_model = cls.Meta.model_fields[relation].to - if source_model and target_model == source_model: + if cls._is_reverse_side_of_same_relation(source_model, target_model): continue if target_model not in source_visited or not source_model: deep_relations = target_model._iterate_related_models( @@ -168,6 +166,39 @@ class RelationMixin: processed_relations.extend(deep_relations) else: processed_relations.append(relation) + + return cls._get_final_relations(processed_relations, source_relation) + + @staticmethod + def _is_reverse_side_of_same_relation( + source_model: Optional[Union[Type["Model"], Type["RelationMixin"]]], + target_model: Type["Model"], + ) -> bool: + """ + Alias to check if source model is the same as target + :param source_model: source model - relation comes from it + :type source_model: Type["Model"] + :param target_model: target model - relation leads to it + :type target_model: Type["Model"] + :return: result of the check + :rtype: bool + """ + return bool(source_model and target_model == source_model) + + @staticmethod + def _get_final_relations( + processed_relations: List, source_relation: Optional[str] + ) -> List[str]: + """ + Helper method to prefix nested relation strings with current source relation + + :param processed_relations: list of already processed relation str + :type processed_relations: List[str] + :param source_relation: name of the current relation + :type source_relation: str + :return: list of relation strings to be passed to select_related + :rtype: List[str] + """ if processed_relations: final_relations = [ f"{source_relation + '__' if source_relation else ''}{relation}" diff --git a/ormar/models/model_row.py b/ormar/models/model_row.py index d9c674d..2cd488a 100644 --- a/ormar/models/model_row.py +++ b/ormar/models/model_row.py @@ -4,7 +4,9 @@ from typing import ( List, Optional, TYPE_CHECKING, + Tuple, Type, + Union, cast, ) @@ -78,21 +80,12 @@ class ModelRow(NewBaseModel): related_models = group_related_list(select_related) if related_field: - if related_field.is_multi: - previous_model = related_field.through - else: - previous_model = related_field.owner - table_prefix = cls.Meta.alias_manager.resolve_relation_alias( - from_model=previous_model, relation_name=related_field.name + table_prefix = cls._process_table_prefix( + source_model=source_model, + current_relation_str=current_relation_str, + related_field=related_field, + used_prefixes=used_prefixes, ) - if not table_prefix or table_prefix in used_prefixes: - manager = cls.Meta.alias_manager - table_prefix = manager.resolve_relation_alias_after_complex( - source_model=source_model, - relation_str=current_relation_str, - relation_field=related_field, - ) - used_prefixes.append(table_prefix) item = cls._populate_nested_models_from_row( item=item, @@ -118,6 +111,44 @@ class ModelRow(NewBaseModel): instance.set_save_status(True) return instance + @classmethod + def _process_table_prefix( + cls, + source_model: Type["Model"], + current_relation_str: str, + related_field: Type["ForeignKeyField"], + used_prefixes: List[str], + ) -> str: + """ + + :param source_model: model on which relation was defined + :type source_model: Type[Model] + :param current_relation_str: current relation string + :type current_relation_str: str + :param related_field: field with relation declaration + :type related_field: Type["ForeignKeyField"] + :param used_prefixes: list of already extracted prefixes + :type used_prefixes: List[str] + :return: table_prefix to use + :rtype: str + """ + if related_field.is_multi: + previous_model = related_field.through + else: + previous_model = related_field.owner + table_prefix = cls.Meta.alias_manager.resolve_relation_alias( + from_model=previous_model, relation_name=related_field.name + ) + if not table_prefix or table_prefix in used_prefixes: + manager = cls.Meta.alias_manager + table_prefix = manager.resolve_relation_alias_after_complex( + source_model=source_model, + relation_str=current_relation_str, + relation_field=related_field, + ) + used_prefixes.append(table_prefix) + return table_prefix + @classmethod def _populate_nested_models_from_row( # noqa: CFQ002 cls, @@ -170,14 +201,11 @@ class ModelRow(NewBaseModel): if model_excludable.is_excluded(related): return item - relation_str = ( - "__".join([current_relation_str, related]) - if current_relation_str - else related + relation_str, remainder = cls._process_remainder_and_relation_string( + related_models=related_models, + current_relation_str=current_relation_str, + related=related, ) - remainder = None - if isinstance(related_models, dict) and related_models[related]: - remainder = related_models[related] child = model_cls.from_row( row, related_models=remainder, @@ -190,24 +218,84 @@ class ModelRow(NewBaseModel): ) item[model_cls.get_column_name_from_alias(related)] = child if field.is_multi and child: - through_name = cls.Meta.model_fields[related].through.get_name() - through_child = cls.populate_through_instance( + cls._populate_through_instance( row=row, + item=item, related=related, - through_name=through_name, excludable=excludable, + child=child, + proxy_source_model=proxy_source_model, ) - if child.__class__ != proxy_source_model: - setattr(child, through_name, through_child) - else: - item[through_name] = through_child - child.set_save_status(True) - return item + @staticmethod + def _process_remainder_and_relation_string( + related_models: Union[Dict, List], + current_relation_str: Optional[str], + related: str, + ) -> Tuple[str, Optional[Union[Dict, List]]]: + """ + Process remainder models and relation string + + :param related_models: list or dict of related models + :type related_models: Union[Dict, List] + :param current_relation_str: current relation string + :type current_relation_str: Optional[str] + :param related: name of the relation + :type related: str + """ + relation_str = ( + "__".join([current_relation_str, related]) + if current_relation_str + else related + ) + + remainder = None + if isinstance(related_models, dict) and related_models[related]: + remainder = related_models[related] + return relation_str, remainder + @classmethod - def populate_through_instance( + def _populate_through_instance( # noqa: CFQ002 + cls, + row: sqlalchemy.engine.ResultProxy, + item: Dict, + related: str, + excludable: ExcludableItems, + child: "Model", + proxy_source_model: Optional[Type["Model"]], + ) -> None: + """ + Populates the through model on reverse side of current query. + Normally it's child class, unless the query is from queryset. + + :param row: row from db result + :type row: sqlalchemy.engine.ResultProxy + :param item: parent item dict + :type item: Dict + :param related: current relation name + :type related: str + :param excludable: structure of fields to include and exclude + :type excludable: ExcludableItems + :param child: child item of parent + :type child: "Model" + :param proxy_source_model: source model from which querysetproxy is constructed + :type proxy_source_model: Type["Model"] + """ + through_name = cls.Meta.model_fields[related].through.get_name() + through_child = cls._create_through_instance( + row=row, related=related, through_name=through_name, excludable=excludable, + ) + + if child.__class__ != proxy_source_model: + setattr(child, through_name, through_child) + else: + item[through_name] = through_child + child.set_save_status(True) + + @classmethod + def _create_through_instance( cls, row: sqlalchemy.engine.ResultProxy, through_name: str, @@ -288,12 +376,11 @@ class ModelRow(NewBaseModel): model=cls, excludable=excludable, alias=table_prefix, use_alias=False, ) + column_prefix = table_prefix + "_" if table_prefix else "" for column in cls.Meta.table.columns: alias = cls.get_column_name_from_alias(column.name) if alias not in item and alias in selected_columns: - prefixed_name = ( - f'{table_prefix + "_" if table_prefix else ""}{column.name}' - ) + prefixed_name = f"{column_prefix}{column.name}" item[alias] = source[prefixed_name] return item diff --git a/ormar/models/newbasemodel.py b/ormar/models/newbasemodel.py index 9308d83..8ffafc7 100644 --- a/ormar/models/newbasemodel.py +++ b/ormar/models/newbasemodel.py @@ -227,7 +227,7 @@ class NewBaseModel(pydantic.BaseModel, ModelTableProxy, metaclass=ModelMetaclass super().__setattr__(name, value) self.set_save_status(False) - def __getattribute__(self, item: str) -> Any: + def __getattribute__(self, item: str) -> Any: # noqa: CCR001 """ Because we need to overwrite getting the attribute by ormar instead of pydantic as well as returning related models and not the value stored on the model the diff --git a/ormar/queryset/actions/filter_action.py b/ormar/queryset/actions/filter_action.py index 7b3bb9e..279e0fa 100644 --- a/ormar/queryset/actions/filter_action.py +++ b/ormar/queryset/actions/filter_action.py @@ -43,9 +43,6 @@ class FilterAction(QueryAction): super().__init__(query_str=filter_str, model_cls=model_cls) self.filter_value = value self._escape_characters_in_clause() - self.is_source_model_filter = False - if self.source_model == self.target_model and "__" not in self.related_str: - self.is_source_model_filter = True def has_escaped_characters(self) -> bool: """Check if value is a string that contains characters to escape""" diff --git a/ormar/queryset/clause.py b/ormar/queryset/clause.py index 4a1ef97..504a2eb 100644 --- a/ormar/queryset/clause.py +++ b/ormar/queryset/clause.py @@ -19,6 +19,11 @@ class FilterType(Enum): class FilterGroup: + """ + Filter groups are used in complex queries condition to group and and or + clauses in where condition + """ + def __init__( self, *args: Any, _filter_type: FilterType = FilterType.AND, **kwargs: Any, ) -> None: @@ -36,6 +41,19 @@ class FilterGroup: select_related: List = None, filter_clauses: List = None, ) -> Tuple[List[FilterAction], List[str]]: + """ + Resolves the FilterGroups actions to use proper target model, replace + complex relation prefixes if needed and nested groups also resolved. + + :param model_cls: model from which the query is run + :type model_cls: Type["Model"] + :param select_related: list of models to join + :type select_related: List[str] + :param filter_clauses: list of filter conditions + :type filter_clauses: List[FilterAction] + :return: list of filter conditions and select_related list + :rtype: Tuple[List[FilterAction], List[str]] + """ select_related = select_related if select_related is not None else [] filter_clauses = filter_clauses if filter_clauses is not None else [] qryclause = QueryClause( @@ -51,42 +69,44 @@ class FilterGroup: self._resolved = True if self._nested_groups: for group in self._nested_groups: - if not group._resolved: - (filter_clauses, select_related) = group.resolve( - model_cls=model_cls, - select_related=select_related, - filter_clauses=filter_clauses, - ) - self._is_self_model_group() + (filter_clauses, select_related) = group.resolve( + model_cls=model_cls, + select_related=select_related, + filter_clauses=filter_clauses, + ) return filter_clauses, select_related def _iter(self) -> Generator: - if not self._nested_groups: - yield from self.actions - return + """ + Iterates all actions in a tree + :return: generator yielding from own actions and nested groups + :rtype: Generator + """ for group in self._nested_groups: yield from group._iter() yield from self.actions - def _is_self_model_group(self) -> None: - if self.actions and self._nested_groups: - if all([action.is_source_model_filter for action in self.actions]) and all( - group.is_source_model_filter for group in self._nested_groups - ): - self.is_source_model_filter = True - elif self.actions: - if all([action.is_source_model_filter for action in self.actions]): - self.is_source_model_filter = True - else: - if all(group.is_source_model_filter for group in self._nested_groups): - self.is_source_model_filter = True - def _get_text_clauses(self) -> List[sqlalchemy.sql.expression.TextClause]: + """ + Helper to return list of text queries from actions and nested groups + :return: list of text queries from actions and nested groups + :rtype: List[sqlalchemy.sql.elements.TextClause] + """ return [x.get_text_clause() for x in self._nested_groups] + [ x.get_text_clause() for x in self.actions ] def get_text_clause(self) -> sqlalchemy.sql.expression.TextClause: + """ + Returns all own actions and nested groups conditions compiled and joined + inside parentheses. + Escapes characters if it's required. + Substitutes values of the models if value is a ormar Model with its pk value. + Compiles the clause. + + :return: complied and escaped clause + :rtype: sqlalchemy.sql.elements.TextClause + """ if self.filter_type == FilterType.AND: clause = sqlalchemy.text( "( " + str(sqlalchemy.sql.and_(*self._get_text_clauses())) + " )" @@ -98,11 +118,31 @@ class FilterGroup: return clause -def or_(*args: Any, **kwargs: Any) -> FilterGroup: +def or_(*args: FilterGroup, **kwargs: Any) -> FilterGroup: + """ + Construct or filter from nested groups and keyword arguments + + :param args: nested filter groups + :type args: Tuple[FilterGroup] + :param kwargs: fields names and proper value types + :type kwargs: Any + :return: FilterGroup ready to be resolved + :rtype: ormar.queryset.clause.FilterGroup + """ return FilterGroup(_filter_type=FilterType.OR, *args, **kwargs) -def and_(*args: Any, **kwargs: Any) -> FilterGroup: +def and_(*args: FilterGroup, **kwargs: Any) -> FilterGroup: + """ + Construct and filter from nested groups and keyword arguments + + :param args: nested filter groups + :type args: Tuple[FilterGroup] + :param kwargs: fields names and proper value types + :type kwargs: Any + :return: FilterGroup ready to be resolved + :rtype: ormar.queryset.clause.FilterGroup + """ return FilterGroup(_filter_type=FilterType.AND, *args, **kwargs) @@ -263,6 +303,11 @@ class QueryClause: return filter_clauses def _verify_prefix_and_switch(self, action: "FilterAction") -> None: + """ + Helper to switch prefix to complex relation one if required + :param action: action to switch prefix in + :type action: ormar.queryset.actions.filter_action.FilterAction + """ manager = self.model_cls.Meta.alias_manager new_alias = manager.resolve_relation_alias(self.model_cls, action.related_str) if "__" in action.related_str and new_alias: diff --git a/ormar/queryset/query.py b/ormar/queryset/query.py index 0987bac..be46e74 100644 --- a/ormar/queryset/query.py +++ b/ormar/queryset/query.py @@ -108,10 +108,7 @@ class Query: "", self.table, self_related_fields ) self.apply_order_bys_for_primary_model() - if self._pagination_query_required(): - self.select_from = self._build_pagination_subquery() - else: - self.select_from = self.table + self.select_from = self.table related_models = group_related_list(self._select_related) @@ -139,6 +136,9 @@ class Query: self.sorted_orders, ) = sql_join.build_join() + if self._pagination_query_required(): + self._build_pagination_condition() + expr = sqlalchemy.sql.select(self.columns) expr = expr.select_from(self.select_from) @@ -149,7 +149,7 @@ class Query: return expr - def _build_pagination_subquery(self) -> sqlalchemy.sql.select: + def _build_pagination_condition(self) -> None: """ In order to apply limit and offset on main table in join only (otherwise you can get only partially constructed main model @@ -160,32 +160,20 @@ class Query: and query has select_related applied. Otherwise we can limit/offset normally at the end of whole query. - :return: constructed subquery on main table with limit, offset and order applied - :rtype: sqlalchemy.sql.select + The condition is added to filters to filter out desired number of main model + primary key values. Whole query is used to determine the values. """ - expr = sqlalchemy.sql.select(self.model_cls.Meta.table.columns) - expr = LimitQuery(limit_count=self.limit_count).apply(expr) - expr = OffsetQuery(query_offset=self.query_offset).apply(expr) - filters_to_use = [ - filter_clause - for filter_clause in self.filter_clauses - if filter_clause.is_source_model_filter - ] - excludes_to_use = [ - filter_clause - for filter_clause in self.exclude_clauses - if filter_clause.is_source_model_filter - ] - sorts_to_use = { - k: v for k, v in self.sorted_orders.items() if k.is_source_model_order - } - expr = FilterQuery(filter_clauses=filters_to_use).apply(expr) - expr = FilterQuery(filter_clauses=excludes_to_use, exclude=True).apply(expr) - expr = OrderQuery(sorted_orders=sorts_to_use).apply(expr) - expr = expr.alias(f"{self.table}") - self.filter_clauses = list(set(self.filter_clauses) - set(filters_to_use)) - self.exclude_clauses = list(set(self.exclude_clauses) - set(excludes_to_use)) - return expr + pk_alias = self.model_cls.get_column_alias(self.model_cls.Meta.pkname) + qry_text = sqlalchemy.text(f"distinct {self.table.name}.{pk_alias}") + limit_qry = sqlalchemy.sql.select([qry_text]) + limit_qry = limit_qry.select_from(self.select_from) + limit_qry = self._apply_expression_modifiers(limit_qry) + limit_qry = LimitQuery(limit_count=self.limit_count).apply(limit_qry) + limit_qry = OffsetQuery(query_offset=self.query_offset).apply(limit_qry) + limit_action = FilterAction( + filter_str=f"{pk_alias}__in", value=limit_qry, model_cls=self.model_cls + ) + self.filter_clauses.append(limit_action) def _apply_expression_modifiers( self, expr: sqlalchemy.sql.select diff --git a/ormar/queryset/queryset.py b/ormar/queryset/queryset.py index 944550e..e84b67d 100644 --- a/ormar/queryset/queryset.py +++ b/ormar/queryset/queryset.py @@ -20,7 +20,7 @@ from ormar import MultipleMatches, NoMatch from ormar.exceptions import ModelError, ModelPersistenceError, QueryDefinitionError from ormar.queryset import FilterQuery from ormar.queryset.actions.order_action import OrderAction -from ormar.queryset.clause import QueryClause +from ormar.queryset.clause import FilterGroup, QueryClause from ormar.queryset.prefetch_query import PrefetchQuery from ormar.queryset.query import Query @@ -192,6 +192,34 @@ class QuerySet: return self.model.merge_instances_list(result_rows) # type: ignore return result_rows + def _resolve_filter_groups(self, groups: Any) -> List[FilterGroup]: + """ + Resolves filter groups to populate FilterAction params in group tree. + + :param groups: tuple of FilterGroups + :type groups: Any + :return: list of resolver groups + :rtype: List[FilterGroup] + """ + filter_groups = [] + if groups: + for group in groups: + if not isinstance(group, FilterGroup): + raise QueryDefinitionError( + "Only ormar.and_ and ormar.or_ " + "can be passed as filter positional" + " arguments," + "other values need to be passed by" + "keyword arguments" + ) + group.resolve( + model_cls=self.model, + select_related=self._select_related, + filter_clauses=self.filter_clauses, + ) + filter_groups.append(group) + return filter_groups + @staticmethod def check_single_result_rows_count(rows: Sequence[Optional["Model"]]) -> None: """ @@ -288,23 +316,14 @@ class QuerySet: :return: filtered QuerySet :rtype: QuerySet """ - filter_groups = [] - if args: - for arg in args: - arg.resolve( - model_cls=self.model, - select_related=self._select_related, - filter_clauses=self.filter_clauses, - ) - filter_groups.append(arg) - + filter_groups = self._resolve_filter_groups(groups=args) qryclause = QueryClause( model_cls=self.model, select_related=self._select_related, filter_clauses=self.filter_clauses, ) filter_clauses, select_related = qryclause.prepare_filter(**kwargs) - filter_clauses = filter_clauses + filter_groups + filter_clauses = filter_clauses + filter_groups # type: ignore if _exclude: exclude_clauses = filter_clauses filter_clauses = self.filter_clauses diff --git a/ormar/queryset/utils.py b/ormar/queryset/utils.py index ca3358d..64e8af2 100644 --- a/ormar/queryset/utils.py +++ b/ormar/queryset/utils.py @@ -4,6 +4,7 @@ from typing import ( Any, Dict, List, + Optional, Sequence, Set, TYPE_CHECKING, @@ -13,7 +14,7 @@ from typing import ( ) if TYPE_CHECKING: # pragma no cover - from ormar import Model + from ormar import Model, BaseField def check_node_not_dict_or_not_last_node( @@ -238,18 +239,13 @@ def get_relationship_alias_model_and_str( related_field = target_model.Meta.model_fields[relation] if related_field.is_through: - # through is always last - cannot go further - is_through = True - related_parts.remove(relation) - through_field = related_field.owner.Meta.model_fields[ - related_field.related_name or "" - ] - if len(previous_models) > 1 and previous_models[-2] == through_field.to: - previous_model = through_field.to - relation = through_field.related_name - else: - relation = related_field.related_name - + (previous_model, relation, is_through) = _process_through_field( + related_parts=related_parts, + relation=relation, + related_field=related_field, + previous_model=previous_model, + previous_models=previous_models, + ) if related_field.is_multi: previous_model = related_field.through relation = related_field.default_target_field_name() # type: ignore @@ -263,3 +259,39 @@ def get_relationship_alias_model_and_str( relation_str = "__".join(related_parts) return table_prefix, target_model, relation_str, is_through + + +def _process_through_field( + related_parts: List, + relation: Optional[str], + related_field: Type["BaseField"], + previous_model: Type["Model"], + previous_models: List[Type["Model"]], +) -> Tuple[Type["Model"], Optional[str], bool]: + """ + Helper processing through models as they need to be treated differently. + + :param related_parts: split relation string + :type related_parts: List[str] + :param relation: relation name + :type relation: str + :param related_field: field with relation declaration + :type related_field: Type["ForeignKeyField"] + :param previous_model: model from which relation is coming + :type previous_model: Type["Model"] + :param previous_models: list of already visited models in relation chain + :type previous_models: List[Type["Model"]] + :return: previous_model, relation, is_through + :rtype: Tuple[Type["Model"], str, bool] + """ + is_through = True + related_parts.remove(relation) + through_field = related_field.owner.Meta.model_fields[ + related_field.related_name or "" + ] + if len(previous_models) > 1 and previous_models[-2] == through_field.to: + previous_model = through_field.to + relation = through_field.related_name + else: + relation = related_field.related_name + return previous_model, relation, is_through diff --git a/tests/test_filter_groups.py b/tests/test_filter_groups.py index ec6c4a0..21581b1 100644 --- a/tests/test_filter_groups.py +++ b/tests/test_filter_groups.py @@ -44,7 +44,6 @@ def test_or_group(): f"{result.actions[1].table_prefix}" f"_books.title = 'bb' )" ) - assert not result.is_source_model_filter def test_and_group(): @@ -58,7 +57,6 @@ def test_and_group(): f"{result.actions[1].table_prefix}" f"_books.title = 'bb' )" ) - assert not result.is_source_model_filter def test_nested_and(): @@ -77,7 +75,6 @@ def test_nested_and(): f"{book_prefix}" f"_books.title = 'dd' ) )" ) - assert not result.is_source_model_filter def test_nested_group_and_action(): @@ -93,7 +90,6 @@ def test_nested_group_and_action(): f"{book_prefix}" f"_books.title = 'dd' )" ) - assert not result.is_source_model_filter def test_deeply_nested_or(): @@ -120,7 +116,6 @@ def test_deeply_nested_or(): f"( {book_prefix}_books.year > 'xx' OR {book_prefix}_books.title = '22' ) ) )" ) assert result_qry.replace("\n", "") == expected_qry.replace("\n", "") - assert not result.is_source_model_filter def test_one_model_group(): @@ -128,7 +123,6 @@ def test_one_model_group(): result.resolve(model_cls=Book) assert len(result.actions) == 2 assert len(result._nested_groups) == 0 - assert result.is_source_model_filter def test_one_model_nested_group(): @@ -138,7 +132,6 @@ def test_one_model_nested_group(): result.resolve(model_cls=Book) assert len(result.actions) == 0 assert len(result._nested_groups) == 2 - assert result.is_source_model_filter def test_one_model_with_group(): @@ -146,4 +139,3 @@ def test_one_model_with_group(): result.resolve(model_cls=Book) assert len(result.actions) == 1 assert len(result._nested_groups) == 1 - assert result.is_source_model_filter diff --git a/tests/test_more_same_table_joins.py b/tests/test_more_same_table_joins.py index b991d13..13bab15 100644 --- a/tests/test_more_same_table_joins.py +++ b/tests/test_more_same_table_joins.py @@ -122,3 +122,45 @@ async def test_load_all_multiple_instances_of_same_table_in_schema(): assert len(math_class.dict().get("students")) == 2 assert math_class.teachers[0].category.department.name == "Law Department" assert math_class.students[0].category.department.name == "Math Department" + + +@pytest.mark.asyncio +async def test_filter_groups_with_instances_of_same_table_in_schema(): + async with database: + await create_data() + math_class = ( + await SchoolClass.objects.select_related( + ["teachers__category__department", "students__category__department"] + ) + .filter( + ormar.or_( + students__name="Jane", + teachers__category__name="Domestic", + students__category__name="Foreign", + ) + ) + .get(name="Math") + ) + assert math_class.name == "Math" + assert math_class.students[0].name == "Jane" + assert len(math_class.dict().get("students")) == 2 + assert math_class.teachers[0].category.department.name == "Law Department" + assert math_class.students[0].category.department.name == "Math Department" + + classes = ( + await SchoolClass.objects.select_related( + ["students__category__department", "teachers__category__department"] + ) + .filter( + ormar.and_( + ormar.or_( + students__name="Jane", students__category__name="Foreign" + ), + teachers__category__department__name="Law Department", + ) + ) + .all() + ) + assert len(classes) == 1 + assert classes[0].teachers[0].category.department.name == "Law Department" + assert classes[0].students[0].category.department.name == "Math Department" diff --git a/tests/test_or_filters.py b/tests/test_or_filters.py index b3300d1..2a33dc7 100644 --- a/tests/test_or_filters.py +++ b/tests/test_or_filters.py @@ -5,6 +5,7 @@ import pytest import sqlalchemy import ormar +from ormar.exceptions import QueryDefinitionError from tests.settings import DATABASE_URL database = databases.Database(DATABASE_URL) @@ -108,11 +109,60 @@ async def test_or_filters(): assert len(books) == 3 assert not any([x.title in ["The Silmarillion", "The Witcher"] for x in books]) + books = ( + await Book.objects.select_related("author") + .filter(ormar.or_(year__gt=1980, year__lt=1910)) + .filter(title__startswith="The") + .limit(1) + .all() + ) + assert len(books) == 1 + assert books[0].title == "The Witcher" + + books = ( + await Book.objects.select_related("author") + .filter(ormar.or_(year__gt=1980, author__name="Andrzej Sapkowski")) + .filter(title__startswith="The") + .limit(1) + .all() + ) + assert len(books) == 1 + assert books[0].title == "The Witcher" + + books = ( + await Book.objects.select_related("author") + .filter(ormar.or_(year__gt=1980, author__name="Andrzej Sapkowski")) + .filter(title__startswith="The") + .limit(1) + .offset(1) + .all() + ) + assert len(books) == 1 + assert books[0].title == "The Tower of Fools" + + books = ( + await Book.objects.select_related("author") + .filter(ormar.or_(year__gt=1980, author__name="Andrzej Sapkowski")) + .filter(title__startswith="The") + .limit(1) + .offset(1) + .order_by("-id") + .all() + ) + assert len(books) == 1 + assert books[0].title == "The Witcher" + + with pytest.raises(QueryDefinitionError): + await Book.objects.select_related("author").filter('wrong').all() + + # TODO: Check / modify # process and and or into filter groups (V) # check exclude queries working (V) +# check complex prefixes properly resolved (V) +# fix limit -> change to where subquery to extract number of distinct pk values (V) +# finish docstrings (V) +# fix types for FilterAction and FilterGroup (X) -# when limit and no sql do not allow main model and other models -# check complex prefixes properly resolved -# fix types for FilterAction and FilterGroup (?) +# add docs From 4a660549871807f6e401bf8b6a8b718801c77c79 Mon Sep 17 00:00:00 2001 From: collerek Date: Sun, 7 Mar 2021 13:40:45 +0100 Subject: [PATCH 03/16] change distinct to group by in limit action subquery --- ormar/queryset/query.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/ormar/queryset/query.py b/ormar/queryset/query.py index be46e74..9ba9dcc 100644 --- a/ormar/queryset/query.py +++ b/ormar/queryset/query.py @@ -164,10 +164,16 @@ class Query: primary key values. Whole query is used to determine the values. """ pk_alias = self.model_cls.get_column_alias(self.model_cls.Meta.pkname) - qry_text = sqlalchemy.text(f"distinct {self.table.name}.{pk_alias}") + qry_text = sqlalchemy.text(f"{self.table.name}.{pk_alias}") limit_qry = sqlalchemy.sql.select([qry_text]) limit_qry = limit_qry.select_from(self.select_from) limit_qry = self._apply_expression_modifiers(limit_qry) + limit_qry = FilterQuery(filter_clauses=self.filter_clauses).apply(limit_qry) + limit_qry = FilterQuery(filter_clauses=self.exclude_clauses, exclude=True).apply( + limit_qry + ) + limit_qry = limit_qry.group_by(qry_text) + limit_qry = OrderQuery(sorted_orders=self.sorted_orders).apply(limit_qry) limit_qry = LimitQuery(limit_count=self.limit_count).apply(limit_qry) limit_qry = OffsetQuery(query_offset=self.query_offset).apply(limit_qry) limit_action = FilterAction( From e8329c0dcd030c44158395c934193c4163a7eef5 Mon Sep 17 00:00:00 2001 From: collerek Date: Sun, 7 Mar 2021 14:10:35 +0100 Subject: [PATCH 04/16] check orders --- .github/workflows/test-package.yml | 8 ++--- ormar/queryset/query.py | 2 +- ormar/queryset/queryset.py | 2 +- tests/test_order_by.py | 55 ++++++++++++++++++------------ 4 files changed, 39 insertions(+), 28 deletions(-) diff --git a/.github/workflows/test-package.yml b/.github/workflows/test-package.yml index 4d92a32..00a1be1 100644 --- a/.github/workflows/test-package.yml +++ b/.github/workflows/test-package.yml @@ -51,14 +51,14 @@ jobs: run: | python -m pip install --upgrade pip if [ -f requirements.txt ]; then pip install -r requirements.txt; fi - - name: Run postgres - env: - DATABASE_URL: "postgresql://username:password@localhost:5432/testsuite" - run: bash scripts/test.sh - name: Run mysql env: DATABASE_URL: "mysql://username:password@127.0.0.1:3306/testsuite" run: bash scripts/test.sh + - name: Run postgres + env: + DATABASE_URL: "postgresql://username:password@localhost:5432/testsuite" + run: bash scripts/test.sh - name: Run sqlite env: DATABASE_URL: "sqlite:///testsuite" diff --git a/ormar/queryset/query.py b/ormar/queryset/query.py index 9ba9dcc..74f449a 100644 --- a/ormar/queryset/query.py +++ b/ormar/queryset/query.py @@ -173,7 +173,7 @@ class Query: limit_qry ) limit_qry = limit_qry.group_by(qry_text) - limit_qry = OrderQuery(sorted_orders=self.sorted_orders).apply(limit_qry) + # limit_qry = OrderQuery(sorted_orders=self.sorted_orders).apply(limit_qry) limit_qry = LimitQuery(limit_count=self.limit_count).apply(limit_qry) limit_qry = OffsetQuery(query_offset=self.query_offset).apply(limit_qry) limit_action = FilterAction( diff --git a/ormar/queryset/queryset.py b/ormar/queryset/queryset.py index e84b67d..844ec19 100644 --- a/ormar/queryset/queryset.py +++ b/ormar/queryset/queryset.py @@ -281,7 +281,7 @@ class QuerySet: limit_raw_sql=self.limit_sql_raw, ) exp = qry.build_select_expression() - # print("\n", exp.compile(compile_kwargs={"literal_binds": True})) + print("\n", exp.compile(compile_kwargs={"literal_binds": True})) return exp def filter( # noqa: A003 diff --git a/tests/test_order_by.py b/tests/test_order_by.py index bbb6385..c735e68 100644 --- a/tests/test_order_by.py +++ b/tests/test_order_by.py @@ -132,6 +132,11 @@ async def test_sort_order_on_main_model(): assert songs[1].name == "Song 2" assert songs[2].name == "Song 3" + songs = await Song.objects.order_by("name").limit(2).all() + assert len(songs) == 2 + assert songs[0].name == "Song 1" + assert songs[1].name == "Song 2" + await Song.objects.create(name="Song 4", sort_order=1) songs = await Song.objects.order_by(["sort_order", "name"]).all() @@ -169,27 +174,27 @@ async def test_sort_order_on_related_model(): owner = ( await Owner.objects.select_related("toys") - .order_by("toys__name") - .filter(name="Zeus") - .get() + .order_by("toys__name") + .filter(name="Zeus") + .get() ) assert owner.toys[0].name == "Toy 1" assert owner.toys[1].name == "Toy 4" owner = ( await Owner.objects.select_related("toys") - .order_by("-toys__name") - .filter(name="Zeus") - .get() + .order_by("-toys__name") + .filter(name="Zeus") + .get() ) assert owner.toys[0].name == "Toy 4" assert owner.toys[1].name == "Toy 1" owners = ( await Owner.objects.select_related("toys") - .order_by("-toys__name") - .filter(name__in=["Zeus", "Hermes"]) - .all() + .order_by("-toys__name") + .filter(name__in=["Zeus", "Hermes"]) + .all() ) assert owners[0].toys[0].name == "Toy 6" assert owners[0].toys[1].name == "Toy 5" @@ -203,9 +208,9 @@ async def test_sort_order_on_related_model(): owners = ( await Owner.objects.select_related("toys") - .order_by("-toys__name") - .filter(name__in=["Zeus", "Hermes"]) - .all() + .order_by("-toys__name") + .filter(name__in=["Zeus", "Hermes"]) + .all() ) assert owners[0].toys[0].name == "Toy 7" assert owners[0].toys[1].name == "Toy 4" @@ -216,6 +221,12 @@ async def test_sort_order_on_related_model(): assert owners[1].toys[1].name == "Toy 5" assert owners[1].name == "Hermes" + toys = await Toy.objects.select_related('owner').order_by('owner__name').limit( + 2).all() + assert len(toys) == 2 + assert toys[0].name == 'Toy 2' + assert toys[1].name == 'Toy 3' + @pytest.mark.asyncio async def test_sort_order_on_many_to_many(): @@ -245,9 +256,9 @@ async def test_sort_order_on_many_to_many(): user = ( await User.objects.select_related("cars") - .filter(name="Mark") - .order_by("cars__name") - .get() + .filter(name="Mark") + .order_by("cars__name") + .get() ) assert user.cars[0].name == "Buggy" assert user.cars[1].name == "Ferrari" @@ -256,9 +267,9 @@ async def test_sort_order_on_many_to_many(): user = ( await User.objects.select_related("cars") - .filter(name="Mark") - .order_by("-cars__name") - .get() + .filter(name="Mark") + .order_by("-cars__name") + .get() ) assert user.cars[3].name == "Buggy" assert user.cars[2].name == "Ferrari" @@ -274,8 +285,8 @@ async def test_sort_order_on_many_to_many(): users = ( await User.objects.select_related(["cars__factory"]) - .order_by(["-cars__factory__name", "cars__name"]) - .all() + .order_by(["-cars__factory__name", "cars__name"]) + .all() ) assert users[0].name == "Julie" @@ -321,8 +332,8 @@ async def test_sort_order_with_aliases(): aliases = ( await AliasTest.objects.select_related("nested") - .order_by("-nested__name") - .all() + .order_by("-nested__name") + .all() ) assert aliases[0].nested.name == "Try4" assert aliases[1].nested.name == "Try3" From b4350d11f1a5d5c33c220ee85c50ae36677b50e5 Mon Sep 17 00:00:00 2001 From: collerek Date: Sun, 7 Mar 2021 15:47:02 +0100 Subject: [PATCH 05/16] check limit as subquery --- ormar/queryset/actions/order_action.py | 14 ++++++- ormar/queryset/query.py | 56 ++++++++++++++++---------- 2 files changed, 48 insertions(+), 22 deletions(-) diff --git a/ormar/queryset/actions/order_action.py b/ormar/queryset/actions/order_action.py index 2173e24..28b92d1 100644 --- a/ormar/queryset/actions/order_action.py +++ b/ormar/queryset/actions/order_action.py @@ -20,7 +20,7 @@ class OrderAction(QueryAction): """ def __init__( - self, order_str: str, model_cls: Type["Model"], alias: str = None + self, order_str: str, model_cls: Type["Model"], alias: str = None ) -> None: self.direction: str = "" super().__init__(query_str=order_str, model_cls=model_cls) @@ -34,6 +34,18 @@ class OrderAction(QueryAction): def field_alias(self) -> str: return self.target_model.get_column_alias(self.field_name) + def get_field_name_text(self) -> str: + """ + Escapes characters if it's required. + Substitutes values of the models if value is a ormar Model with its pk value. + Compiles the clause. + + :return: complied and escaped clause + :rtype: sqlalchemy.sql.elements.TextClause + """ + prefix = f"{self.table_prefix}_" if self.table_prefix else "" + return f"{prefix}{self.table}" f".{self.field_alias}" + def get_text_clause(self) -> sqlalchemy.sql.expression.TextClause: """ Escapes characters if it's required. diff --git a/ormar/queryset/query.py b/ormar/queryset/query.py index 74f449a..e655359 100644 --- a/ormar/queryset/query.py +++ b/ormar/queryset/query.py @@ -18,16 +18,16 @@ if TYPE_CHECKING: # pragma no cover class Query: def __init__( # noqa CFQ002 - self, - model_cls: Type["Model"], - filter_clauses: List[FilterAction], - exclude_clauses: List[FilterAction], - select_related: List, - limit_count: Optional[int], - offset: Optional[int], - excludable: "ExcludableItems", - order_bys: Optional[List["OrderAction"]], - limit_raw_sql: bool, + self, + model_cls: Type["Model"], + filter_clauses: List[FilterAction], + exclude_clauses: List[FilterAction], + select_related: List, + limit_count: Optional[int], + offset: Optional[int], + excludable: "ExcludableItems", + order_bys: Optional[List["OrderAction"]], + limit_raw_sql: bool, ) -> None: self.query_offset = offset self.limit_count = limit_count @@ -137,7 +137,10 @@ class Query: ) = sql_join.build_join() if self._pagination_query_required(): - self._build_pagination_condition() + limit_qry, on_clause = self._build_pagination_condition() + self.select_from = sqlalchemy.sql.join( + self.select_from, limit_qry, on_clause + ) expr = sqlalchemy.sql.select(self.columns) expr = expr.select_from(self.select_from) @@ -149,7 +152,10 @@ class Query: return expr - def _build_pagination_condition(self) -> None: + def _build_pagination_condition( + self + ) -> Tuple[ + sqlalchemy.sql.expression.TextClause, sqlalchemy.sql.expression.TextClause]: """ In order to apply limit and offset on main table in join only (otherwise you can get only partially constructed main model @@ -164,25 +170,33 @@ class Query: primary key values. Whole query is used to determine the values. """ pk_alias = self.model_cls.get_column_alias(self.model_cls.Meta.pkname) - qry_text = sqlalchemy.text(f"{self.table.name}.{pk_alias}") - limit_qry = sqlalchemy.sql.select([qry_text]) + pk_aliased_name = f"{self.table.name}.{pk_alias}" + qry_text = sqlalchemy.text(pk_aliased_name) + maxes = dict() + for order in list(self.sorted_orders.keys()): + if order is not None and order.get_field_name_text() != pk_aliased_name: + maxes[order.get_field_name_text()] = sqlalchemy.text( + f"max({order.get_field_name_text()})") + + limit_qry = sqlalchemy.sql.select([qry_text]+list(maxes.values())) limit_qry = limit_qry.select_from(self.select_from) limit_qry = self._apply_expression_modifiers(limit_qry) limit_qry = FilterQuery(filter_clauses=self.filter_clauses).apply(limit_qry) - limit_qry = FilterQuery(filter_clauses=self.exclude_clauses, exclude=True).apply( + limit_qry = FilterQuery(filter_clauses=self.exclude_clauses, + exclude=True).apply( limit_qry ) limit_qry = limit_qry.group_by(qry_text) - # limit_qry = OrderQuery(sorted_orders=self.sorted_orders).apply(limit_qry) + limit_qry = OrderQuery(sorted_orders=self.sorted_orders).apply(limit_qry) limit_qry = LimitQuery(limit_count=self.limit_count).apply(limit_qry) limit_qry = OffsetQuery(query_offset=self.query_offset).apply(limit_qry) - limit_action = FilterAction( - filter_str=f"{pk_alias}__in", value=limit_qry, model_cls=self.model_cls - ) - self.filter_clauses.append(limit_action) + limit_qry = limit_qry.alias("limit_query") + on_clause = sqlalchemy.text( + f"limit_query.{pk_alias}={self.table.name}.{pk_alias}") + return limit_qry, on_clause def _apply_expression_modifiers( - self, expr: sqlalchemy.sql.select + self, expr: sqlalchemy.sql.select ) -> sqlalchemy.sql.select: """ Receives the select query (might be join) and applies: From c1eebf5096c0a9dd1c2d254c0f07083d084c2a57 Mon Sep 17 00:00:00 2001 From: collerek Date: Sun, 7 Mar 2021 16:07:22 +0100 Subject: [PATCH 06/16] another try - join with distinct from subquery --- ormar/queryset/query.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/ormar/queryset/query.py b/ormar/queryset/query.py index e655359..80ee4a7 100644 --- a/ormar/queryset/query.py +++ b/ormar/queryset/query.py @@ -171,12 +171,12 @@ class Query: """ pk_alias = self.model_cls.get_column_alias(self.model_cls.Meta.pkname) pk_aliased_name = f"{self.table.name}.{pk_alias}" - qry_text = sqlalchemy.text(pk_aliased_name) + qry_text = sqlalchemy.text(f"{pk_aliased_name} as limit_column") maxes = dict() for order in list(self.sorted_orders.keys()): if order is not None and order.get_field_name_text() != pk_aliased_name: - maxes[order.get_field_name_text()] = sqlalchemy.text( - f"max({order.get_field_name_text()})") + aliased_col = order.get_field_name_text() + maxes[aliased_col] = sqlalchemy.text(aliased_col) limit_qry = sqlalchemy.sql.select([qry_text]+list(maxes.values())) limit_qry = limit_qry.select_from(self.select_from) @@ -186,14 +186,18 @@ class Query: exclude=True).apply( limit_qry ) - limit_qry = limit_qry.group_by(qry_text) + limit_qry = limit_qry.group_by(sqlalchemy.text(f"{pk_aliased_name}")) limit_qry = OrderQuery(sorted_orders=self.sorted_orders).apply(limit_qry) - limit_qry = LimitQuery(limit_count=self.limit_count).apply(limit_qry) - limit_qry = OffsetQuery(query_offset=self.query_offset).apply(limit_qry) - limit_qry = limit_qry.alias("limit_query") + limit_qry = limit_qry.alias("inner_limit_query") + outer_text = sqlalchemy.text(f"distinct limit_column") + outer_qry = sqlalchemy.sql.select([outer_text]) + outer_qry = outer_qry.select_from(limit_qry) + outer_qry = LimitQuery(limit_count=self.limit_count).apply(outer_qry) + outer_qry = OffsetQuery(query_offset=self.query_offset).apply(outer_qry) + outer_qry = outer_qry.alias("limit_query") on_clause = sqlalchemy.text( - f"limit_query.{pk_alias}={self.table.name}.{pk_alias}") - return limit_qry, on_clause + f"limit_query.limit_column={self.table.name}.{pk_alias}") + return outer_qry, on_clause def _apply_expression_modifiers( self, expr: sqlalchemy.sql.select From 7d0e74e42b489b5dc31632fd941d7c99d2f891dc Mon Sep 17 00:00:00 2001 From: collerek Date: Sun, 7 Mar 2021 16:11:19 +0100 Subject: [PATCH 07/16] remove groupby --- ormar/queryset/query.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ormar/queryset/query.py b/ormar/queryset/query.py index 80ee4a7..66d7e8c 100644 --- a/ormar/queryset/query.py +++ b/ormar/queryset/query.py @@ -186,7 +186,6 @@ class Query: exclude=True).apply( limit_qry ) - limit_qry = limit_qry.group_by(sqlalchemy.text(f"{pk_aliased_name}")) limit_qry = OrderQuery(sorted_orders=self.sorted_orders).apply(limit_qry) limit_qry = limit_qry.alias("inner_limit_query") outer_text = sqlalchemy.text(f"distinct limit_column") From 30c8e725e6804843aef64a42a8ba4688b59658d8 Mon Sep 17 00:00:00 2001 From: collerek Date: Sun, 7 Mar 2021 16:15:08 +0100 Subject: [PATCH 08/16] remove other columns --- ormar/queryset/query.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/ormar/queryset/query.py b/ormar/queryset/query.py index 66d7e8c..0d5ac0f 100644 --- a/ormar/queryset/query.py +++ b/ormar/queryset/query.py @@ -172,13 +172,13 @@ class Query: pk_alias = self.model_cls.get_column_alias(self.model_cls.Meta.pkname) pk_aliased_name = f"{self.table.name}.{pk_alias}" qry_text = sqlalchemy.text(f"{pk_aliased_name} as limit_column") - maxes = dict() - for order in list(self.sorted_orders.keys()): - if order is not None and order.get_field_name_text() != pk_aliased_name: - aliased_col = order.get_field_name_text() - maxes[aliased_col] = sqlalchemy.text(aliased_col) + # maxes = dict() + # for order in list(self.sorted_orders.keys()): + # if order is not None and order.get_field_name_text() != pk_aliased_name: + # aliased_col = order.get_field_name_text() + # maxes[aliased_col] = sqlalchemy.text(aliased_col) - limit_qry = sqlalchemy.sql.select([qry_text]+list(maxes.values())) + limit_qry = sqlalchemy.sql.select([qry_text]) limit_qry = limit_qry.select_from(self.select_from) limit_qry = self._apply_expression_modifiers(limit_qry) limit_qry = FilterQuery(filter_clauses=self.filter_clauses).apply(limit_qry) From 037fc418a4fef03ab2623438469a0292dac9887e Mon Sep 17 00:00:00 2001 From: collerek Date: Sun, 7 Mar 2021 16:41:12 +0100 Subject: [PATCH 09/16] back to group by --- ormar/queryset/query.py | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/ormar/queryset/query.py b/ormar/queryset/query.py index 0d5ac0f..755c5fc 100644 --- a/ormar/queryset/query.py +++ b/ormar/queryset/query.py @@ -183,20 +183,15 @@ class Query: limit_qry = self._apply_expression_modifiers(limit_qry) limit_qry = FilterQuery(filter_clauses=self.filter_clauses).apply(limit_qry) limit_qry = FilterQuery(filter_clauses=self.exclude_clauses, - exclude=True).apply( - limit_qry - ) + exclude=True).apply(limit_qry) + limit_qry = limit_qry.group_by(sqlalchemy.text(f"{pk_aliased_name}")) limit_qry = OrderQuery(sorted_orders=self.sorted_orders).apply(limit_qry) - limit_qry = limit_qry.alias("inner_limit_query") - outer_text = sqlalchemy.text(f"distinct limit_column") - outer_qry = sqlalchemy.sql.select([outer_text]) - outer_qry = outer_qry.select_from(limit_qry) - outer_qry = LimitQuery(limit_count=self.limit_count).apply(outer_qry) - outer_qry = OffsetQuery(query_offset=self.query_offset).apply(outer_qry) - outer_qry = outer_qry.alias("limit_query") + limit_qry = LimitQuery(limit_count=self.limit_count).apply(limit_qry) + limit_qry = OffsetQuery(query_offset=self.query_offset).apply(limit_qry) + limit_qry = limit_qry.alias("limit_query") on_clause = sqlalchemy.text( f"limit_query.limit_column={self.table.name}.{pk_alias}") - return outer_qry, on_clause + return limit_qry, on_clause def _apply_expression_modifiers( self, expr: sqlalchemy.sql.select From 9c463831304fd1e516a3dae875d766ede2f71126 Mon Sep 17 00:00:00 2001 From: collerek Date: Sun, 7 Mar 2021 16:53:38 +0100 Subject: [PATCH 10/16] another try --- ormar/queryset/query.py | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/ormar/queryset/query.py b/ormar/queryset/query.py index 755c5fc..8e7db64 100644 --- a/ormar/queryset/query.py +++ b/ormar/queryset/query.py @@ -172,26 +172,31 @@ class Query: pk_alias = self.model_cls.get_column_alias(self.model_cls.Meta.pkname) pk_aliased_name = f"{self.table.name}.{pk_alias}" qry_text = sqlalchemy.text(f"{pk_aliased_name} as limit_column") - # maxes = dict() - # for order in list(self.sorted_orders.keys()): - # if order is not None and order.get_field_name_text() != pk_aliased_name: - # aliased_col = order.get_field_name_text() - # maxes[aliased_col] = sqlalchemy.text(aliased_col) + maxes = dict() + for ind, order in enumerate(list(self.sorted_orders.keys())): + if order is not None and order.get_field_name_text() != pk_aliased_name: + aliased_col = order.get_field_name_text() + maxes[aliased_col] = sqlalchemy.text(f"{aliased_col} as col{ind}") - limit_qry = sqlalchemy.sql.select([qry_text]) + limit_qry = sqlalchemy.sql.select([qry_text] + list(maxes.values())) limit_qry = limit_qry.select_from(self.select_from) limit_qry = self._apply_expression_modifiers(limit_qry) limit_qry = FilterQuery(filter_clauses=self.filter_clauses).apply(limit_qry) limit_qry = FilterQuery(filter_clauses=self.exclude_clauses, - exclude=True).apply(limit_qry) - limit_qry = limit_qry.group_by(sqlalchemy.text(f"{pk_aliased_name}")) + exclude=True).apply( + limit_qry + ) limit_qry = OrderQuery(sorted_orders=self.sorted_orders).apply(limit_qry) - limit_qry = LimitQuery(limit_count=self.limit_count).apply(limit_qry) - limit_qry = OffsetQuery(query_offset=self.query_offset).apply(limit_qry) - limit_qry = limit_qry.alias("limit_query") + limit_qry = limit_qry.alias("inner_limit_query") + outer_text = sqlalchemy.text(f"distinct limit_column") + outer_qry = sqlalchemy.sql.select([outer_text]) + outer_qry = outer_qry.select_from(limit_qry) + outer_qry = LimitQuery(limit_count=self.limit_count).apply(outer_qry) + outer_qry = OffsetQuery(query_offset=self.query_offset).apply(outer_qry) + outer_qry = outer_qry.alias("limit_query") on_clause = sqlalchemy.text( f"limit_query.limit_column={self.table.name}.{pk_alias}") - return limit_qry, on_clause + return outer_qry, on_clause def _apply_expression_modifiers( self, expr: sqlalchemy.sql.select From d067c03130595dad67bffb61898898049ddfb344 Mon Sep 17 00:00:00 2001 From: collerek Date: Sun, 7 Mar 2021 17:37:32 +0100 Subject: [PATCH 11/16] try with min and max filters depending on direction --- ormar/queryset/actions/order_action.py | 7 ++++++ ormar/queryset/query.py | 31 +++++++++++++------------- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/ormar/queryset/actions/order_action.py b/ormar/queryset/actions/order_action.py index 28b92d1..f999f32 100644 --- a/ormar/queryset/actions/order_action.py +++ b/ormar/queryset/actions/order_action.py @@ -46,6 +46,13 @@ class OrderAction(QueryAction): prefix = f"{self.table_prefix}_" if self.table_prefix else "" return f"{prefix}{self.table}" f".{self.field_alias}" + def get_min_or_max(self): + prefix = f"{self.table_prefix}_" if self.table_prefix else "" + if self.direction == '': + return text(f"min({prefix}{self.table}" f".{self.field_alias})") + else: + return text(f"max({prefix}{self.table}" f".{self.field_alias}) desc") + def get_text_clause(self) -> sqlalchemy.sql.expression.TextClause: """ Escapes characters if it's required. diff --git a/ormar/queryset/query.py b/ormar/queryset/query.py index 8e7db64..3a3e96c 100644 --- a/ormar/queryset/query.py +++ b/ormar/queryset/query.py @@ -171,32 +171,31 @@ class Query: """ pk_alias = self.model_cls.get_column_alias(self.model_cls.Meta.pkname) pk_aliased_name = f"{self.table.name}.{pk_alias}" - qry_text = sqlalchemy.text(f"{pk_aliased_name} as limit_column") - maxes = dict() - for ind, order in enumerate(list(self.sorted_orders.keys())): + qry_text = sqlalchemy.text(f"{pk_aliased_name}") + maxes = OrderedDict() + for order in list(self.sorted_orders.keys()): if order is not None and order.get_field_name_text() != pk_aliased_name: aliased_col = order.get_field_name_text() - maxes[aliased_col] = sqlalchemy.text(f"{aliased_col} as col{ind}") + maxes[aliased_col] = order.get_min_or_max() + elif order.get_field_name_text() == pk_aliased_name: + maxes[pk_aliased_name] = order.get_text_clause() - limit_qry = sqlalchemy.sql.select([qry_text] + list(maxes.values())) + limit_qry = sqlalchemy.sql.select([qry_text]) limit_qry = limit_qry.select_from(self.select_from) - limit_qry = self._apply_expression_modifiers(limit_qry) limit_qry = FilterQuery(filter_clauses=self.filter_clauses).apply(limit_qry) limit_qry = FilterQuery(filter_clauses=self.exclude_clauses, exclude=True).apply( limit_qry ) - limit_qry = OrderQuery(sorted_orders=self.sorted_orders).apply(limit_qry) - limit_qry = limit_qry.alias("inner_limit_query") - outer_text = sqlalchemy.text(f"distinct limit_column") - outer_qry = sqlalchemy.sql.select([outer_text]) - outer_qry = outer_qry.select_from(limit_qry) - outer_qry = LimitQuery(limit_count=self.limit_count).apply(outer_qry) - outer_qry = OffsetQuery(query_offset=self.query_offset).apply(outer_qry) - outer_qry = outer_qry.alias("limit_query") + limit_qry = limit_qry.group_by(qry_text) + for order_by in maxes.values(): + limit_qry = limit_qry.order_by(order_by) + limit_qry = LimitQuery(limit_count=self.limit_count).apply(limit_qry) + limit_qry = OffsetQuery(query_offset=self.query_offset).apply(limit_qry) + limit_qry = limit_qry.alias("limit_query") on_clause = sqlalchemy.text( - f"limit_query.limit_column={self.table.name}.{pk_alias}") - return outer_qry, on_clause + f"limit_query.{pk_alias}={self.table.name}.{pk_alias}") + return limit_qry, on_clause def _apply_expression_modifiers( self, expr: sqlalchemy.sql.select From f85fa7b8a7b7a08653ff449ceccca3a0ac213253 Mon Sep 17 00:00:00 2001 From: collerek Date: Sun, 7 Mar 2021 17:41:24 +0100 Subject: [PATCH 12/16] order also toy name --- tests/test_order_by.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_order_by.py b/tests/test_order_by.py index c735e68..21ff4b6 100644 --- a/tests/test_order_by.py +++ b/tests/test_order_by.py @@ -221,7 +221,8 @@ async def test_sort_order_on_related_model(): assert owners[1].toys[1].name == "Toy 5" assert owners[1].name == "Hermes" - toys = await Toy.objects.select_related('owner').order_by('owner__name').limit( + toys = await Toy.objects.select_related('owner').order_by( + ['owner__name', 'name']).limit( 2).all() assert len(toys) == 2 assert toys[0].name == 'Toy 2' From d388b9f74505da0ff904ed55c3f05fcf180bfd0f Mon Sep 17 00:00:00 2001 From: collerek Date: Sun, 7 Mar 2021 17:48:26 +0100 Subject: [PATCH 13/16] fix everything? --- ormar/queryset/actions/order_action.py | 13 ++++-- ormar/queryset/query.py | 37 +++++++++-------- ormar/queryset/queryset.py | 2 +- tests/test_or_filters.py | 3 +- tests/test_order_by.py | 57 ++++++++++++++------------ 5 files changed, 61 insertions(+), 51 deletions(-) diff --git a/ormar/queryset/actions/order_action.py b/ormar/queryset/actions/order_action.py index f999f32..2bba2d2 100644 --- a/ormar/queryset/actions/order_action.py +++ b/ormar/queryset/actions/order_action.py @@ -20,7 +20,7 @@ class OrderAction(QueryAction): """ def __init__( - self, order_str: str, model_cls: Type["Model"], alias: str = None + self, order_str: str, model_cls: Type["Model"], alias: str = None ) -> None: self.direction: str = "" super().__init__(query_str=order_str, model_cls=model_cls) @@ -46,9 +46,16 @@ class OrderAction(QueryAction): prefix = f"{self.table_prefix}_" if self.table_prefix else "" return f"{prefix}{self.table}" f".{self.field_alias}" - def get_min_or_max(self): + def get_min_or_max(self) -> sqlalchemy.sql.expression.TextClause: + """ + Used in limit sub queries where you need to use aggregated functions + in order to order by columns not included in group by. + + :return: min or max function to order + :rtype: sqlalchemy.sql.elements.TextClause + """ prefix = f"{self.table_prefix}_" if self.table_prefix else "" - if self.direction == '': + if self.direction == "": return text(f"min({prefix}{self.table}" f".{self.field_alias})") else: return text(f"max({prefix}{self.table}" f".{self.field_alias}) desc") diff --git a/ormar/queryset/query.py b/ormar/queryset/query.py index 3a3e96c..c5bea48 100644 --- a/ormar/queryset/query.py +++ b/ormar/queryset/query.py @@ -18,16 +18,16 @@ if TYPE_CHECKING: # pragma no cover class Query: def __init__( # noqa CFQ002 - self, - model_cls: Type["Model"], - filter_clauses: List[FilterAction], - exclude_clauses: List[FilterAction], - select_related: List, - limit_count: Optional[int], - offset: Optional[int], - excludable: "ExcludableItems", - order_bys: Optional[List["OrderAction"]], - limit_raw_sql: bool, + self, + model_cls: Type["Model"], + filter_clauses: List[FilterAction], + exclude_clauses: List[FilterAction], + select_related: List, + limit_count: Optional[int], + offset: Optional[int], + excludable: "ExcludableItems", + order_bys: Optional[List["OrderAction"]], + limit_raw_sql: bool, ) -> None: self.query_offset = offset self.limit_count = limit_count @@ -153,9 +153,10 @@ class Query: return expr def _build_pagination_condition( - self + self, ) -> Tuple[ - sqlalchemy.sql.expression.TextClause, sqlalchemy.sql.expression.TextClause]: + sqlalchemy.sql.expression.TextClause, sqlalchemy.sql.expression.TextClause + ]: """ In order to apply limit and offset on main table in join only (otherwise you can get only partially constructed main model @@ -183,10 +184,9 @@ class Query: limit_qry = sqlalchemy.sql.select([qry_text]) limit_qry = limit_qry.select_from(self.select_from) limit_qry = FilterQuery(filter_clauses=self.filter_clauses).apply(limit_qry) - limit_qry = FilterQuery(filter_clauses=self.exclude_clauses, - exclude=True).apply( - limit_qry - ) + limit_qry = FilterQuery( + filter_clauses=self.exclude_clauses, exclude=True + ).apply(limit_qry) limit_qry = limit_qry.group_by(qry_text) for order_by in maxes.values(): limit_qry = limit_qry.order_by(order_by) @@ -194,11 +194,12 @@ class Query: limit_qry = OffsetQuery(query_offset=self.query_offset).apply(limit_qry) limit_qry = limit_qry.alias("limit_query") on_clause = sqlalchemy.text( - f"limit_query.{pk_alias}={self.table.name}.{pk_alias}") + f"limit_query.{pk_alias}={self.table.name}.{pk_alias}" + ) return limit_qry, on_clause def _apply_expression_modifiers( - self, expr: sqlalchemy.sql.select + self, expr: sqlalchemy.sql.select ) -> sqlalchemy.sql.select: """ Receives the select query (might be join) and applies: diff --git a/ormar/queryset/queryset.py b/ormar/queryset/queryset.py index 844ec19..e84b67d 100644 --- a/ormar/queryset/queryset.py +++ b/ormar/queryset/queryset.py @@ -281,7 +281,7 @@ class QuerySet: limit_raw_sql=self.limit_sql_raw, ) exp = qry.build_select_expression() - print("\n", exp.compile(compile_kwargs={"literal_binds": True})) + # print("\n", exp.compile(compile_kwargs={"literal_binds": True})) return exp def filter( # noqa: A003 diff --git a/tests/test_or_filters.py b/tests/test_or_filters.py index 2a33dc7..0e24384 100644 --- a/tests/test_or_filters.py +++ b/tests/test_or_filters.py @@ -153,8 +153,7 @@ async def test_or_filters(): assert books[0].title == "The Witcher" with pytest.raises(QueryDefinitionError): - await Book.objects.select_related("author").filter('wrong').all() - + await Book.objects.select_related("author").filter("wrong").all() # TODO: Check / modify diff --git a/tests/test_order_by.py b/tests/test_order_by.py index 21ff4b6..ad688d2 100644 --- a/tests/test_order_by.py +++ b/tests/test_order_by.py @@ -174,27 +174,27 @@ async def test_sort_order_on_related_model(): owner = ( await Owner.objects.select_related("toys") - .order_by("toys__name") - .filter(name="Zeus") - .get() + .order_by("toys__name") + .filter(name="Zeus") + .get() ) assert owner.toys[0].name == "Toy 1" assert owner.toys[1].name == "Toy 4" owner = ( await Owner.objects.select_related("toys") - .order_by("-toys__name") - .filter(name="Zeus") - .get() + .order_by("-toys__name") + .filter(name="Zeus") + .get() ) assert owner.toys[0].name == "Toy 4" assert owner.toys[1].name == "Toy 1" owners = ( await Owner.objects.select_related("toys") - .order_by("-toys__name") - .filter(name__in=["Zeus", "Hermes"]) - .all() + .order_by("-toys__name") + .filter(name__in=["Zeus", "Hermes"]) + .all() ) assert owners[0].toys[0].name == "Toy 6" assert owners[0].toys[1].name == "Toy 5" @@ -208,9 +208,9 @@ async def test_sort_order_on_related_model(): owners = ( await Owner.objects.select_related("toys") - .order_by("-toys__name") - .filter(name__in=["Zeus", "Hermes"]) - .all() + .order_by("-toys__name") + .filter(name__in=["Zeus", "Hermes"]) + .all() ) assert owners[0].toys[0].name == "Toy 7" assert owners[0].toys[1].name == "Toy 4" @@ -221,12 +221,15 @@ async def test_sort_order_on_related_model(): assert owners[1].toys[1].name == "Toy 5" assert owners[1].name == "Hermes" - toys = await Toy.objects.select_related('owner').order_by( - ['owner__name', 'name']).limit( - 2).all() + toys = ( + await Toy.objects.select_related("owner") + .order_by(["owner__name", "name"]) + .limit(2) + .all() + ) assert len(toys) == 2 - assert toys[0].name == 'Toy 2' - assert toys[1].name == 'Toy 3' + assert toys[0].name == "Toy 2" + assert toys[1].name == "Toy 3" @pytest.mark.asyncio @@ -257,9 +260,9 @@ async def test_sort_order_on_many_to_many(): user = ( await User.objects.select_related("cars") - .filter(name="Mark") - .order_by("cars__name") - .get() + .filter(name="Mark") + .order_by("cars__name") + .get() ) assert user.cars[0].name == "Buggy" assert user.cars[1].name == "Ferrari" @@ -268,9 +271,9 @@ async def test_sort_order_on_many_to_many(): user = ( await User.objects.select_related("cars") - .filter(name="Mark") - .order_by("-cars__name") - .get() + .filter(name="Mark") + .order_by("-cars__name") + .get() ) assert user.cars[3].name == "Buggy" assert user.cars[2].name == "Ferrari" @@ -286,8 +289,8 @@ async def test_sort_order_on_many_to_many(): users = ( await User.objects.select_related(["cars__factory"]) - .order_by(["-cars__factory__name", "cars__name"]) - .all() + .order_by(["-cars__factory__name", "cars__name"]) + .all() ) assert users[0].name == "Julie" @@ -333,8 +336,8 @@ async def test_sort_order_with_aliases(): aliases = ( await AliasTest.objects.select_related("nested") - .order_by("-nested__name") - .all() + .order_by("-nested__name") + .all() ) assert aliases[0].nested.name == "Try4" assert aliases[1].nested.name == "Try3" From 472c8368e40e43f84021de11182f2a0fba190a8e Mon Sep 17 00:00:00 2001 From: collerek Date: Sun, 7 Mar 2021 18:50:35 +0100 Subject: [PATCH 14/16] add docs, release docs --- docs/queries/filter-and-sort.md | 179 +++++++++++++++++++++++++++++++- docs/releases.md | 17 ++- tests/test_or_filters.py | 35 ++++++- 3 files changed, 227 insertions(+), 4 deletions(-) diff --git a/docs/queries/filter-and-sort.md b/docs/queries/filter-and-sort.md index 8de1550..ed51f85 100644 --- a/docs/queries/filter-and-sort.md +++ b/docs/queries/filter-and-sort.md @@ -26,7 +26,7 @@ And following methods to sort the data (sql order by clause). ### filter -`filter(**kwargs) -> QuerySet` +`filter(*args, **kwargs) -> QuerySet` Allows you to filter by any `Model` attribute/field as well as to fetch instances, with a filter across an FK relationship. @@ -97,7 +97,7 @@ You can use special filter suffix to change the filter operands: ### exclude -`exclude(**kwargs) -> QuerySet` +`exclude(*args, **kwargs) -> QuerySet` Works exactly the same as filter and all modifiers (suffixes) are the same, but returns a not condition. @@ -139,6 +139,181 @@ notes = await Track.objects.exclude(position_gt=3).all() # returns all tracks with position < 3 ``` +## Complex filters (including OR) + +By default both `filter()` and `exclude()` methods combine provided filter options with +`AND` condition so `filter(name="John", age__gt=30)` translates into `WHERE name = 'John' AND age > 30`. + +Sometimes it's useful to query the database with conditions that should not be applied +jointly like `WHERE name = 'John' OR age > 30`, or build a complex where query that you would +like to have bigger control over. After all `WHERE (name = 'John' OR age > 30) and city='New York'` is +completely different than `WHERE name = 'John' OR (age > 30 and city='New York')`. + +In order to build `OR` and nested conditions ormar provides two functions that can be used in +`filter()` and `exclude()` in `QuerySet` and `QuerysetProxy`. + +!!!note + Note that you cannot provide those methods in any other method like `get()` or `all()` which accepts only keyword arguments. + +Call to `or_` and `and_` can be nested in each other, as well as combined with keyword arguments. +Since it sounds more complicated than it is, let's look at some examples. + +Given a sample models like this: +```python +database = databases.Database(DATABASE_URL) +metadata = sqlalchemy.MetaData() + + +class BaseMeta(ormar.ModelMeta): + metadata = metadata + database = database + + +class Author(ormar.Model): + class Meta(BaseMeta): + tablename = "authors" + + id: int = ormar.Integer(primary_key=True) + name: str = ormar.String(max_length=100) + + +class Book(ormar.Model): + class Meta(BaseMeta): + tablename = "books" + + id: int = ormar.Integer(primary_key=True) + author: Optional[Author] = ormar.ForeignKey(Author) + title: str = ormar.String(max_length=100) + year: int = ormar.Integer(nullable=True) +``` + +Let's create some sample data: + +```python +tolkien = await Author(name="J.R.R. Tolkien").save() +await Book(author=tolkien, title="The Hobbit", year=1933).save() +await Book(author=tolkien, title="The Lord of the Rings", year=1955).save() +await Book(author=tolkien, title="The Silmarillion", year=1977).save() +sapkowski = await Author(name="Andrzej Sapkowski").save() +await Book(author=sapkowski, title="The Witcher", year=1990).save() +await Book(author=sapkowski, title="The Tower of Fools", year=2002).save() +``` + +We can construct some sample complex queries: + +Let's select books of Tolkien **OR** books written after 1970 + +sql: +`WHERE ( authors.name = 'J.R.R. Tolkien' OR books.year > 1970 )` + +```python +books = ( + await Book.objects.select_related("author") + .filter(ormar.or_(author__name="J.R.R. Tolkien", year__gt=1970)) + .all() +) +assert len(books) == 5 +``` + +Now let's select books written after 1960 or before 1940 which were written by Tolkien. + +sql: +`WHERE ( books.year > 1960 OR books.year < 1940 ) AND authors.name = 'J.R.R. Tolkien'` + +```python +# OPTION 1 - split and into separate call +books = ( + await Book.objects.select_related("author") + .filter(ormar.or_(year__gt=1960, year__lt=1940)) + .filter(author__name="J.R.R. Tolkien") + .all() +) +assert len(books) == 2 + +# OPTION 2 - all in one +books = ( + await Book.objects.select_related("author") + .filter( + ormar.and_( + ormar.or_(year__gt=1960, year__lt=1940), + author__name="J.R.R. Tolkien", + ) + ) + .all() +) + +assert len(books) == 2 +assert books[0].title == "The Hobbit" +assert books[1].title == "The Silmarillion" +``` + +Books of Sapkowski from before 2000 or books of Tolkien written after 1960 + +sql: +`WHERE ( ( books.year > 1960 AND authors.name = 'J.R.R. Tolkien' ) OR ( books.year < 2000 AND authors.name = 'Andrzej Sapkowski' ) ) ` + +```python +books = ( + await Book.objects.select_related("author") + .filter( + ormar.or_( + ormar.and_(year__gt=1960, author__name="J.R.R. Tolkien"), + ormar.and_(year__lt=2000, author__name="Andrzej Sapkowski"), + ) + ) + .all() +) +assert len(books) == 2 +``` + +Of course those functions can have more than 2 conditions, so if we for example want also +books that contains 'hobbit': + +sql: +`WHERE ( ( books.year > 1960 AND authors.name = 'J.R.R. Tolkien' ) OR +( books.year < 2000 AND os0cec_authors.name = 'Andrzej Sapkowski' ) OR +books.title LIKE '%hobbit%' )` + +```python +books = ( + await Book.objects.select_related("author") + .filter( + ormar.or_( + ormar.and_(year__gt=1960, author__name="J.R.R. Tolkien"), + ormar.and_(year__lt=2000, author__name="Andrzej Sapkowski"), + title__icontains="hobbit", + ) + ) + .all() +) +``` + +By now you should already have an idea how `ormar.or_` and `ormar.and_` works. +Of course, you could chain them in any other methods of queryset, so in example a perfectly +valid query can look like follows: + +```python +books = ( + await Book.objects.select_related("author") + .filter(ormar.or_(year__gt=1980, author__name="Andrzej Sapkowski")) + .filter(title__startswith="The") + .limit(1) + .offset(1) + .order_by("-id") + .all() +) +assert len(books) == 1 +assert books[0].title == "The Witcher" +``` + + +!!!note + Note that you cannot provide the same keyword argument several times so queries like `filter(ormar.or_(name='Jack', name='John'))` are not allowed. If you want to check the same + column for several values simply use `in` operator: `filter(name__in=['Jack','John'])`. + Note that also that technically you can still do `filter(ormar.or_(name='Jack', name__exact='John'))` + but it's not recommended. The different operators can be used as long as they do not + repeat so `filter(ormar.or_(year__lt=1560, year__gt=2000))` is fine. + ## get `get(**kwargs) -> Model` diff --git a/docs/releases.md b/docs/releases.md index dca68fc..0b06a1b 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -6,7 +6,22 @@ album__name__isnull=True #(sql: album.name is null) album__name__isnull=False #(sql: album.name is not null)) ``` - +* Add `ormar.or_` and `ormar.and_` functions that can be used to compose + complex queries with nested conditions. + Sample query: + ```python + books = ( + await Book.objects.select_related("author") + .filter( + ormar.and_( + ormar.or_(year__gt=1960, year__lt=1940), + author__name="J.R.R. Tolkien", + ) + ) + .all() + ) + ``` + Check the updated docs in Queries -> Filtering and sorting -> Complex filters # 0.9.6 diff --git a/tests/test_or_filters.py b/tests/test_or_filters.py index 0e24384..8568715 100644 --- a/tests/test_or_filters.py +++ b/tests/test_or_filters.py @@ -80,6 +80,21 @@ async def test_or_filters(): assert books[0].title == "The Hobbit" assert books[1].title == "The Silmarillion" + books = ( + await Book.objects.select_related("author") + .filter( + ormar.and_( + ormar.or_(year__gt=1960, year__lt=1940), + author__name="J.R.R. Tolkien", + ) + ) + .all() + ) + + assert len(books) == 2 + assert books[0].title == "The Hobbit" + assert books[1].title == "The Silmarillion" + books = ( await Book.objects.select_related("author") .filter( @@ -109,6 +124,23 @@ async def test_or_filters(): assert len(books) == 3 assert not any([x.title in ["The Silmarillion", "The Witcher"] for x in books]) + books = ( + await Book.objects.select_related("author") + .filter( + ormar.or_( + ormar.and_(year__gt=1960, author__name="J.R.R. Tolkien"), + ormar.and_(year__lt=2000, author__name="Andrzej Sapkowski"), + title__icontains="hobbit", + ) + ) + .filter(title__startswith="The") + .all() + ) + assert len(books) == 3 + assert not any( + [x.title in ["The Tower of Fools", "The Lord of the Rings"] for x in books] + ) + books = ( await Book.objects.select_related("author") .filter(ormar.or_(year__gt=1980, year__lt=1910)) @@ -163,5 +195,6 @@ async def test_or_filters(): # fix limit -> change to where subquery to extract number of distinct pk values (V) # finish docstrings (V) # fix types for FilterAction and FilterGroup (X) +# add docs (V) -# add docs +# fix querysetproxy From 0ea9b0952c2e9b472069cbd5bf321f67ca965d05 Mon Sep 17 00:00:00 2001 From: collerek Date: Tue, 9 Mar 2021 10:13:51 +0100 Subject: [PATCH 15/16] add default exceptions to relations, test one argument, test querysetproxy, test deeply nested --- .codeclimate.yml | 3 + README.md | 10 +-- docs/index.md | 10 +-- docs/queries/filter-and-sort.md | 76 ++++++++++++++++++++++- docs/relations/queryset-proxy.md | 4 +- docs/releases.md | 3 + ormar/fields/foreign_key.py | 7 ++- ormar/fields/many_to_many.py | 6 ++ ormar/relations/querysetproxy.py | 8 +-- tests/test_or_filters.py | 53 +++++++++++++++- tests/test_relations_default_exception.py | 65 +++++++++++++++++++ 11 files changed, 223 insertions(+), 22 deletions(-) create mode 100644 tests/test_relations_default_exception.py diff --git a/.codeclimate.yml b/.codeclimate.yml index e893c57..4872210 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -3,6 +3,9 @@ checks: method-complexity: config: threshold: 8 + argument-count: + config: + threshold: 6 file-lines: config: threshold: 500 diff --git a/README.md b/README.md index d4655e5..fd526bd 100644 --- a/README.md +++ b/README.md @@ -437,8 +437,8 @@ metadata.drop_all(engine) * `bulk_update(objects: List[Model], columns: List[str] = None) -> None` * `delete(each: bool = False, **kwargs) -> int` * `all(**kwargs) -> List[Optional[Model]]` -* `filter(**kwargs) -> QuerySet` -* `exclude(**kwargs) -> QuerySet` +* `filter(*args, **kwargs) -> QuerySet` +* `exclude(*args, **kwargs) -> QuerySet` * `select_related(related: Union[List, str]) -> QuerySet` * `prefetch_related(related: Union[List, str]) -> QuerySet` * `limit(limit_count: int) -> QuerySet` @@ -453,7 +453,7 @@ metadata.drop_all(engine) #### Relation types * One to many - with `ForeignKey(to: Model)` -* Many to many - with `ManyToMany(to: Model, through: Model)` +* Many to many - with `ManyToMany(to: Model, Optional[through]: Model)` #### Model fields types @@ -491,8 +491,8 @@ The following keyword arguments are supported on all field types. All fields are required unless one of the following is set: * `nullable` - Creates a nullable column. Sets the default to `None`. - * `default` - Set a default value for the field. - * `server_default` - Set a default value for the field on server side (like sqlalchemy's `func.now()`). + * `default` - Set a default value for the field. **Not available for relation fields** + * `server_default` - Set a default value for the field on server side (like sqlalchemy's `func.now()`). **Not available for relation fields** * `primary key` with `autoincrement` - When a column is set to primary key and autoincrement is set on this column. Autoincrement is set by default on int primary keys. * `pydantic_only` - Field is available only as normal pydantic field, not stored in the database. diff --git a/docs/index.md b/docs/index.md index d4655e5..fd526bd 100644 --- a/docs/index.md +++ b/docs/index.md @@ -437,8 +437,8 @@ metadata.drop_all(engine) * `bulk_update(objects: List[Model], columns: List[str] = None) -> None` * `delete(each: bool = False, **kwargs) -> int` * `all(**kwargs) -> List[Optional[Model]]` -* `filter(**kwargs) -> QuerySet` -* `exclude(**kwargs) -> QuerySet` +* `filter(*args, **kwargs) -> QuerySet` +* `exclude(*args, **kwargs) -> QuerySet` * `select_related(related: Union[List, str]) -> QuerySet` * `prefetch_related(related: Union[List, str]) -> QuerySet` * `limit(limit_count: int) -> QuerySet` @@ -453,7 +453,7 @@ metadata.drop_all(engine) #### Relation types * One to many - with `ForeignKey(to: Model)` -* Many to many - with `ManyToMany(to: Model, through: Model)` +* Many to many - with `ManyToMany(to: Model, Optional[through]: Model)` #### Model fields types @@ -491,8 +491,8 @@ The following keyword arguments are supported on all field types. All fields are required unless one of the following is set: * `nullable` - Creates a nullable column. Sets the default to `None`. - * `default` - Set a default value for the field. - * `server_default` - Set a default value for the field on server side (like sqlalchemy's `func.now()`). + * `default` - Set a default value for the field. **Not available for relation fields** + * `server_default` - Set a default value for the field on server side (like sqlalchemy's `func.now()`). **Not available for relation fields** * `primary key` with `autoincrement` - When a column is set to primary key and autoincrement is set on this column. Autoincrement is set by default on int primary keys. * `pydantic_only` - Field is available only as normal pydantic field, not stored in the database. diff --git a/docs/queries/filter-and-sort.md b/docs/queries/filter-and-sort.md index ed51f85..a2b74e9 100644 --- a/docs/queries/filter-and-sort.md +++ b/docs/queries/filter-and-sort.md @@ -288,6 +288,37 @@ books = ( ) ``` +If you want or need to you can nest deeper conditions as deep as you want, in example to +acheive a query like this: + +sql: +``` +WHERE ( ( ( books.year > 1960 OR books.year < 1940 ) +AND authors.name = 'J.R.R. Tolkien' ) OR +( books.year < 2000 AND authors.name = 'Andrzej Sapkowski' ) ) +``` + +You can construct a query as follows: +```python +books = ( + await Book.objects.select_related("author") + .filter( + ormar.or_( + ormar.and_( + ormar.or_(year__gt=1960, year__lt=1940), + author__name="J.R.R. Tolkien", + ), + ormar.and_(year__lt=2000, author__name="Andrzej Sapkowski"), + ) + ) + .all() +) +assert len(books) == 3 +assert books[0].title == "The Hobbit" +assert books[1].title == "The Silmarillion" +assert books[2].title == "The Witcher" +``` + By now you should already have an idea how `ormar.or_` and `ormar.and_` works. Of course, you could chain them in any other methods of queryset, so in example a perfectly valid query can look like follows: @@ -310,9 +341,48 @@ assert books[0].title == "The Witcher" !!!note Note that you cannot provide the same keyword argument several times so queries like `filter(ormar.or_(name='Jack', name='John'))` are not allowed. If you want to check the same column for several values simply use `in` operator: `filter(name__in=['Jack','John'])`. - Note that also that technically you can still do `filter(ormar.or_(name='Jack', name__exact='John'))` - but it's not recommended. The different operators can be used as long as they do not - repeat so `filter(ormar.or_(year__lt=1560, year__gt=2000))` is fine. + +If you pass only one parameter to `or_` or `and_` functions it's simply wrapped in parenthesis and +has no effect on actual query, so in the end all 3 queries are identical: + +```python +await Book.objects.filter(title='The Hobbit').get() +await Book.objects.filter(ormar.or_(title='The Hobbit')).get() +await Book.objects.filter(ormar.and_(title='The Hobbit')).get() +``` + +!!!note + Note that `or_` and `and_` queries will have `WHERE (title='The Hobbit')` but the parenthesis is redundant and has no real effect. + +This feature can be used if you **really** need to use the same field name twice. +Remember that you cannot pass the same keyword arguments twice to the function, so +how you can query in example `WHERE (authors.name LIKE '%tolkien%') OR (authors.name LIKE '%sapkowski%'))`? + +You cannot do: +```python +books = ( + await Book.objects.select_related("author") + .filter(ormar.or_( + author__name__icontains="tolkien", + author__name__icontains="sapkowski" # you cannot use same keyword twice in or_! + )) # python syntax error + .all() +) +``` + +But you can do this: + +```python +books = ( + await Book.objects.select_related("author") + .filter(ormar.or_( + ormar.and_(author__name__icontains="tolkien"), # one argument == just wrapped in () + ormar.and_(author__name__icontains="sapkowski") + )) + .all() +) +assert len(books) == 5 +``` ## get diff --git a/docs/relations/queryset-proxy.md b/docs/relations/queryset-proxy.md index db1343d..fc22d75 100644 --- a/docs/relations/queryset-proxy.md +++ b/docs/relations/queryset-proxy.md @@ -172,7 +172,7 @@ await post.categories.filter(name="Test category3").update( ### filter -`filter(**kwargs) -> QuerySet` +`filter(*args, **kwargs) -> QuerySet` Allows you to filter by any Model attribute/field as well as to fetch instances, with a filter across an FK relationship. @@ -181,7 +181,7 @@ Allows you to filter by any Model attribute/field as well as to fetch instances, ### exclude -`exclude(**kwargs) -> QuerySet` +`exclude(*args, **kwargs) -> QuerySet` Works exactly the same as filter and all modifiers (suffixes) are the same, but returns a not condition. diff --git a/docs/releases.md b/docs/releases.md index 0b06a1b..2e85e43 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -23,6 +23,9 @@ ``` Check the updated docs in Queries -> Filtering and sorting -> Complex filters +## Other +* Setting default on `ForeignKey` or `ManyToMany` raises and `ModelDefinition` exception as it is (and was) not supported + # 0.9.6 ##Important diff --git a/ormar/fields/foreign_key.py b/ormar/fields/foreign_key.py index 3c8d18b..7f1a500 100644 --- a/ormar/fields/foreign_key.py +++ b/ormar/fields/foreign_key.py @@ -11,7 +11,7 @@ from pydantic.typing import ForwardRef, evaluate_forwardref from sqlalchemy import UniqueConstraint import ormar # noqa I101 -from ormar.exceptions import RelationshipInstanceError +from ormar.exceptions import ModelDefinitionError, RelationshipInstanceError from ormar.fields.base import BaseField if TYPE_CHECKING: # pragma no cover @@ -184,6 +184,11 @@ def ForeignKey( # noqa CFQ002 owner = kwargs.pop("owner", None) self_reference = kwargs.pop("self_reference", False) + default = kwargs.pop("default", None) + if default is not None: + raise ModelDefinitionError( + "Argument 'default' is not supported " "on relation fields!" + ) if to.__class__ == ForwardRef: __type__ = to if not nullable else Optional[to] diff --git a/ormar/fields/many_to_many.py b/ormar/fields/many_to_many.py index ec8a6f1..db763e3 100644 --- a/ormar/fields/many_to_many.py +++ b/ormar/fields/many_to_many.py @@ -96,6 +96,12 @@ def ManyToMany( if through is not None and through.__class__ != ForwardRef: forbid_through_relations(cast(Type["Model"], through)) + default = kwargs.pop("default", None) + if default is not None: + raise ModelDefinitionError( + "Argument 'default' is not supported " "on relation fields!" + ) + if to.__class__ == ForwardRef: __type__ = to if not nullable else Optional[to] column_type = None diff --git a/ormar/relations/querysetproxy.py b/ormar/relations/querysetproxy.py index 69a0f54..d90776a 100644 --- a/ormar/relations/querysetproxy.py +++ b/ormar/relations/querysetproxy.py @@ -374,7 +374,7 @@ class QuerysetProxy: model = await self.queryset.get(pk=kwargs[pk_name]) return await model.update(**kwargs) - def filter(self, **kwargs: Any) -> "QuerysetProxy": # noqa: A003, A001 + def filter(self, *args: Any, **kwargs: Any) -> "QuerysetProxy": # noqa: A003, A001 """ Allows you to filter by any `Model` attribute/field as well as to fetch instances, with a filter across an FK relationship. @@ -404,10 +404,10 @@ class QuerysetProxy: :return: filtered QuerysetProxy :rtype: QuerysetProxy """ - queryset = self.queryset.filter(**kwargs) + queryset = self.queryset.filter(*args, **kwargs) return self.__class__(relation=self.relation, type_=self.type_, qryset=queryset) - def exclude(self, **kwargs: Any) -> "QuerysetProxy": # noqa: A003, A001 + def exclude(self, *args: Any, **kwargs: Any) -> "QuerysetProxy": # noqa: A003, A001 """ Works exactly the same as filter and all modifiers (suffixes) are the same, but returns a *not* condition. @@ -428,7 +428,7 @@ class QuerysetProxy: :return: filtered QuerysetProxy :rtype: QuerysetProxy """ - queryset = self.queryset.exclude(**kwargs) + queryset = self.queryset.exclude(*args, **kwargs) return self.__class__(relation=self.relation, type_=self.type_, qryset=queryset) def select_related(self, related: Union[List, str]) -> "QuerysetProxy": diff --git a/tests/test_or_filters.py b/tests/test_or_filters.py index 8568715..1a483cc 100644 --- a/tests/test_or_filters.py +++ b/tests/test_or_filters.py @@ -110,6 +110,24 @@ async def test_or_filters(): assert books[0].title == "The Silmarillion" assert books[1].title == "The Witcher" + books = ( + await Book.objects.select_related("author") + .filter( + ormar.or_( + ormar.and_( + ormar.or_(year__gt=1960, year__lt=1940), + author__name="J.R.R. Tolkien", + ), + ormar.and_(year__lt=2000, author__name="Andrzej Sapkowski"), + ) + ) + .all() + ) + assert len(books) == 3 + assert books[0].title == "The Hobbit" + assert books[1].title == "The Silmarillion" + assert books[2].title == "The Witcher" + books = ( await Book.objects.select_related("author") .exclude( @@ -187,6 +205,38 @@ async def test_or_filters(): with pytest.raises(QueryDefinitionError): await Book.objects.select_related("author").filter("wrong").all() + books = await tolkien.books.filter( + ormar.or_(year__lt=1940, year__gt=1960) + ).all() + assert len(books) == 2 + + books = await tolkien.books.filter( + ormar.and_( + ormar.or_(year__lt=1940, year__gt=1960), title__icontains="hobbit" + ) + ).all() + assert len(books) == 1 + assert tolkien.books[0].title == "The Hobbit" + + books = ( + await Book.objects.select_related("author") + .filter(ormar.or_(author__name="J.R.R. Tolkien")) + .all() + ) + assert len(books) == 3 + + books = ( + await Book.objects.select_related("author") + .filter( + ormar.or_( + ormar.and_(author__name__icontains="tolkien"), + ormar.and_(author__name__icontains="sapkowski"), + ) + ) + .all() + ) + assert len(books) == 5 + # TODO: Check / modify # process and and or into filter groups (V) @@ -196,5 +246,4 @@ async def test_or_filters(): # finish docstrings (V) # fix types for FilterAction and FilterGroup (X) # add docs (V) - -# fix querysetproxy +# fix querysetproxy (V) diff --git a/tests/test_relations_default_exception.py b/tests/test_relations_default_exception.py new file mode 100644 index 0000000..1573e13 --- /dev/null +++ b/tests/test_relations_default_exception.py @@ -0,0 +1,65 @@ +# type: ignore +from typing import List, Optional + +import databases +import pytest +import sqlalchemy + +import ormar +from ormar.exceptions import ModelDefinitionError +from tests.settings import DATABASE_URL + +database = databases.Database(DATABASE_URL, force_rollback=True) +metadata = sqlalchemy.MetaData() + + +class Author(ormar.Model): + class Meta: + tablename = "authors" + database = database + metadata = metadata + + id: int = ormar.Integer(primary_key=True) + first_name: str = ormar.String(max_length=80) + last_name: str = ormar.String(max_length=80) + + +class Category(ormar.Model): + class Meta: + tablename = "categories" + database = database + metadata = metadata + + id: int = ormar.Integer(primary_key=True) + name: str = ormar.String(max_length=40) + + +def test_fk_error(): + with pytest.raises(ModelDefinitionError): + + class Post(ormar.Model): + class Meta: + tablename = "posts" + database = database + metadata = metadata + + id: int = ormar.Integer(primary_key=True) + title: str = ormar.String(max_length=200) + categories: Optional[List[Category]] = ormar.ManyToMany(Category) + author: Optional[Author] = ormar.ForeignKey(Author, default="aa") + + +def test_m2m_error(): + with pytest.raises(ModelDefinitionError): + + class Post(ormar.Model): + class Meta: + tablename = "posts" + database = database + metadata = metadata + + id: int = ormar.Integer(primary_key=True) + title: str = ormar.String(max_length=200) + categories: Optional[List[Category]] = ormar.ManyToMany( + Category, default="aa" + ) From fdb6c3089f8d0d92e91e651b1a866015bc58ce79 Mon Sep 17 00:00:00 2001 From: collerek Date: Tue, 9 Mar 2021 10:27:20 +0100 Subject: [PATCH 16/16] fix minor code smells --- ormar/queryset/actions/order_action.py | 3 +- tests/test_or_filters.py | 97 ++++++++++++-------------- 2 files changed, 44 insertions(+), 56 deletions(-) diff --git a/ormar/queryset/actions/order_action.py b/ormar/queryset/actions/order_action.py index 2bba2d2..9d5f248 100644 --- a/ormar/queryset/actions/order_action.py +++ b/ormar/queryset/actions/order_action.py @@ -57,8 +57,7 @@ class OrderAction(QueryAction): prefix = f"{self.table_prefix}_" if self.table_prefix else "" if self.direction == "": return text(f"min({prefix}{self.table}" f".{self.field_alias})") - else: - return text(f"max({prefix}{self.table}" f".{self.field_alias}) desc") + return text(f"max({prefix}{self.table}" f".{self.field_alias}) desc") def get_text_clause(self) -> sqlalchemy.sql.expression.TextClause: """ diff --git a/tests/test_or_filters.py b/tests/test_or_filters.py index 1a483cc..6d6f8ab 100644 --- a/tests/test_or_filters.py +++ b/tests/test_or_filters.py @@ -57,24 +57,24 @@ async def test_or_filters(): books = ( await Book.objects.select_related("author") - .filter(ormar.or_(author__name="J.R.R. Tolkien", year__gt=1970)) - .all() + .filter(ormar.or_(author__name="J.R.R. Tolkien", year__gt=1970)) + .all() ) assert len(books) == 5 books = ( await Book.objects.select_related("author") - .filter(ormar.or_(author__name="J.R.R. Tolkien", year__lt=1995)) - .all() + .filter(ormar.or_(author__name="J.R.R. Tolkien", year__lt=1995)) + .all() ) assert len(books) == 4 assert not any([x.title == "The Tower of Fools" for x in books]) books = ( await Book.objects.select_related("author") - .filter(ormar.or_(year__gt=1960, year__lt=1940)) - .filter(author__name="J.R.R. Tolkien") - .all() + .filter(ormar.or_(year__gt=1960, year__lt=1940)) + .filter(author__name="J.R.R. Tolkien") + .all() ) assert len(books) == 2 assert books[0].title == "The Hobbit" @@ -82,13 +82,13 @@ async def test_or_filters(): books = ( await Book.objects.select_related("author") - .filter( + .filter( ormar.and_( ormar.or_(year__gt=1960, year__lt=1940), author__name="J.R.R. Tolkien", ) ) - .all() + .all() ) assert len(books) == 2 @@ -97,14 +97,14 @@ async def test_or_filters(): books = ( await Book.objects.select_related("author") - .filter( + .filter( ormar.or_( ormar.and_(year__gt=1960, author__name="J.R.R. Tolkien"), ormar.and_(year__lt=2000, author__name="Andrzej Sapkowski"), ) ) - .filter(title__startswith="The") - .all() + .filter(title__startswith="The") + .all() ) assert len(books) == 2 assert books[0].title == "The Silmarillion" @@ -112,7 +112,7 @@ async def test_or_filters(): books = ( await Book.objects.select_related("author") - .filter( + .filter( ormar.or_( ormar.and_( ormar.or_(year__gt=1960, year__lt=1940), @@ -121,7 +121,7 @@ async def test_or_filters(): ormar.and_(year__lt=2000, author__name="Andrzej Sapkowski"), ) ) - .all() + .all() ) assert len(books) == 3 assert books[0].title == "The Hobbit" @@ -130,29 +130,29 @@ async def test_or_filters(): books = ( await Book.objects.select_related("author") - .exclude( + .exclude( ormar.or_( ormar.and_(year__gt=1960, author__name="J.R.R. Tolkien"), ormar.and_(year__lt=2000, author__name="Andrzej Sapkowski"), ) ) - .filter(title__startswith="The") - .all() + .filter(title__startswith="The") + .all() ) assert len(books) == 3 assert not any([x.title in ["The Silmarillion", "The Witcher"] for x in books]) books = ( await Book.objects.select_related("author") - .filter( + .filter( ormar.or_( ormar.and_(year__gt=1960, author__name="J.R.R. Tolkien"), ormar.and_(year__lt=2000, author__name="Andrzej Sapkowski"), title__icontains="hobbit", ) ) - .filter(title__startswith="The") - .all() + .filter(title__startswith="The") + .all() ) assert len(books) == 3 assert not any( @@ -161,43 +161,43 @@ async def test_or_filters(): books = ( await Book.objects.select_related("author") - .filter(ormar.or_(year__gt=1980, year__lt=1910)) - .filter(title__startswith="The") - .limit(1) - .all() + .filter(ormar.or_(year__gt=1980, year__lt=1910)) + .filter(title__startswith="The") + .limit(1) + .all() ) assert len(books) == 1 assert books[0].title == "The Witcher" books = ( await Book.objects.select_related("author") - .filter(ormar.or_(year__gt=1980, author__name="Andrzej Sapkowski")) - .filter(title__startswith="The") - .limit(1) - .all() + .filter(ormar.or_(year__gt=1980, author__name="Andrzej Sapkowski")) + .filter(title__startswith="The") + .limit(1) + .all() ) assert len(books) == 1 assert books[0].title == "The Witcher" books = ( await Book.objects.select_related("author") - .filter(ormar.or_(year__gt=1980, author__name="Andrzej Sapkowski")) - .filter(title__startswith="The") - .limit(1) - .offset(1) - .all() + .filter(ormar.or_(year__gt=1980, author__name="Andrzej Sapkowski")) + .filter(title__startswith="The") + .limit(1) + .offset(1) + .all() ) assert len(books) == 1 assert books[0].title == "The Tower of Fools" books = ( await Book.objects.select_related("author") - .filter(ormar.or_(year__gt=1980, author__name="Andrzej Sapkowski")) - .filter(title__startswith="The") - .limit(1) - .offset(1) - .order_by("-id") - .all() + .filter(ormar.or_(year__gt=1980, author__name="Andrzej Sapkowski")) + .filter(title__startswith="The") + .limit(1) + .offset(1) + .order_by("-id") + .all() ) assert len(books) == 1 assert books[0].title == "The Witcher" @@ -220,30 +220,19 @@ async def test_or_filters(): books = ( await Book.objects.select_related("author") - .filter(ormar.or_(author__name="J.R.R. Tolkien")) - .all() + .filter(ormar.or_(author__name="J.R.R. Tolkien")) + .all() ) assert len(books) == 3 books = ( await Book.objects.select_related("author") - .filter( + .filter( ormar.or_( ormar.and_(author__name__icontains="tolkien"), ormar.and_(author__name__icontains="sapkowski"), ) ) - .all() + .all() ) assert len(books) == 5 - - -# TODO: Check / modify -# process and and or into filter groups (V) -# check exclude queries working (V) -# check complex prefixes properly resolved (V) -# fix limit -> change to where subquery to extract number of distinct pk values (V) -# finish docstrings (V) -# fix types for FilterAction and FilterGroup (X) -# add docs (V) -# fix querysetproxy (V)