From 9f4bde595f56a354a0abab72538503ad483ee93a Mon Sep 17 00:00:00 2001 From: collerek Date: Sun, 8 Nov 2020 09:44:30 +0100 Subject: [PATCH 1/9] working for simple models, not including related models yet --- ormar/queryset/query.py | 13 ++++- ormar/queryset/queryset.py | 27 ++++++++- tests/test_order_by.py | 117 +++++++++++++++++++++++++++++++++++++ 3 files changed, 154 insertions(+), 3 deletions(-) create mode 100644 tests/test_order_by.py diff --git a/ormar/queryset/query.py b/ormar/queryset/query.py index 880b3c1..31be3f9 100644 --- a/ormar/queryset/query.py +++ b/ormar/queryset/query.py @@ -21,6 +21,7 @@ class Query: limit_count: Optional[int], offset: Optional[int], fields: Optional[List], + order_bys: Optional[List], ) -> None: self.query_offset = offset self.limit_count = limit_count @@ -36,6 +37,7 @@ class Query: self.select_from: List[str] = [] self.columns = [sqlalchemy.Column] + self.order_columns = order_bys self.order_bys: List[sqlalchemy.sql.elements.TextClause] = [] @property @@ -43,6 +45,15 @@ class Query: pkname_alias = self.model_cls.get_column_alias(self.model_cls.Meta.pkname) return f"{self.table.name}.{pkname_alias}" + def apply_order_bys_for_primary_model(self): + if self.order_columns: + return [ + text(f"{x[1:]} desc") if x.startswith("-") else text(x) + for x in self.order_columns + if "__" not in x + ] + return [text(self.prefixed_pk_name)] + def build_select_expression(self) -> Tuple[sqlalchemy.sql.select, List[str]]: self_related_fields = self.model_cls.own_table_columns( self.model_cls, self.fields @@ -50,7 +61,7 @@ class Query: self.columns = self.model_cls.Meta.alias_manager.prefixed_columns( "", self.table, self_related_fields ) - self.order_bys = [text(self.prefixed_pk_name)] + self.order_bys = self.apply_order_bys_for_primary_model() self.select_from = self.table self._select_related.sort(key=lambda item: (item, -len(item))) diff --git a/ormar/queryset/queryset.py b/ormar/queryset/queryset.py index 749eb75..4991403 100644 --- a/ormar/queryset/queryset.py +++ b/ormar/queryset/queryset.py @@ -27,6 +27,7 @@ class QuerySet: limit_count: int = None, offset: int = None, columns: List = None, + order_bys: List = None, ) -> None: self.model_cls = model_cls self.filter_clauses = [] if filter_clauses is None else filter_clauses @@ -35,7 +36,7 @@ class QuerySet: self.limit_count = limit_count self.query_offset = offset self._columns = columns or [] - self.order_bys = None + self.order_bys = order_bys or [] def __get__( self, @@ -110,9 +111,10 @@ class QuerySet: offset=self.query_offset, limit_count=self.limit_count, fields=self._columns, + order_bys=self.order_bys, ) exp = qry.build_select_expression() - # print(exp.compile(compile_kwargs={"literal_binds": True})) + print(exp.compile(compile_kwargs={"literal_binds": True})) return exp def filter(self, _exclude: bool = False, **kwargs: Any) -> "QuerySet": # noqa: A003 @@ -137,6 +139,7 @@ class QuerySet: limit_count=self.limit_count, offset=self.query_offset, columns=self._columns, + order_bys=self.order_bys, ) def exclude(self, **kwargs: Any) -> "QuerySet": # noqa: A003 @@ -155,6 +158,7 @@ class QuerySet: limit_count=self.limit_count, offset=self.query_offset, columns=self._columns, + order_bys=self.order_bys, ) def fields(self, columns: Union[List, str]) -> "QuerySet": @@ -170,6 +174,23 @@ class QuerySet: limit_count=self.limit_count, offset=self.query_offset, columns=columns, + order_bys=self.order_bys, + ) + + def order_by(self, columns: Union[List, str]) -> "QuerySet": + if not isinstance(columns, list): + columns = [columns] + + order_bys = self.order_bys + [x for x in columns if x not in self.order_bys] + return self.__class__( + model_cls=self.model, + filter_clauses=self.filter_clauses, + exclude_clauses=self.exclude_clauses, + select_related=self._select_related, + limit_count=self.limit_count, + offset=self.query_offset, + columns=self._columns, + order_bys=order_bys, ) async def exists(self) -> bool: @@ -218,6 +239,7 @@ class QuerySet: limit_count=limit_count, offset=self.query_offset, columns=self._columns, + order_bys=self.order_bys, ) def offset(self, offset: int) -> "QuerySet": @@ -229,6 +251,7 @@ class QuerySet: limit_count=self.limit_count, offset=offset, columns=self._columns, + order_bys=self.order_bys, ) async def first(self, **kwargs: Any) -> "Model": diff --git a/tests/test_order_by.py b/tests/test_order_by.py new file mode 100644 index 0000000..8049de1 --- /dev/null +++ b/tests/test_order_by.py @@ -0,0 +1,117 @@ +import databases +import pytest +import sqlalchemy + +import ormar +from tests.settings import DATABASE_URL + +database = databases.Database(DATABASE_URL, force_rollback=True) +metadata = sqlalchemy.MetaData() + + +class Song(ormar.Model): + class Meta: + tablename = "songs" + metadata = metadata + database = database + + id: int = ormar.Integer(primary_key=True) + name: str = ormar.String(max_length=100) + sort_order: int = ormar.Integer() + + +class Owner(ormar.Model): + class Meta: + tablename = "owners" + metadata = metadata + database = database + + id: int = ormar.Integer(primary_key=True) + name: str = ormar.String(max_length=100) + + +class Toy(ormar.Model): + class Meta: + tablename = "toys" + metadata = metadata + database = database + + id: int = ormar.Integer(primary_key=True) + name: str = ormar.String(max_length=100) + owner: Owner = ormar.ForeignKey(Owner) + + +@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_sort_order_on_main_model(): + async with database: + await Song.objects.create(name="Song 3", sort_order=3) + await Song.objects.create(name="Song 1", sort_order=1) + await Song.objects.create(name="Song 2", sort_order=2) + + songs = await Song.objects.all() + assert songs[0].name == "Song 3" + assert songs[1].name == "Song 1" + assert songs[2].name == "Song 2" + + songs = await Song.objects.order_by("-sort_order").all() + assert songs[0].name == "Song 3" + assert songs[1].name == "Song 2" + assert songs[2].name == "Song 1" + + songs = await Song.objects.order_by("sort_order").all() + assert songs[0].name == "Song 1" + assert songs[1].name == "Song 2" + assert songs[2].name == "Song 3" + + songs = await Song.objects.order_by("name").all() + assert songs[0].name == "Song 1" + assert songs[1].name == "Song 2" + assert songs[2].name == "Song 3" + + await Song.objects.create(name="Song 4", sort_order=1) + + songs = await Song.objects.order_by(["sort_order", "name"]).all() + assert songs[0].name == "Song 1" + assert songs[1].name == "Song 4" + assert songs[2].name == "Song 2" + assert songs[3].name == "Song 3" + + +@pytest.mark.asyncio +async def test_sort_order_on_related_model(): + async with database: + aphrodite = await Owner.objects.create(name="Aphrodite") + hermes = await Owner.objects.create(name="Hermes") + zeus = await Owner.objects.create(name="Zeus") + + await Toy.objects.create(name="Toy 1", owner=zeus) + await Toy.objects.create(name="Toy 5", owner=hermes) + await Toy.objects.create(name="Toy 2", owner=aphrodite) + await Toy.objects.create(name="Toy 4", owner=zeus) + await Toy.objects.create(name="Toy 3", owner=aphrodite) + await Toy.objects.create(name="Toy 6", owner=hermes) + + toys = await Toy.objects.select_related("owner").order_by("name").all() + assert [x.name.replace("Toy ", "") for x in toys] == [ + str(x + 1) for x in range(6) + ] + assert toys[0].owner == zeus + assert toys[1].owner == aphrodite + + toys = await Toy.objects.select_related("owner").order_by("owner__name").all() + + owner = ( + await Owner.objects.select_related("toys") + .order_by("toys__name") + .filter(name="Zeus") + .all() + ) From ca38f7853eb9333773c3d450aa8b8b1d5e5bce08 Mon Sep 17 00:00:00 2001 From: collerek Date: Mon, 9 Nov 2020 12:45:32 +0100 Subject: [PATCH 2/9] working order by, refactor models merging, refactors ordering into dict --- ormar/models/model.py | 5 +- ormar/models/modelproxy.py | 18 ++-- ormar/queryset/join.py | 58 ++++++++++-- ormar/queryset/order_query.py | 13 +-- ormar/queryset/query.py | 36 +++++--- ormar/queryset/queryset.py | 2 +- tests/test_order_by.py | 164 +++++++++++++++++++++++++++++++++- 7 files changed, 259 insertions(+), 37 deletions(-) diff --git a/ormar/models/model.py b/ormar/models/model.py index a086a3b..d2bf904 100644 --- a/ormar/models/model.py +++ b/ormar/models/model.py @@ -37,10 +37,7 @@ class Model(NewBaseModel): objects: "QuerySet" def __repr__(self) -> str: # pragma nocover - attrs_to_include = ["tablename", "columns", "pkname"] - _repr = {k: v for k, v in self.Meta.model_fields.items()} - for atr in attrs_to_include: - _repr[atr] = getattr(self.Meta, atr) + _repr = {k: getattr(self, k) for k, v in self.Meta.model_fields.items()} return f"{self.__class__.__name__}({str(_repr)})" @classmethod diff --git a/ormar/models/modelproxy.py b/ormar/models/modelproxy.py index fb0e789..ae96010 100644 --- a/ormar/models/modelproxy.py +++ b/ormar/models/modelproxy.py @@ -1,4 +1,5 @@ import inspect +from collections import OrderedDict from typing import Dict, List, Sequence, Set, TYPE_CHECKING, Type, TypeVar, Union import ormar @@ -181,11 +182,18 @@ class ModelTableProxy: @classmethod def merge_instances_list(cls, result_rows: Sequence["Model"]) -> Sequence["Model"]: merged_rows: List["Model"] = [] - for index, model in enumerate(result_rows): - if index > 0 and model is not None and model.pk == merged_rows[-1].pk: - merged_rows[-1] = cls.merge_two_instances(model, merged_rows[-1]) - else: - merged_rows.append(model) + grouped_instances = OrderedDict() + + for model in result_rows: + grouped_instances.setdefault(model.pk, []).append(model) + + for group in grouped_instances.values(): + model = group.pop(0) + if group: + for next_model in group: + model = cls.merge_two_instances(next_model, model) + merged_rows.append(model) + return merged_rows @classmethod diff --git a/ormar/queryset/join.py b/ormar/queryset/join.py index c045e16..486ed66 100644 --- a/ormar/queryset/join.py +++ b/ormar/queryset/join.py @@ -1,4 +1,4 @@ -from typing import List, NamedTuple, TYPE_CHECKING, Tuple, Type +from typing import Dict, List, NamedTuple, TYPE_CHECKING, Tuple, Type import sqlalchemy from sqlalchemy import text @@ -18,19 +18,21 @@ class JoinParameters(NamedTuple): class SqlJoin: - def __init__( + def __init__( # noqa: CFQ002 self, used_aliases: List, select_from: sqlalchemy.sql.select, - order_bys: List[sqlalchemy.sql.elements.TextClause], columns: List[sqlalchemy.Column], fields: List, + order_columns: List, + sorted_orders: Dict, ) -> None: self.used_aliases = used_aliases self.select_from = select_from - self.order_bys = order_bys self.columns = columns self.fields = fields + self.order_columns = order_columns + self.sorted_orders = sorted_orders @staticmethod def relation_manager(model_cls: Type["Model"]) -> AliasManager: @@ -46,20 +48,26 @@ class SqlJoin: def build_join( self, item: str, join_parameters: JoinParameters - ) -> Tuple[List, sqlalchemy.sql.select, List, List]: + ) -> Tuple[List, sqlalchemy.sql.select, List, Dict]: for part in item.split("__"): if issubclass( join_parameters.model_cls.Meta.model_fields[part], ManyToManyField ): _fields = join_parameters.model_cls.Meta.model_fields new_part = _fields[part].to.get_name() + self._switch_many_to_many_order_columns(part, new_part) join_parameters = self._build_join_parameters( part, join_parameters, is_multi=True ) part = new_part join_parameters = self._build_join_parameters(part, join_parameters) - return self.used_aliases, self.select_from, self.columns, self.order_bys + return ( + self.used_aliases, + self.select_from, + self.columns, + self.sorted_orders, + ) def _build_join_parameters( self, part: str, join_params: JoinParameters, is_multi: bool = False @@ -108,7 +116,9 @@ class SqlJoin: ) pkname_alias = model_cls.get_column_alias(model_cls.Meta.pkname) - self.order_bys.append(text(f"{alias}_{to_table}.{pkname_alias}")) + if not is_multi: + self.get_order_bys(alias, to_table, pkname_alias, part) + self_related_fields = model_cls.own_table_columns( model_cls, self.fields, nested=True, ) @@ -119,6 +129,40 @@ class SqlJoin: ) self.used_aliases.append(alias) + def _switch_many_to_many_order_columns(self, part: str, new_part: str) -> None: + if self.order_columns: + split_order_columns = [ + x.split("__") for x in self.order_columns if "__" in x + ] + for condition in split_order_columns: + if condition[-2] == part or condition[-2][1:] == part: + condition[-2] = condition[-2].replace(part, new_part) + self.order_columns = [x for x in self.order_columns if "__" not in x] + [ + "__".join(x) for x in split_order_columns + ] + + @staticmethod + def _check_if_condition_apply(condition: List, part: str) -> bool: + return len(condition) >= 2 and ( + condition[-2] == part or condition[-2][1:] == part + ) + + def get_order_bys( + self, alias: str, to_table: str, pkname_alias: str, part: str + ) -> None: + if self.order_columns: + split_order_columns = [ + x.split("__") for x in self.order_columns if "__" in x + ] + for condition in split_order_columns: + if self._check_if_condition_apply(condition, part): + direction = f"{'desc' if condition[0][0] == '-' else ''}" + order = text(f"{alias}_{to_table}.{condition[-1]} {direction}") + self.sorted_orders["__".join(condition)] = order + else: + order = text(f"{alias}_{to_table}.{pkname_alias}") + self.sorted_orders[pkname_alias] = order + @staticmethod def get_to_and_from_keys( join_params: JoinParameters, diff --git a/ormar/queryset/order_query.py b/ormar/queryset/order_query.py index cbb4a81..5d4964f 100644 --- a/ormar/queryset/order_query.py +++ b/ormar/queryset/order_query.py @@ -1,14 +1,15 @@ -from typing import List +from typing import Dict import sqlalchemy class OrderQuery: - def __init__(self, order_bys: List) -> None: - self.order_bys = order_bys + def __init__(self, sorted_orders: Dict) -> None: + self.sorted_orders = sorted_orders def apply(self, expr: sqlalchemy.sql.select) -> sqlalchemy.sql.select: - if self.order_bys: - for order in self.order_bys: - expr = expr.order_by(order) + if self.sorted_orders: + for order in list(self.sorted_orders.values()): + if order is not None: + expr = expr.order_by(order) return expr diff --git a/ormar/queryset/query.py b/ormar/queryset/query.py index 31be3f9..a12a89f 100644 --- a/ormar/queryset/query.py +++ b/ormar/queryset/query.py @@ -1,3 +1,4 @@ +from collections import OrderedDict from typing import List, Optional, TYPE_CHECKING, Tuple, Type import sqlalchemy @@ -38,21 +39,31 @@ class Query: self.select_from: List[str] = [] self.columns = [sqlalchemy.Column] self.order_columns = order_bys - self.order_bys: List[sqlalchemy.sql.elements.TextClause] = [] + self.sorted_orders = OrderedDict() + self._init_sorted_orders() + + def _init_sorted_orders(self) -> None: + for clause in self.order_columns: + self.sorted_orders[clause] = None @property def prefixed_pk_name(self) -> str: pkname_alias = self.model_cls.get_column_alias(self.model_cls.Meta.pkname) return f"{self.table.name}.{pkname_alias}" - def apply_order_bys_for_primary_model(self): + def apply_order_bys_for_primary_model(self) -> None: if self.order_columns: - return [ - text(f"{x[1:]} desc") if x.startswith("-") else text(x) - for x in self.order_columns - if "__" not in x - ] - return [text(self.prefixed_pk_name)] + for clause in self.order_columns: + if "__" not in clause: + clause = ( + text(f"{clause[1:]} desc") + if clause.startswith("-") + else text(clause) + ) + self.sorted_orders[clause] = clause + else: + order = text(self.prefixed_pk_name) + self.sorted_orders[self.prefixed_pk_name] = order def build_select_expression(self) -> Tuple[sqlalchemy.sql.select, List[str]]: self_related_fields = self.model_cls.own_table_columns( @@ -61,7 +72,7 @@ class Query: self.columns = self.model_cls.Meta.alias_manager.prefixed_columns( "", self.table, self_related_fields ) - self.order_bys = self.apply_order_bys_for_primary_model() + self.apply_order_bys_for_primary_model() self.select_from = self.table self._select_related.sort(key=lambda item: (item, -len(item))) @@ -75,15 +86,16 @@ class Query: used_aliases=self.used_aliases, select_from=self.select_from, columns=self.columns, - order_bys=self.order_bys, fields=self.fields, + order_columns=self.order_columns, + sorted_orders=self.sorted_orders, ) ( self.used_aliases, self.select_from, self.columns, - self.order_bys, + self.sorted_orders, ) = sql_join.build_join(item, join_parameters) expr = sqlalchemy.sql.select(self.columns) @@ -105,7 +117,7 @@ class Query: ) expr = LimitQuery(limit_count=self.limit_count).apply(expr) expr = OffsetQuery(query_offset=self.query_offset).apply(expr) - expr = OrderQuery(order_bys=self.order_bys).apply(expr) + expr = OrderQuery(sorted_orders=self.sorted_orders).apply(expr) return expr def _reset_query_parameters(self) -> None: diff --git a/ormar/queryset/queryset.py b/ormar/queryset/queryset.py index 4991403..401f7a0 100644 --- a/ormar/queryset/queryset.py +++ b/ormar/queryset/queryset.py @@ -114,7 +114,7 @@ class QuerySet: order_bys=self.order_bys, ) exp = qry.build_select_expression() - print(exp.compile(compile_kwargs={"literal_binds": True})) + # print(exp.compile(compile_kwargs={"literal_binds": True})) return exp def filter(self, _exclude: bool = False, **kwargs: Any) -> "QuerySet": # noqa: A003 diff --git a/tests/test_order_by.py b/tests/test_order_by.py index 8049de1..37ef2dd 100644 --- a/tests/test_order_by.py +++ b/tests/test_order_by.py @@ -1,3 +1,5 @@ +from typing import List, Optional + import databases import pytest import sqlalchemy @@ -41,6 +43,45 @@ class Toy(ormar.Model): owner: Owner = ormar.ForeignKey(Owner) +class Factory(ormar.Model): + class Meta: + tablename = "factories" + metadata = metadata + database = database + + id: int = ormar.Integer(primary_key=True) + name: str = ormar.String(max_length=100) + + +class Car(ormar.Model): + class Meta: + tablename = "cars" + metadata = metadata + database = database + + id: int = ormar.Integer(primary_key=True) + name: str = ormar.String(max_length=100) + factory: Optional[Factory] = ormar.ForeignKey(Factory) + + +class UsersCar(ormar.Model): + class Meta: + tablename = "cars_x_users" + metadata = metadata + database = database + + +class User(ormar.Model): + class Meta: + tablename = "users" + metadata = metadata + database = database + + id: int = ormar.Integer(primary_key=True) + name: str = ormar.String(max_length=100) + cars: List[Car] = ormar.ManyToMany(Car, through=UsersCar) + + @pytest.fixture(autouse=True, scope="module") def create_test_database(): engine = sqlalchemy.create_engine(DATABASE_URL) @@ -93,10 +134,10 @@ async def test_sort_order_on_related_model(): hermes = await Owner.objects.create(name="Hermes") zeus = await Owner.objects.create(name="Zeus") - await Toy.objects.create(name="Toy 1", owner=zeus) + await Toy.objects.create(name="Toy 4", owner=zeus) await Toy.objects.create(name="Toy 5", owner=hermes) await Toy.objects.create(name="Toy 2", owner=aphrodite) - await Toy.objects.create(name="Toy 4", owner=zeus) + await Toy.objects.create(name="Toy 1", owner=zeus) await Toy.objects.create(name="Toy 3", owner=aphrodite) await Toy.objects.create(name="Toy 6", owner=hermes) @@ -108,10 +149,129 @@ async def test_sort_order_on_related_model(): assert toys[1].owner == aphrodite toys = await Toy.objects.select_related("owner").order_by("owner__name").all() + assert toys[0].owner.name == toys[1].owner.name == "Aphrodite" + assert toys[2].owner.name == toys[3].owner.name == "Hermes" + assert toys[4].owner.name == toys[5].owner.name == "Zeus" owner = ( await Owner.objects.select_related("toys") .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() + ) + 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() ) + assert owners[0].toys[0].name == "Toy 6" + assert owners[0].toys[1].name == "Toy 5" + assert owners[0].name == "Hermes" + + assert owners[1].toys[0].name == "Toy 4" + assert owners[1].toys[1].name == "Toy 1" + assert owners[1].name == "Zeus" + + await Toy.objects.create(name="Toy 7", owner=zeus) + + owners = ( + await Owner.objects.select_related("toys") + .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" + assert owners[0].toys[2].name == "Toy 1" + assert owners[0].name == "Zeus" + + assert owners[1].toys[0].name == "Toy 6" + assert owners[1].toys[1].name == "Toy 5" + assert owners[1].name == "Hermes" + + +@pytest.mark.asyncio +async def test_sort_order_on_many_to_many(): + async with database: + factory1 = await Factory.objects.create(name="Factory 1") + factory2 = await Factory.objects.create(name="Factory 2") + + car1 = await Car.objects.create(name="Buggy", factory=factory1) + car2 = await Car.objects.create(name="VW", factory=factory2) + car3 = await Car.objects.create(name="Ferrari", factory=factory1) + car4 = await Car.objects.create(name="Volvo", factory=factory2) + car5 = await Car.objects.create(name="Skoda", factory=factory1) + car6 = await Car.objects.create(name="Seat", factory=factory2) + + user1 = await User.objects.create(name="Mark") + user2 = await User.objects.create(name="Julie") + + await user1.cars.add(car1) + await user1.cars.add(car3) + await user1.cars.add(car4) + await user1.cars.add(car5) + + await user2.cars.add(car1) + await user2.cars.add(car2) + await user2.cars.add(car5) + await user2.cars.add(car6) + + user = ( + await User.objects.select_related("cars") + .filter(name="Mark") + .order_by("cars__name") + .get() + ) + assert user.cars[0].name == "Buggy" + assert user.cars[1].name == "Ferrari" + assert user.cars[2].name == "Skoda" + assert user.cars[3].name == "Volvo" + + user = ( + await User.objects.select_related("cars") + .filter(name="Mark") + .order_by("-cars__name") + .get() + ) + assert user.cars[3].name == "Buggy" + assert user.cars[2].name == "Ferrari" + assert user.cars[1].name == "Skoda" + assert user.cars[0].name == "Volvo" + + users = await User.objects.select_related("cars").order_by("-cars__name").all() + assert users[0].name == "Mark" + assert users[1].cars[0].name == "VW" + assert users[1].cars[1].name == "Skoda" + assert users[1].cars[2].name == "Seat" + assert users[1].cars[3].name == "Buggy" + + users = ( + await User.objects.select_related(["cars", "cars__factory"]) + .order_by(["-cars__factory__name", "cars__name"]) + .all() + ) + + assert users[0].name == "Julie" + assert users[0].cars[0].name == "Seat" + assert users[0].cars[1].name == "VW" + assert users[0].cars[2].name == "Buggy" + assert users[0].cars[3].name == "Skoda" + + assert users[1].name == "Mark" + assert users[1].cars[0].name == "Volvo" + assert users[1].cars[1].name == "Buggy" + assert users[1].cars[2].name == "Ferrari" + assert users[1].cars[3].name == "Skoda" From 62d3388a97f44ea99436479d9cc65da0c9613261 Mon Sep 17 00:00:00 2001 From: collerek Date: Mon, 9 Nov 2020 13:01:02 +0100 Subject: [PATCH 3/9] cleaning and linting --- ormar/models/modelproxy.py | 2 +- ormar/queryset/join.py | 11 ++++++----- ormar/queryset/query.py | 10 +++++----- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/ormar/models/modelproxy.py b/ormar/models/modelproxy.py index ae96010..9033eda 100644 --- a/ormar/models/modelproxy.py +++ b/ormar/models/modelproxy.py @@ -182,7 +182,7 @@ class ModelTableProxy: @classmethod def merge_instances_list(cls, result_rows: Sequence["Model"]) -> Sequence["Model"]: merged_rows: List["Model"] = [] - grouped_instances = OrderedDict() + grouped_instances: OrderedDict = OrderedDict() for model in result_rows: grouped_instances.setdefault(model.pk, []).append(model) diff --git a/ormar/queryset/join.py b/ormar/queryset/join.py index 486ed66..8a24139 100644 --- a/ormar/queryset/join.py +++ b/ormar/queryset/join.py @@ -1,4 +1,5 @@ -from typing import Dict, List, NamedTuple, TYPE_CHECKING, Tuple, Type +from collections import OrderedDict +from typing import List, NamedTuple, Optional, TYPE_CHECKING, Tuple, Type import sqlalchemy from sqlalchemy import text @@ -24,8 +25,8 @@ class SqlJoin: select_from: sqlalchemy.sql.select, columns: List[sqlalchemy.Column], fields: List, - order_columns: List, - sorted_orders: Dict, + order_columns: Optional[List], + sorted_orders: OrderedDict, ) -> None: self.used_aliases = used_aliases self.select_from = select_from @@ -48,7 +49,7 @@ class SqlJoin: def build_join( self, item: str, join_parameters: JoinParameters - ) -> Tuple[List, sqlalchemy.sql.select, List, Dict]: + ) -> Tuple[List, sqlalchemy.sql.select, List, OrderedDict]: for part in item.split("__"): if issubclass( join_parameters.model_cls.Meta.model_fields[part], ManyToManyField @@ -147,7 +148,7 @@ class SqlJoin: condition[-2] == part or condition[-2][1:] == part ) - def get_order_bys( + def get_order_bys( # noqa: CCR001 self, alias: str, to_table: str, pkname_alias: str, part: str ) -> None: if self.order_columns: diff --git a/ormar/queryset/query.py b/ormar/queryset/query.py index a12a89f..8665829 100644 --- a/ormar/queryset/query.py +++ b/ormar/queryset/query.py @@ -39,19 +39,20 @@ class Query: self.select_from: List[str] = [] self.columns = [sqlalchemy.Column] self.order_columns = order_bys - self.sorted_orders = OrderedDict() + self.sorted_orders: OrderedDict = OrderedDict() self._init_sorted_orders() def _init_sorted_orders(self) -> None: - for clause in self.order_columns: - self.sorted_orders[clause] = None + if self.order_columns: + for clause in self.order_columns: + self.sorted_orders[clause] = None @property def prefixed_pk_name(self) -> str: pkname_alias = self.model_cls.get_column_alias(self.model_cls.Meta.pkname) return f"{self.table.name}.{pkname_alias}" - def apply_order_bys_for_primary_model(self) -> None: + def apply_order_bys_for_primary_model(self) -> None: # noqa: CCR001 if self.order_columns: for clause in self.order_columns: if "__" not in clause: @@ -123,6 +124,5 @@ class Query: def _reset_query_parameters(self) -> None: self.select_from = [] self.columns = [] - self.order_bys = [] self.used_aliases = [] self.fields = [] From 9c119aa80b8ba18872c52d8969b123942635e66c Mon Sep 17 00:00:00 2001 From: collerek Date: Mon, 9 Nov 2020 13:29:48 +0100 Subject: [PATCH 4/9] update docs, bump version --- README.md | 2 +- docs/index.md | 1 + docs/queries.md | 81 +++++++++++++++++++++++++++++++++++++ docs_src/queries/docs007.py | 42 +++++++++++++++++++ ormar/__init__.py | 2 +- 5 files changed, 126 insertions(+), 2 deletions(-) create mode 100644 docs_src/queries/docs007.py diff --git a/README.md b/README.md index 6ce8908..5dc94bd 100644 --- a/README.md +++ b/README.md @@ -164,7 +164,7 @@ assert len(tracks) == 1 * `count() -> int` * `exists() -> bool` * `fields(columns: Union[List, str]) -> QuerySet` - +* `order_by(columns:Union[List, str]) -> QuerySet` #### Relation types diff --git a/docs/index.md b/docs/index.md index 6ce8908..7a264da 100644 --- a/docs/index.md +++ b/docs/index.md @@ -164,6 +164,7 @@ assert len(tracks) == 1 * `count() -> int` * `exists() -> bool` * `fields(columns: Union[List, str]) -> QuerySet` +* `order_by(columns:Union[List, str]) -> QuerySet` #### Relation types diff --git a/docs/queries.md b/docs/queries.md index 77c6229..ad82d40 100644 --- a/docs/queries.md +++ b/docs/queries.md @@ -370,5 +370,86 @@ With `fields()` you can select subset of model columns to limit the data load. Something like `Track.object.select_related("album").filter(album__name="Malibu").offset(1).limit(1).all()` +### order_by + +`order_by(columns: Union[List, str]) -> QuerySet` + +With `order_by()` you can order the results from database based on your choice of fields. + +You can provide a string with field name or list of strings with different fields. + +Ordering in sql will be applied in order of names you provide in order_by. + +!!!tip + By default if you do not provide ordering `ormar` explicitly orders by all primary keys + +!!!warning + If you are sorting by nested models that causes that the result rows are unsorted by the main model + `ormar` will combine those children rows into one main model. + + Sample raw database rows result (sort by child model desc): + ``` + MODEL: 1 - Child Model - 3 + MODEL: 2 - Child Model - 2 + MODEL: 1 - Child Model - 1 + ``` + + will result in 2 rows of result: + ``` + MODEL: 1 - Child Models: [3, 1] # encountered first in result, all children rows combined + MODEL: 2 - Child Modles: [2] + ``` + + The main model will never duplicate in the result + +Given sample Models like following: + +```python +--8<-- "../docs_src/queries/docs007.py" +``` + +To order by main model field just provide a field name + +```python +toys = await Toy.objects.select_related("owner").order_by("name").all() +assert [x.name.replace("Toy ", "") for x in toys] == [ + str(x + 1) for x in range(6) +] +assert toys[0].owner == zeus +assert toys[1].owner == aphrodite +``` + +To sort on nested models separate field names with dunder '__'. + +You can sort this way across all relation types -> `ForeignKey`, reverse virtual FK and `ManyToMany` fields. + +```python +toys = await Toy.objects.select_related("owner").order_by("owner__name").all() +assert toys[0].owner.name == toys[1].owner.name == "Aphrodite" +assert toys[2].owner.name == toys[3].owner.name == "Hermes" +assert toys[4].owner.name == toys[5].owner.name == "Zeus" +``` + +To sort in descending order provide a hyphen in front of the field name + +```python +owner = ( + await Owner.objects.select_related("toys") + .order_by("-toys__name") + .filter(name="Zeus") + .get() +) +assert owner.toys[0].name == "Toy 4" +assert owner.toys[1].name == "Toy 1" +``` + +!!!note + All methods that do not return the rows explicitly returns a QueySet instance so you can chain them together + + So operations like `filter()`, `select_related()`, `limit()` and `offset()` etc. can be chained. + + Something like `Track.object.select_related("album").filter(album__name="Malibu").offset(1).limit(1).all()` + + [models]: ./models.md [relations]: ./relations.md \ No newline at end of file diff --git a/docs_src/queries/docs007.py b/docs_src/queries/docs007.py new file mode 100644 index 0000000..7e8ddc0 --- /dev/null +++ b/docs_src/queries/docs007.py @@ -0,0 +1,42 @@ +import databases +import sqlalchemy + +import ormar +from tests.settings import DATABASE_URL + +database = databases.Database(DATABASE_URL, force_rollback=True) +metadata = sqlalchemy.MetaData() + + +class Owner(ormar.Model): + class Meta: + tablename = "owners" + metadata = metadata + database = database + + id: int = ormar.Integer(primary_key=True) + name: str = ormar.String(max_length=100) + + +class Toy(ormar.Model): + class Meta: + tablename = "toys" + metadata = metadata + database = database + + id: int = ormar.Integer(primary_key=True) + name: str = ormar.String(max_length=100) + owner: Owner = ormar.ForeignKey(Owner) + + +# build some sample data +aphrodite = await Owner.objects.create(name="Aphrodite") +hermes = await Owner.objects.create(name="Hermes") +zeus = await Owner.objects.create(name="Zeus") + +await Toy.objects.create(name="Toy 4", owner=zeus) +await Toy.objects.create(name="Toy 5", owner=hermes) +await Toy.objects.create(name="Toy 2", owner=aphrodite) +await Toy.objects.create(name="Toy 1", owner=zeus) +await Toy.objects.create(name="Toy 3", owner=aphrodite) +await Toy.objects.create(name="Toy 6", owner=hermes) diff --git a/ormar/__init__.py b/ormar/__init__.py index b121bb1..e5e9262 100644 --- a/ormar/__init__.py +++ b/ormar/__init__.py @@ -30,7 +30,7 @@ class UndefinedType: # pragma no cover Undefined = UndefinedType() -__version__ = "0.4.0" +__version__ = "0.4.1" __all__ = [ "Integer", "BigInteger", From d716799325b669a7463c6d4416dd50aff720cd8f Mon Sep 17 00:00:00 2001 From: collerek Date: Mon, 9 Nov 2020 13:57:09 +0100 Subject: [PATCH 5/9] fix overwriting order-bys with same column name --- ormar/queryset/join.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ormar/queryset/join.py b/ormar/queryset/join.py index 8a24139..b3bbb06 100644 --- a/ormar/queryset/join.py +++ b/ormar/queryset/join.py @@ -162,7 +162,7 @@ class SqlJoin: self.sorted_orders["__".join(condition)] = order else: order = text(f"{alias}_{to_table}.{pkname_alias}") - self.sorted_orders[pkname_alias] = order + self.sorted_orders[f"{to_table}.{pkname_alias}"] = order @staticmethod def get_to_and_from_keys( From 76a541d0e4c26cf8cc01d221f9c55c242d9a5105 Mon Sep 17 00:00:00 2001 From: collerek Date: Mon, 9 Nov 2020 14:04:09 +0100 Subject: [PATCH 6/9] change test values to make it clear what is the order, not case sensitive - different on diff backends --- tests/test_order_by.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_order_by.py b/tests/test_order_by.py index 37ef2dd..dbe5cd2 100644 --- a/tests/test_order_by.py +++ b/tests/test_order_by.py @@ -210,7 +210,7 @@ async def test_sort_order_on_many_to_many(): factory2 = await Factory.objects.create(name="Factory 2") car1 = await Car.objects.create(name="Buggy", factory=factory1) - car2 = await Car.objects.create(name="VW", factory=factory2) + car2 = await Car.objects.create(name="Volkswagen", factory=factory2) car3 = await Car.objects.create(name="Ferrari", factory=factory1) car4 = await Car.objects.create(name="Volvo", factory=factory2) car5 = await Car.objects.create(name="Skoda", factory=factory1) @@ -253,7 +253,7 @@ async def test_sort_order_on_many_to_many(): users = await User.objects.select_related("cars").order_by("-cars__name").all() assert users[0].name == "Mark" - assert users[1].cars[0].name == "VW" + assert users[1].cars[0].name == "Volkswagen" assert users[1].cars[1].name == "Skoda" assert users[1].cars[2].name == "Seat" assert users[1].cars[3].name == "Buggy" @@ -266,7 +266,7 @@ async def test_sort_order_on_many_to_many(): assert users[0].name == "Julie" assert users[0].cars[0].name == "Seat" - assert users[0].cars[1].name == "VW" + assert users[0].cars[1].name == "Volkswagen" assert users[0].cars[2].name == "Buggy" assert users[0].cars[3].name == "Skoda" From 1fe16f7b2ee2660defc3b98a6546f2245e51f318 Mon Sep 17 00:00:00 2001 From: collerek Date: Mon, 9 Nov 2020 14:11:55 +0100 Subject: [PATCH 7/9] fix docs deploy to only push on master --- .github/workflows/deploy-docs.yml | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/.github/workflows/deploy-docs.yml b/.github/workflows/deploy-docs.yml index d788acd..1cbb31d 100644 --- a/.github/workflows/deploy-docs.yml +++ b/.github/workflows/deploy-docs.yml @@ -1,11 +1,7 @@ -name: Build Documentation using MkDocs -# Controls when the action will run. Triggers the workflow on push or pull request -# events but only for the master branch +name: Build Documentation using MkDocs on: push: - branches: [master] - pull_request: - branches: [master] + branches: [master] jobs: build: name: Build and Deploy Documentation @@ -23,5 +19,5 @@ jobs: pip install mkdocs-material - name: Deploy run: | - git pull + git pull origin master mkdocs gh-deploy From b9040c5105ad70323efaa2f063af86994293254f Mon Sep 17 00:00:00 2001 From: collerek Date: Mon, 9 Nov 2020 14:13:05 +0100 Subject: [PATCH 8/9] fix docs deploy to only push on master --- .github/workflows/deploy-docs.yml | 36 +++++++++++++++---------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/.github/workflows/deploy-docs.yml b/.github/workflows/deploy-docs.yml index 1cbb31d..44fff5f 100644 --- a/.github/workflows/deploy-docs.yml +++ b/.github/workflows/deploy-docs.yml @@ -1,23 +1,23 @@ name: Build Documentation using MkDocs -on: - push: - branches: [master] -jobs: - build: - name: Build and Deploy Documentation - runs-on: ubuntu-latest - steps: - - name: Checkout Master - uses: actions/checkout@v2 - - name: Set up Python 3.7 - uses: actions/setup-python@v2 - with: - python-version: '3.x' - - name: Install dependencies - run: | +on: + push: + branches: [ master ] +jobs: + build: + name: Build and Deploy Documentation + runs-on: ubuntu-latest + steps: + - name: Checkout Master + uses: actions/checkout@v2 + - name: Set up Python 3.7 + uses: actions/setup-python@v2 + with: + python-version: '3.x' + - name: Install dependencies + run: | python -m pip install --upgrade pip pip install mkdocs-material - - name: Deploy - run: | + - name: Deploy + run: | git pull origin master mkdocs gh-deploy From 3315899e6d92d8c4274e2ac48b9b81235d88c1ef Mon Sep 17 00:00:00 2001 From: collerek Date: Mon, 9 Nov 2020 14:13:59 +0100 Subject: [PATCH 9/9] fix docs deploy to only push on master --- .github/workflows/deploy-docs.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/deploy-docs.yml b/.github/workflows/deploy-docs.yml index 44fff5f..c63a113 100644 --- a/.github/workflows/deploy-docs.yml +++ b/.github/workflows/deploy-docs.yml @@ -15,8 +15,8 @@ jobs: python-version: '3.x' - name: Install dependencies run: | - python -m pip install --upgrade pip - pip install mkdocs-material + python -m pip install --upgrade pip + pip install mkdocs-material - name: Deploy run: | git pull origin master