From d4ad40f0a9c61e766cbf6570558435bfecbe6b32 Mon Sep 17 00:00:00 2001 From: Sepehr Date: Sun, 31 Jul 2022 15:43:15 +0430 Subject: [PATCH 01/22] feat: add optional parameter nulls in asc and desc --- ormar/queryset/actions/order_action.py | 9 +++-- ormar/queryset/field_accessor.py | 46 +++++++++++++++++++++++--- 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/ormar/queryset/actions/order_action.py b/ormar/queryset/actions/order_action.py index 7330d727f..8aa4d5ecd 100644 --- a/ormar/queryset/actions/order_action.py +++ b/ormar/queryset/actions/order_action.py @@ -1,4 +1,4 @@ -from typing import TYPE_CHECKING, Type +from typing import Optional, TYPE_CHECKING, Type import sqlalchemy from sqlalchemy import text @@ -20,7 +20,12 @@ 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, + nulls_last: Optional[bool] = None, + nulls_first: Optional[bool] = None, ) -> None: self.direction: str = "" super().__init__(query_str=order_str, model_cls=model_cls) diff --git a/ormar/queryset/field_accessor.py b/ormar/queryset/field_accessor.py index b27483771..53cac46c2 100644 --- a/ormar/queryset/field_accessor.py +++ b/ormar/queryset/field_accessor.py @@ -1,4 +1,4 @@ -from typing import Any, TYPE_CHECKING, Type, cast +from typing import Any, Optional, TYPE_CHECKING, Type, cast from ormar.queryset.actions import OrderAction from ormar.queryset.actions.filter_action import METHODS_TO_OPERATORS @@ -268,22 +268,58 @@ def isnull(self, other: Any) -> FilterGroup: """ return self._select_operator(op="isnull", other=other) - def asc(self) -> OrderAction: + def asc( + self, + nulls_last: Optional[bool] = None, + nulls_first: Optional[bool] = None, + ) -> OrderAction: """ works as sql `column asc` + :param nulls_last: optional boolean flag to Produce the `NULLS LAST` + :type nulls_last: Optional[bool] + :param nulls_first: optional boolean flag to Produce the `NULLS FIRST` + :type nulls_first: Optional[bool] :return: OrderGroup for operator :rtype: ormar.queryset.actions.OrderGroup """ - return OrderAction(order_str=self._access_chain, model_cls=self._source_model) - def desc(self) -> OrderAction: + nulls_last = bool(nulls_last) if nulls_last is not None else None + nulls_first = bool(nulls_first) if nulls_first is not None else None + if nulls_last is not None and nulls_first is not None: + raise ValueError("Both values cannot be specified together.") + + return OrderAction( + order_str=self._access_chain, + model_cls=self._source_model, + nulls_first=nulls_first, + nulls_last=nulls_last, + ) + + def desc( + self, + nulls_last: Optional[bool] = None, + nulls_first: Optional[bool] = None, + ) -> OrderAction: """ works as sql `column desc` + :param nulls_last: optional boolean flag to Produce the `NULLS LAST` + :type nulls_last: Optional[bool] + :param nulls_first: optional boolean flag to Produce the `NULLS FIRST` + :type nulls_first: Optional[bool] :return: OrderGroup for operator :rtype: ormar.queryset.actions.OrderGroup """ + + nulls_last = bool(nulls_last) if nulls_last is not None else None + nulls_first = bool(nulls_first) if nulls_first is not None else None + if nulls_last is not None and nulls_first is not None: + raise ValueError("Both values cannot be specified together.") + return OrderAction( - order_str="-" + self._access_chain, model_cls=self._source_model + order_str="-" + self._access_chain, + model_cls=self._source_model, + nulls_first=nulls_first, + nulls_last=nulls_last, ) From fcb225418301da1aa1b397311b5a599c0f2a5690 Mon Sep 17 00:00:00 2001 From: Sepehr Date: Sun, 31 Jul 2022 16:01:06 +0430 Subject: [PATCH 02/22] feat: set nulls value in get text clause --- ormar/queryset/actions/order_action.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/ormar/queryset/actions/order_action.py b/ormar/queryset/actions/order_action.py index 8aa4d5ecd..bc2e339ec 100644 --- a/ormar/queryset/actions/order_action.py +++ b/ormar/queryset/actions/order_action.py @@ -27,6 +27,8 @@ def __init__( nulls_last: Optional[bool] = None, nulls_first: Optional[bool] = None, ) -> None: + + self.nulls: str = "" self.direction: str = "" super().__init__(query_str=order_str, model_cls=model_cls) self.is_source_model_order = False @@ -35,6 +37,11 @@ def __init__( if self.source_model == self.target_model and "__" not in self.related_str: self.is_source_model_order = True + if nulls_first or (not nulls_last and nulls_last is not None): + self.nulls = "nulls first" + if nulls_last or (not nulls_first and nulls_first is not None): + self.nulls = "nulls last" + @property def field_alias(self) -> str: return self.target_model.get_column_alias(self.field_name) @@ -83,6 +90,7 @@ def get_text_clause(self) -> sqlalchemy.sql.expression.TextClause: :return: complied and escaped clause :rtype: sqlalchemy.sql.elements.TextClause """ + prefix = f"{self.table_prefix}_" if self.table_prefix else "" table_name = self.table.name field_name = self.field_alias @@ -90,7 +98,11 @@ def get_text_clause(self) -> sqlalchemy.sql.expression.TextClause: dialect = self.target_model.Meta.database._backend._dialect table_name = dialect.identifier_preparer.quote(table_name) field_name = dialect.identifier_preparer.quote(field_name) - return text(f"{prefix}{table_name}" f".{field_name} {self.direction}") + + return text( + f"{prefix}{table_name}" + f".{field_name} {self.direction} {self.nulls}" + ) def _split_value_into_parts(self, order_str: str) -> None: if order_str.startswith("-"): From f3133d2e81016ad98f5857e286cc58b136d9ff23 Mon Sep 17 00:00:00 2001 From: Sepehr Date: Sun, 31 Jul 2022 16:20:36 +0430 Subject: [PATCH 03/22] test: write some tests for nulls last and first --- tests/test_queries/test_order_by.py | 39 ++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/tests/test_queries/test_order_by.py b/tests/test_queries/test_order_by.py index d32cf685c..38ef67ef9 100644 --- a/tests/test_queries/test_order_by.py +++ b/tests/test_queries/test_order_by.py @@ -19,7 +19,7 @@ class Meta: id: int = ormar.Integer(primary_key=True) name: str = ormar.String(max_length=100) - sort_order: int = ormar.Integer() + sort_order: Optional[int] = ormar.Integer(nullable=True) class Owner(ormar.Model): @@ -163,6 +163,43 @@ async def test_sort_order_on_main_model(): assert songs[2].name == "Song 2" assert songs[3].name == "Song 3" + await Song.objects.create(name="Song 5") + + songs = await Song.objects.order_by(Song.sort_order.asc(nulls_last=True)).all() + assert songs[0].name == "Song 1" + assert songs[1].name == "Song 2" + assert songs[2].name == "Song 3" + assert songs[3].name == "Song 4" + assert songs[4].name == "Song 5" + + songs = await Song.objects.order_by(Song.sort_order.asc(nulls_first=True)).all() + assert songs[0].name == "Song 5" + assert songs[1].name == "Song 1" + assert songs[2].name == "Song 2" + assert songs[3].name == "Song 3" + assert songs[4].name == "Song 4" + + songs = await Song.objects.order_by(Song.sort_order.desc(nulls_last=True)).all() + assert songs[0].name == "Song 4" + assert songs[1].name == "Song 3" + assert songs[2].name == "Song 2" + assert songs[3].name == "Song 1" + assert songs[4].name == "Song 5" + + songs = await Song.objects.order_by( + Song.sort_order.desc(nulls_first=True) + ).all() + assert songs[0].name == "Song 5" + assert songs[1].name == "Song 4" + assert songs[2].name == "Song 3" + assert songs[3].name == "Song 2" + assert songs[4].name == "Song 1" + + with pytest.raises(ValueError): + await Song.objects.order_by( + Song.sort_order.asc(nulls_last=True, nulls_first=True) + ).all() + @pytest.mark.asyncio async def test_sort_order_on_related_model(): From 90e16c57117d388c6a69613a50909309b2c87055 Mon Sep 17 00:00:00 2001 From: Sepehr Date: Sun, 31 Jul 2022 16:27:33 +0430 Subject: [PATCH 04/22] docs: write sample document nulls last bool flag --- docs/queries/filter-and-sort.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/queries/filter-and-sort.md b/docs/queries/filter-and-sort.md index 530debcb0..8e5e9325d 100644 --- a/docs/queries/filter-and-sort.md +++ b/docs/queries/filter-and-sort.md @@ -800,7 +800,12 @@ assert owner.toys[1].name == "Toy 1" 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()` + Something like `Track.objects.select_related("album").filter(album__name="Malibu").offset(1).limit(1).all()` + +!!!note + You can use the parameters `nulls_last` and `nulls_first` to determine the behavior in dealing with `NULL` values. + + Something like `Owner.objects.order_by(Owner.toys.name.desc(nulls_last=True)).all()` ### Default sorting in ormar From 685c495734ecbf79dd62e54fc60a290148600cce Mon Sep 17 00:00:00 2001 From: Sepehr Date: Sun, 31 Jul 2022 17:30:54 +0430 Subject: [PATCH 05/22] fix: temp debug for handle mysql --- ormar/queryset/actions/order_action.py | 44 ++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/ormar/queryset/actions/order_action.py b/ormar/queryset/actions/order_action.py index bc2e339ec..52202e993 100644 --- a/ormar/queryset/actions/order_action.py +++ b/ormar/queryset/actions/order_action.py @@ -28,7 +28,6 @@ def __init__( nulls_first: Optional[bool] = None, ) -> None: - self.nulls: str = "" self.direction: str = "" super().__init__(query_str=order_str, model_cls=model_cls) self.is_source_model_order = False @@ -37,15 +36,18 @@ def __init__( if self.source_model == self.target_model and "__" not in self.related_str: self.is_source_model_order = True - if nulls_first or (not nulls_last and nulls_last is not None): - self.nulls = "nulls first" - if nulls_last or (not nulls_first and nulls_first is not None): - self.nulls = "nulls last" + self.nulls: Optional[str] = self._get_nulls( + nulls_last=nulls_last, nulls_first=nulls_first + ) @property def field_alias(self) -> str: return self.target_model.get_column_alias(self.field_name) + @property + def is_mysql_bool(self) -> bool: + return self.target_model.Meta.database._backend._dialect.name == "mysql" + @property def is_postgres_bool(self) -> bool: dialect = self.target_model.Meta.database._backend._dialect.name @@ -101,7 +103,7 @@ def get_text_clause(self) -> sqlalchemy.sql.expression.TextClause: return text( f"{prefix}{table_name}" - f".{field_name} {self.direction} {self.nulls}" + f".{self._get_field_name_direction_nulls(field_name=field_name)}" ) def _split_value_into_parts(self, order_str: str) -> None: @@ -112,6 +114,36 @@ def _split_value_into_parts(self, order_str: str) -> None: self.field_name = parts[-1] self.related_parts = parts[:-1] + @staticmethod + def _get_nulls( + nulls_last: Optional[bool] = None, + nulls_first: Optional[bool] = None, + ) -> Optional[str]: + + if nulls_first or (not nulls_last and nulls_last is not None): + return "first" + + if nulls_last or (not nulls_first and nulls_first is not None): + return "last" + + return None + + def _handle_field_nulls_mysql(self, field_name: str, result: str) -> str: + + if not self.is_mysql_bool: + return result + f" nulls {self.nulls}" + + not_: str = "not" if self.nulls == "first" else "" + return f"{field_name} is {not_} null, {result}" + + def _get_field_name_direction_nulls(self, field_name: str) -> str: + + result: str = f"{field_name} {self.direction}" + if self.nulls is not None: + return self._handle_field_nulls_mysql(field_name=field_name, result=result) + + return result + def check_if_filter_apply(self, target_model: Type["Model"], alias: str) -> bool: """ Checks filter conditions to find if they apply to current join. From d6d4a65ea422396acdd817b653a21ca5ace330a1 Mon Sep 17 00:00:00 2001 From: Sepehr Date: Sun, 31 Jul 2022 17:41:10 +0430 Subject: [PATCH 06/22] fix: debug order of songs by sort odrer --- tests/test_queries/test_order_by.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/test_queries/test_order_by.py b/tests/test_queries/test_order_by.py index 38ef67ef9..629e44f1a 100644 --- a/tests/test_queries/test_order_by.py +++ b/tests/test_queries/test_order_by.py @@ -167,22 +167,22 @@ async def test_sort_order_on_main_model(): songs = await Song.objects.order_by(Song.sort_order.asc(nulls_last=True)).all() assert songs[0].name == "Song 1" - assert songs[1].name == "Song 2" - assert songs[2].name == "Song 3" - assert songs[3].name == "Song 4" + assert songs[1].name == "Song 4" + assert songs[2].name == "Song 2" + assert songs[3].name == "Song 3" assert songs[4].name == "Song 5" songs = await Song.objects.order_by(Song.sort_order.asc(nulls_first=True)).all() assert songs[0].name == "Song 5" assert songs[1].name == "Song 1" - assert songs[2].name == "Song 2" - assert songs[3].name == "Song 3" - assert songs[4].name == "Song 4" + assert songs[2].name == "Song 4" + assert songs[3].name == "Song 2" + assert songs[4].name == "Song 3" songs = await Song.objects.order_by(Song.sort_order.desc(nulls_last=True)).all() - assert songs[0].name == "Song 4" - assert songs[1].name == "Song 3" - assert songs[2].name == "Song 2" + assert songs[0].name == "Song 3" + assert songs[1].name == "Song 2" + assert songs[2].name == "Song 4" assert songs[3].name == "Song 1" assert songs[4].name == "Song 5" @@ -190,9 +190,9 @@ async def test_sort_order_on_main_model(): Song.sort_order.desc(nulls_first=True) ).all() assert songs[0].name == "Song 5" - assert songs[1].name == "Song 4" - assert songs[2].name == "Song 3" - assert songs[3].name == "Song 2" + assert songs[1].name == "Song 3" + assert songs[2].name == "Song 2" + assert songs[3].name == "Song 4" assert songs[4].name == "Song 1" with pytest.raises(ValueError): From cdb6468d72e5babdb5111cf7bda9e520b49f9b1c Mon Sep 17 00:00:00 2001 From: Sepehr Date: Mon, 1 Aug 2022 09:42:50 +0430 Subject: [PATCH 07/22] fix: debug test result with write docstring types --- ormar/queryset/actions/order_action.py | 36 ++++++++++++++++++++++---- tests/test_queries/test_order_by.py | 16 ++++++------ 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/ormar/queryset/actions/order_action.py b/ormar/queryset/actions/order_action.py index 52202e993..53498727c 100644 --- a/ormar/queryset/actions/order_action.py +++ b/ormar/queryset/actions/order_action.py @@ -36,9 +36,7 @@ def __init__( if self.source_model == self.target_model and "__" not in self.related_str: self.is_source_model_order = True - self.nulls: Optional[str] = self._get_nulls( - nulls_last=nulls_last, nulls_first=nulls_first - ) + self.nulls = self._get_nulls(nulls_last=nulls_last, nulls_first=nulls_first) @property def field_alias(self) -> str: @@ -119,6 +117,16 @@ def _get_nulls( nulls_last: Optional[bool] = None, nulls_first: Optional[bool] = None, ) -> Optional[str]: + """ + Returned `FIRST` or `LAST` string for condition on nulls value + + :param nulls_last: optional boolean flag to Produce the `NULLS LAST` + :type nulls_last: Optional[bool] + :param nulls_first: optional boolean flag to Produce the `NULLS FIRST` + :type nulls_first: Optional[bool] + :return: result of the nulls last of nulls first or none + :rtype: Optional[str] + """ if nulls_first or (not nulls_last and nulls_last is not None): return "first" @@ -129,14 +137,32 @@ def _get_nulls( return None def _handle_field_nulls_mysql(self, field_name: str, result: str) -> str: + """ + Generate the Final Query with handling mysql syntax for nulls value + + :param field_name: string name of this field for order + :type field_name: str + :param result: query generated in previous stage without nulls value + :type result: str + :return: result of the final query by field name and direction and nulls value + :rtype: str + """ if not self.is_mysql_bool: return result + f" nulls {self.nulls}" - not_: str = "not" if self.nulls == "first" else "" - return f"{field_name} is {not_} null, {result}" + condition: str = "not" if self.nulls == "first" else "" + return f"{field_name} is {condition} null, {result}" def _get_field_name_direction_nulls(self, field_name: str) -> str: + """ + Generate the Query of Order for this field name by direction and nulls value + + :param field_name: string name of this field for order + :type field_name: str + :return: result of the query by field name and direction and nulls value + :rtype: str + """ result: str = f"{field_name} {self.direction}" if self.nulls is not None: diff --git a/tests/test_queries/test_order_by.py b/tests/test_queries/test_order_by.py index 629e44f1a..924fe5bfa 100644 --- a/tests/test_queries/test_order_by.py +++ b/tests/test_queries/test_order_by.py @@ -166,24 +166,24 @@ async def test_sort_order_on_main_model(): await Song.objects.create(name="Song 5") songs = await Song.objects.order_by(Song.sort_order.asc(nulls_last=True)).all() - assert songs[0].name == "Song 1" - assert songs[1].name == "Song 4" + assert songs[0].name in "Song 1", "Song 4" + assert songs[1].name in "Song 1", "Song 4" assert songs[2].name == "Song 2" assert songs[3].name == "Song 3" assert songs[4].name == "Song 5" songs = await Song.objects.order_by(Song.sort_order.asc(nulls_first=True)).all() assert songs[0].name == "Song 5" - assert songs[1].name == "Song 1" - assert songs[2].name == "Song 4" + assert songs[1].name in "Song 1", "Song 4" + assert songs[2].name in "Song 1", "Song 4" assert songs[3].name == "Song 2" assert songs[4].name == "Song 3" songs = await Song.objects.order_by(Song.sort_order.desc(nulls_last=True)).all() assert songs[0].name == "Song 3" assert songs[1].name == "Song 2" - assert songs[2].name == "Song 4" - assert songs[3].name == "Song 1" + assert songs[2].name in "Song 1", "Song 4" + assert songs[3].name in "Song 1", "Song 4" assert songs[4].name == "Song 5" songs = await Song.objects.order_by( @@ -192,8 +192,8 @@ async def test_sort_order_on_main_model(): assert songs[0].name == "Song 5" assert songs[1].name == "Song 3" assert songs[2].name == "Song 2" - assert songs[3].name == "Song 4" - assert songs[4].name == "Song 1" + assert songs[3].name in "Song 1", "Song 4" + assert songs[4].name in "Song 1", "Song 4" with pytest.raises(ValueError): await Song.objects.order_by( From 945d165ec3b371ecb000e63a3afe4162d3cd535a Mon Sep 17 00:00:00 2001 From: Sepehr Date: Mon, 1 Aug 2022 09:48:03 +0430 Subject: [PATCH 08/22] fix: set the parenthese for tuple type --- tests/test_queries/test_order_by.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/test_queries/test_order_by.py b/tests/test_queries/test_order_by.py index 924fe5bfa..46cc7dcba 100644 --- a/tests/test_queries/test_order_by.py +++ b/tests/test_queries/test_order_by.py @@ -166,24 +166,24 @@ async def test_sort_order_on_main_model(): await Song.objects.create(name="Song 5") songs = await Song.objects.order_by(Song.sort_order.asc(nulls_last=True)).all() - assert songs[0].name in "Song 1", "Song 4" - assert songs[1].name in "Song 1", "Song 4" + assert songs[0].name in ("Song 1", "Song 4") + assert songs[1].name in ("Song 1", "Song 4") assert songs[2].name == "Song 2" assert songs[3].name == "Song 3" assert songs[4].name == "Song 5" songs = await Song.objects.order_by(Song.sort_order.asc(nulls_first=True)).all() assert songs[0].name == "Song 5" - assert songs[1].name in "Song 1", "Song 4" - assert songs[2].name in "Song 1", "Song 4" + assert songs[1].name in ("Song 1", "Song 4") + assert songs[2].name in ("Song 1", "Song 4") assert songs[3].name == "Song 2" assert songs[4].name == "Song 3" songs = await Song.objects.order_by(Song.sort_order.desc(nulls_last=True)).all() assert songs[0].name == "Song 3" assert songs[1].name == "Song 2" - assert songs[2].name in "Song 1", "Song 4" - assert songs[3].name in "Song 1", "Song 4" + assert songs[2].name in ("Song 1", "Song 4") + assert songs[3].name in ("Song 1", "Song 4") assert songs[4].name == "Song 5" songs = await Song.objects.order_by( @@ -192,8 +192,8 @@ async def test_sort_order_on_main_model(): assert songs[0].name == "Song 5" assert songs[1].name == "Song 3" assert songs[2].name == "Song 2" - assert songs[3].name in "Song 1", "Song 4" - assert songs[4].name in "Song 1", "Song 4" + assert songs[3].name in ("Song 1", "Song 4") + assert songs[4].name in ("Song 1", "Song 4") with pytest.raises(ValueError): await Song.objects.order_by( From f89631e3c5b5c5c293f64c9738509460078efae6 Mon Sep 17 00:00:00 2001 From: Sepehr Date: Mon, 1 Aug 2022 09:57:44 +0430 Subject: [PATCH 09/22] fix: write new test and no cover for coverages --- ormar/queryset/actions/order_action.py | 6 +++--- tests/test_queries/test_order_by.py | 5 +++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/ormar/queryset/actions/order_action.py b/ormar/queryset/actions/order_action.py index 53498727c..52320df9f 100644 --- a/ormar/queryset/actions/order_action.py +++ b/ormar/queryset/actions/order_action.py @@ -149,10 +149,10 @@ def _handle_field_nulls_mysql(self, field_name: str, result: str) -> str: """ if not self.is_mysql_bool: - return result + f" nulls {self.nulls}" + return result + f" nulls {self.nulls}" # pragma: no cover - condition: str = "not" if self.nulls == "first" else "" - return f"{field_name} is {condition} null, {result}" + condition: str = "not" if self.nulls == "first" else "" # pragma: no cover + return f"{field_name} is {condition} null, {result}" # pragma: no cover def _get_field_name_direction_nulls(self, field_name: str) -> str: """ diff --git a/tests/test_queries/test_order_by.py b/tests/test_queries/test_order_by.py index 46cc7dcba..4c166a39e 100644 --- a/tests/test_queries/test_order_by.py +++ b/tests/test_queries/test_order_by.py @@ -200,6 +200,11 @@ async def test_sort_order_on_main_model(): Song.sort_order.asc(nulls_last=True, nulls_first=True) ).all() + with pytest.raises(ValueError): + await Song.objects.order_by( + Song.sort_order.desc(nulls_last=True, nulls_first=True) + ).all() + @pytest.mark.asyncio async def test_sort_order_on_related_model(): From 324a165ed7bda73ee0706c0f8c73dbb4e67a481e Mon Sep 17 00:00:00 2001 From: Sepehr Date: Fri, 26 Aug 2022 23:06:59 +0430 Subject: [PATCH 10/22] feat: added nulls ordering enum for single option --- ormar/__init__.py | 3 ++- ormar/queryset/__init__.py | 3 ++- ormar/queryset/clause.py | 7 +++++++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/ormar/__init__.py b/ormar/__init__.py index 9259d1d46..16c32255a 100644 --- a/ormar/__init__.py +++ b/ormar/__init__.py @@ -75,7 +75,7 @@ ) # noqa: I100 from ormar.models import ExcludableItems, Extra, Model from ormar.models.metaclass import ModelMeta -from ormar.queryset import OrderAction, QuerySet, and_, or_ +from ormar.queryset import OrderAction, QuerySet, and_, or_, NullsOrdering from ormar.relations import RelationType from ormar.signals import Signal @@ -146,4 +146,5 @@ def __repr__(self) -> str: "DECODERS_MAP", "LargeBinary", "Extra", + "NullsOrdering", ] diff --git a/ormar/queryset/__init__.py b/ormar/queryset/__init__.py index 32fbee48e..beeb20111 100644 --- a/ormar/queryset/__init__.py +++ b/ormar/queryset/__init__.py @@ -2,7 +2,7 @@ Contains QuerySet and different Query classes to allow for constructing of sql queries. """ from ormar.queryset.actions import FilterAction, OrderAction, SelectAction -from ormar.queryset.clause import and_, or_ +from ormar.queryset.clause import and_, or_, NullsOrdering from ormar.queryset.field_accessor import FieldAccessor from ormar.queryset.queries import FilterQuery from ormar.queryset.queries import LimitQuery @@ -22,4 +22,5 @@ "and_", "or_", "FieldAccessor", + "NullsOrdering", ] diff --git a/ormar/queryset/clause.py b/ormar/queryset/clause.py index 2a86d454c..431df4bd9 100644 --- a/ormar/queryset/clause.py +++ b/ormar/queryset/clause.py @@ -18,6 +18,13 @@ class FilterType(Enum): OR = 2 +class NullsOrdering(Enum): + """Nulls ordering options for the `.order_by()` queries.""" + + FIRST: str = "FIRST" + LAST: str = "LAST" + + class FilterGroup: """ Filter groups are used in complex queries condition to group and and or From 5f9d231a989c27c2dd43229703a697826e65ca7d Mon Sep 17 00:00:00 2001 From: Sepehr Date: Fri, 26 Aug 2022 23:25:21 +0430 Subject: [PATCH 11/22] refactor: replace nulls ordering with boolean flag --- ormar/queryset/actions/order_action.py | 29 ++-------------- ormar/queryset/clause.py | 4 +-- ormar/queryset/field_accessor.py | 48 ++++++++------------------ 3 files changed, 19 insertions(+), 62 deletions(-) diff --git a/ormar/queryset/actions/order_action.py b/ormar/queryset/actions/order_action.py index 52320df9f..bbda4230d 100644 --- a/ormar/queryset/actions/order_action.py +++ b/ormar/queryset/actions/order_action.py @@ -24,8 +24,7 @@ def __init__( order_str: str, model_cls: Type["Model"], alias: str = None, - nulls_last: Optional[bool] = None, - nulls_first: Optional[bool] = None, + nulls_ordering: Optional[str] = None, ) -> None: self.direction: str = "" @@ -36,7 +35,7 @@ def __init__( if self.source_model == self.target_model and "__" not in self.related_str: self.is_source_model_order = True - self.nulls = self._get_nulls(nulls_last=nulls_last, nulls_first=nulls_first) + self.nulls = nulls_ordering if nulls_ordering is not None else None @property def field_alias(self) -> str: @@ -112,30 +111,6 @@ def _split_value_into_parts(self, order_str: str) -> None: self.field_name = parts[-1] self.related_parts = parts[:-1] - @staticmethod - def _get_nulls( - nulls_last: Optional[bool] = None, - nulls_first: Optional[bool] = None, - ) -> Optional[str]: - """ - Returned `FIRST` or `LAST` string for condition on nulls value - - :param nulls_last: optional boolean flag to Produce the `NULLS LAST` - :type nulls_last: Optional[bool] - :param nulls_first: optional boolean flag to Produce the `NULLS FIRST` - :type nulls_first: Optional[bool] - :return: result of the nulls last of nulls first or none - :rtype: Optional[str] - """ - - if nulls_first or (not nulls_last and nulls_last is not None): - return "first" - - if nulls_last or (not nulls_first and nulls_first is not None): - return "last" - - return None - def _handle_field_nulls_mysql(self, field_name: str, result: str) -> str: """ Generate the Final Query with handling mysql syntax for nulls value diff --git a/ormar/queryset/clause.py b/ormar/queryset/clause.py index 431df4bd9..d563e44b4 100644 --- a/ormar/queryset/clause.py +++ b/ormar/queryset/clause.py @@ -21,8 +21,8 @@ class FilterType(Enum): class NullsOrdering(Enum): """Nulls ordering options for the `.order_by()` queries.""" - FIRST: str = "FIRST" - LAST: str = "LAST" + FIRST: str = "first" + LAST: str = "last" class FilterGroup: diff --git a/ormar/queryset/field_accessor.py b/ormar/queryset/field_accessor.py index 53cac46c2..94234287d 100644 --- a/ormar/queryset/field_accessor.py +++ b/ormar/queryset/field_accessor.py @@ -2,7 +2,7 @@ from ormar.queryset.actions import OrderAction from ormar.queryset.actions.filter_action import METHODS_TO_OPERATORS -from ormar.queryset.clause import FilterGroup +from ormar.queryset.clause import FilterGroup, NullsOrdering if TYPE_CHECKING: # pragma: no cover from ormar import BaseField, Model @@ -268,58 +268,40 @@ def isnull(self, other: Any) -> FilterGroup: """ return self._select_operator(op="isnull", other=other) - def asc( - self, - nulls_last: Optional[bool] = None, - nulls_first: Optional[bool] = None, - ) -> OrderAction: + def asc(self, nulls_ordering: Optional[NullsOrdering] = None) -> OrderAction: """ works as sql `column asc` - :param nulls_last: optional boolean flag to Produce the `NULLS LAST` - :type nulls_last: Optional[bool] - :param nulls_first: optional boolean flag to Produce the `NULLS FIRST` - :type nulls_first: Optional[bool] + :param nulls_ordering: nulls ordering option first or last, defaults to None + :type nulls_ordering: Optional[NullsOrdering], optional + :raises ValueError: if nulls_ordering is not None or NullsOrdering Enum :return: OrderGroup for operator :rtype: ormar.queryset.actions.OrderGroup """ - - nulls_last = bool(nulls_last) if nulls_last is not None else None - nulls_first = bool(nulls_first) if nulls_first is not None else None - if nulls_last is not None and nulls_first is not None: - raise ValueError("Both values cannot be specified together.") + if nulls_ordering is not None and not isinstance(nulls_ordering, NullsOrdering): + raise ValueError("Invalid option for ordering nulls values.") return OrderAction( order_str=self._access_chain, model_cls=self._source_model, - nulls_first=nulls_first, - nulls_last=nulls_last, + nulls_ordering=nulls_ordering.value if nulls_ordering is not None else None, ) - def desc( - self, - nulls_last: Optional[bool] = None, - nulls_first: Optional[bool] = None, - ) -> OrderAction: + def desc(self, nulls_ordering: Optional[NullsOrdering] = None) -> OrderAction: """ works as sql `column desc` - :param nulls_last: optional boolean flag to Produce the `NULLS LAST` - :type nulls_last: Optional[bool] - :param nulls_first: optional boolean flag to Produce the `NULLS FIRST` - :type nulls_first: Optional[bool] + :param nulls_ordering: nulls ordering option first or last, defaults to None + :type nulls_ordering: Optional[NullsOrdering], optional + :raises ValueError: if nulls_ordering is not None or NullsOrdering Enum :return: OrderGroup for operator :rtype: ormar.queryset.actions.OrderGroup """ - - nulls_last = bool(nulls_last) if nulls_last is not None else None - nulls_first = bool(nulls_first) if nulls_first is not None else None - if nulls_last is not None and nulls_first is not None: - raise ValueError("Both values cannot be specified together.") + if nulls_ordering is not None and not isinstance(nulls_ordering, NullsOrdering): + raise ValueError("Invalid option for ordering nulls values.") return OrderAction( order_str="-" + self._access_chain, model_cls=self._source_model, - nulls_first=nulls_first, - nulls_last=nulls_last, + nulls_ordering=nulls_ordering.value if nulls_ordering is not None else None, ) From 355b8414ea07b8795e3a3a323d6578a12465a83c Mon Sep 17 00:00:00 2001 From: Sepehr Date: Fri, 26 Aug 2022 23:29:56 +0430 Subject: [PATCH 12/22] test: rewrite and update nulls ordering tests --- tests/test_queries/test_order_by.py | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/tests/test_queries/test_order_by.py b/tests/test_queries/test_order_by.py index 4c166a39e..01e97340b 100644 --- a/tests/test_queries/test_order_by.py +++ b/tests/test_queries/test_order_by.py @@ -165,21 +165,27 @@ async def test_sort_order_on_main_model(): await Song.objects.create(name="Song 5") - songs = await Song.objects.order_by(Song.sort_order.asc(nulls_last=True)).all() + songs = await Song.objects.order_by( + Song.sort_order.asc(nulls_ordering=ormar.NullsOrdering.LAST) + ).all() assert songs[0].name in ("Song 1", "Song 4") assert songs[1].name in ("Song 1", "Song 4") assert songs[2].name == "Song 2" assert songs[3].name == "Song 3" assert songs[4].name == "Song 5" - songs = await Song.objects.order_by(Song.sort_order.asc(nulls_first=True)).all() + songs = await Song.objects.order_by( + Song.sort_order.asc(nulls_ordering=ormar.NullsOrdering.FIRST) + ).all() assert songs[0].name == "Song 5" assert songs[1].name in ("Song 1", "Song 4") assert songs[2].name in ("Song 1", "Song 4") assert songs[3].name == "Song 2" assert songs[4].name == "Song 3" - songs = await Song.objects.order_by(Song.sort_order.desc(nulls_last=True)).all() + songs = await Song.objects.order_by( + Song.sort_order.desc(nulls_ordering=ormar.NullsOrdering.LAST) + ).all() assert songs[0].name == "Song 3" assert songs[1].name == "Song 2" assert songs[2].name in ("Song 1", "Song 4") @@ -187,7 +193,7 @@ async def test_sort_order_on_main_model(): assert songs[4].name == "Song 5" songs = await Song.objects.order_by( - Song.sort_order.desc(nulls_first=True) + Song.sort_order.desc(nulls_ordering=ormar.NullsOrdering.FIRST) ).all() assert songs[0].name == "Song 5" assert songs[1].name == "Song 3" @@ -196,14 +202,7 @@ async def test_sort_order_on_main_model(): assert songs[4].name in ("Song 1", "Song 4") with pytest.raises(ValueError): - await Song.objects.order_by( - Song.sort_order.asc(nulls_last=True, nulls_first=True) - ).all() - - with pytest.raises(ValueError): - await Song.objects.order_by( - Song.sort_order.desc(nulls_last=True, nulls_first=True) - ).all() + await Song.objects.order_by(Song.sort_order.asc(nulls_ordering=True)).all() @pytest.mark.asyncio From c8d69d6ee9ee37b5b9a671a566e55da5bfc09d40 Mon Sep 17 00:00:00 2001 From: Sepehr Date: Fri, 26 Aug 2022 23:32:47 +0430 Subject: [PATCH 13/22] docs: update nulls ordering in documents --- docs/queries/filter-and-sort.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/queries/filter-and-sort.md b/docs/queries/filter-and-sort.md index 8e5e9325d..2ba84b881 100644 --- a/docs/queries/filter-and-sort.md +++ b/docs/queries/filter-and-sort.md @@ -803,9 +803,9 @@ assert owner.toys[1].name == "Toy 1" Something like `Track.objects.select_related("album").filter(album__name="Malibu").offset(1).limit(1).all()` !!!note - You can use the parameters `nulls_last` and `nulls_first` to determine the behavior in dealing with `NULL` values. + You can use the parameter `nulls_ordering` to determine the behavior in dealing with `NULL` values. - Something like `Owner.objects.order_by(Owner.toys.name.desc(nulls_last=True)).all()` + Something like `Owner.objects.order_by(Owner.toys.name.desc(nulls_ordering=ormar.NullsOrdering.LAST)).all()` ### Default sorting in ormar From be783d153a5fa4662569ed3a1146520016862e1b Mon Sep 17 00:00:00 2001 From: Sepehr Date: Fri, 26 Aug 2022 23:38:16 +0430 Subject: [PATCH 14/22] fix: write new test to coverage value errors --- tests/test_queries/test_order_by.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_queries/test_order_by.py b/tests/test_queries/test_order_by.py index 01e97340b..de6599922 100644 --- a/tests/test_queries/test_order_by.py +++ b/tests/test_queries/test_order_by.py @@ -202,7 +202,10 @@ async def test_sort_order_on_main_model(): assert songs[4].name in ("Song 1", "Song 4") with pytest.raises(ValueError): - await Song.objects.order_by(Song.sort_order.asc(nulls_ordering=True)).all() + await Song.objects.order_by(Song.sort_order.asc(nulls_ordering=False)).all() + + with pytest.raises(ValueError): + await Song.objects.order_by(Song.sort_order.desc(nulls_ordering=True)).all() @pytest.mark.asyncio From 2920100cdcf969b8379d3ca120ad486002a39c1e Mon Sep 17 00:00:00 2001 From: Sepehr Date: Tue, 13 Sep 2022 16:45:01 +0430 Subject: [PATCH 15/22] refactor: rename function to generate nulls query --- ormar/queryset/actions/order_action.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ormar/queryset/actions/order_action.py b/ormar/queryset/actions/order_action.py index bbda4230d..31971d988 100644 --- a/ormar/queryset/actions/order_action.py +++ b/ormar/queryset/actions/order_action.py @@ -111,7 +111,7 @@ def _split_value_into_parts(self, order_str: str) -> None: self.field_name = parts[-1] self.related_parts = parts[:-1] - def _handle_field_nulls_mysql(self, field_name: str, result: str) -> str: + def _generate_field_nulls_query(self, field_name: str, result: str) -> str: """ Generate the Final Query with handling mysql syntax for nulls value @@ -141,7 +141,7 @@ def _get_field_name_direction_nulls(self, field_name: str) -> str: result: str = f"{field_name} {self.direction}" if self.nulls is not None: - return self._handle_field_nulls_mysql(field_name=field_name, result=result) + return self._generate_field_nulls_query(field_name=field_name, result=result) return result From 3e986ed03c79ed9f81e9e3eb24fe779c777f2153 Mon Sep 17 00:00:00 2001 From: Sepehr Date: Tue, 13 Sep 2022 17:02:36 +0430 Subject: [PATCH 16/22] refactor: set prefix and table name generate query --- ormar/queryset/actions/order_action.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/ormar/queryset/actions/order_action.py b/ormar/queryset/actions/order_action.py index 31971d988..62e939676 100644 --- a/ormar/queryset/actions/order_action.py +++ b/ormar/queryset/actions/order_action.py @@ -98,10 +98,10 @@ def get_text_clause(self) -> sqlalchemy.sql.expression.TextClause: table_name = dialect.identifier_preparer.quote(table_name) field_name = dialect.identifier_preparer.quote(field_name) - return text( - f"{prefix}{table_name}" - f".{self._get_field_name_direction_nulls(field_name=field_name)}" + field_name_direction_nulls = self._get_field_name_direction_nulls( + field_name=field_name, prefix=prefix, table_name=table_name ) + return text(f"{prefix}{table_name}.{field_name_direction_nulls}") def _split_value_into_parts(self, order_str: str) -> None: if order_str.startswith("-"): @@ -129,19 +129,27 @@ def _generate_field_nulls_query(self, field_name: str, result: str) -> str: condition: str = "not" if self.nulls == "first" else "" # pragma: no cover return f"{field_name} is {condition} null, {result}" # pragma: no cover - def _get_field_name_direction_nulls(self, field_name: str) -> str: + def _get_field_name_direction_nulls( + self, field_name: str, prefix: str, table_name: str + ) -> str: """ Generate the Query of Order for this field name by direction and nulls value :param field_name: string name of this field for order :type field_name: str + :param prefix: string prefix name of this field for order + :type prefix: str + :param table_name: string table name of this field for order + :type table_name: str :return: result of the query by field name and direction and nulls value :rtype: str """ result: str = f"{field_name} {self.direction}" if self.nulls is not None: - return self._generate_field_nulls_query(field_name=field_name, result=result) + return self._generate_field_nulls_query( + field_name=f"{prefix}{table_name}.{field_name}", result=result + ) return result From a72579cbb8d40908979f5e264ef707d198b9fdb1 Mon Sep 17 00:00:00 2001 From: Sepehr Date: Tue, 13 Sep 2022 17:28:10 +0430 Subject: [PATCH 17/22] test: write tests to order by related model --- tests/test_queries/test_order_by.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/test_queries/test_order_by.py b/tests/test_queries/test_order_by.py index de6599922..317efc55f 100644 --- a/tests/test_queries/test_order_by.py +++ b/tests/test_queries/test_order_by.py @@ -30,6 +30,7 @@ class Meta: id: int = ormar.Integer(primary_key=True) name: str = ormar.String(max_length=100) + age: Optional[int] = ormar.Integer(nullable=True) class AliasNested(ormar.Model): @@ -293,6 +294,21 @@ async def test_sort_order_on_related_model(): assert toys[0].name == "Toy 2" assert toys[1].name == "Toy 3" + akbar = await Owner.objects.create(name="Akbar", age=22) + asqar = await Owner.objects.create(name="Asqar", age=18) + + await Toy.objects.create(name="Toy 7", owner=akbar) + await Toy.objects.create(name="Toy 8", owner=asqar) + + toys = ( + await Toy.objects.select_related("owner") + .order_by(Toy.owner.age.desc(nulls_ordering=ormar.NullsOrdering.LAST)) + .all() + ) + assert len(toys) == 10 + assert toys[0].name == "Toy 7" + assert toys[1].name == "Toy 8" + @pytest.mark.asyncio async def test_sort_order_on_many_to_many(): From bb0250263d5dd473d992ff67634fac7eda370eca Mon Sep 17 00:00:00 2001 From: Sepehr Date: Tue, 13 Sep 2022 17:42:22 +0430 Subject: [PATCH 18/22] Revert "refactor: set prefix and table name generate query" This reverts commit 3e986ed03c79ed9f81e9e3eb24fe779c777f2153. --- ormar/queryset/actions/order_action.py | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/ormar/queryset/actions/order_action.py b/ormar/queryset/actions/order_action.py index 62e939676..31971d988 100644 --- a/ormar/queryset/actions/order_action.py +++ b/ormar/queryset/actions/order_action.py @@ -98,10 +98,10 @@ def get_text_clause(self) -> sqlalchemy.sql.expression.TextClause: table_name = dialect.identifier_preparer.quote(table_name) field_name = dialect.identifier_preparer.quote(field_name) - field_name_direction_nulls = self._get_field_name_direction_nulls( - field_name=field_name, prefix=prefix, table_name=table_name + return text( + f"{prefix}{table_name}" + f".{self._get_field_name_direction_nulls(field_name=field_name)}" ) - return text(f"{prefix}{table_name}.{field_name_direction_nulls}") def _split_value_into_parts(self, order_str: str) -> None: if order_str.startswith("-"): @@ -129,27 +129,19 @@ def _generate_field_nulls_query(self, field_name: str, result: str) -> str: condition: str = "not" if self.nulls == "first" else "" # pragma: no cover return f"{field_name} is {condition} null, {result}" # pragma: no cover - def _get_field_name_direction_nulls( - self, field_name: str, prefix: str, table_name: str - ) -> str: + def _get_field_name_direction_nulls(self, field_name: str) -> str: """ Generate the Query of Order for this field name by direction and nulls value :param field_name: string name of this field for order :type field_name: str - :param prefix: string prefix name of this field for order - :type prefix: str - :param table_name: string table name of this field for order - :type table_name: str :return: result of the query by field name and direction and nulls value :rtype: str """ result: str = f"{field_name} {self.direction}" if self.nulls is not None: - return self._generate_field_nulls_query( - field_name=f"{prefix}{table_name}.{field_name}", result=result - ) + return self._generate_field_nulls_query(field_name=field_name, result=result) return result From ee4328a45f62f39c08337632f1b34d484a03eb98 Mon Sep 17 00:00:00 2001 From: Sepehr Date: Tue, 13 Sep 2022 17:46:40 +0430 Subject: [PATCH 19/22] fix: use get_field_name_text function field name --- ormar/queryset/actions/order_action.py | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/ormar/queryset/actions/order_action.py b/ormar/queryset/actions/order_action.py index 31971d988..44601e091 100644 --- a/ormar/queryset/actions/order_action.py +++ b/ormar/queryset/actions/order_action.py @@ -98,10 +98,7 @@ def get_text_clause(self) -> sqlalchemy.sql.expression.TextClause: table_name = dialect.identifier_preparer.quote(table_name) field_name = dialect.identifier_preparer.quote(field_name) - return text( - f"{prefix}{table_name}" - f".{self._get_field_name_direction_nulls(field_name=field_name)}" - ) + return text(f"{prefix}{table_name}.{self._get_field_direction_nulls()}") def _split_value_into_parts(self, order_str: str) -> None: if order_str.startswith("-"): @@ -111,12 +108,10 @@ def _split_value_into_parts(self, order_str: str) -> None: self.field_name = parts[-1] self.related_parts = parts[:-1] - def _generate_field_nulls_query(self, field_name: str, result: str) -> str: + def _generate_field_nulls_query(self, result: str) -> str: # pragma: no cover """ Generate the Final Query with handling mysql syntax for nulls value - :param field_name: string name of this field for order - :type field_name: str :param result: query generated in previous stage without nulls value :type result: str :return: result of the final query by field name and direction and nulls value @@ -124,24 +119,22 @@ def _generate_field_nulls_query(self, field_name: str, result: str) -> str: """ if not self.is_mysql_bool: - return result + f" nulls {self.nulls}" # pragma: no cover + return result + f" nulls {self.nulls}" - condition: str = "not" if self.nulls == "first" else "" # pragma: no cover - return f"{field_name} is {condition} null, {result}" # pragma: no cover + condition: str = "not" if self.nulls == "first" else "" + return f"{self.get_field_name_text()} is {condition} null, {result}" - def _get_field_name_direction_nulls(self, field_name: str) -> str: + def _get_field_direction_nulls(self) -> str: """ Generate the Query of Order for this field name by direction and nulls value - :param field_name: string name of this field for order - :type field_name: str :return: result of the query by field name and direction and nulls value :rtype: str """ - result: str = f"{field_name} {self.direction}" + result: str = f"{self.get_field_name_text()} {self.direction}" if self.nulls is not None: - return self._generate_field_nulls_query(field_name=field_name, result=result) + return self._generate_field_nulls_query(result=result) return result From d41131d03deac525cab58f0446abc50d82e0dd57 Mon Sep 17 00:00:00 2001 From: Sepehr Date: Tue, 13 Sep 2022 17:50:18 +0430 Subject: [PATCH 20/22] Revert "fix: use get_field_name_text function field name" This reverts commit ee4328a45f62f39c08337632f1b34d484a03eb98. --- ormar/queryset/actions/order_action.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/ormar/queryset/actions/order_action.py b/ormar/queryset/actions/order_action.py index 44601e091..31971d988 100644 --- a/ormar/queryset/actions/order_action.py +++ b/ormar/queryset/actions/order_action.py @@ -98,7 +98,10 @@ def get_text_clause(self) -> sqlalchemy.sql.expression.TextClause: table_name = dialect.identifier_preparer.quote(table_name) field_name = dialect.identifier_preparer.quote(field_name) - return text(f"{prefix}{table_name}.{self._get_field_direction_nulls()}") + return text( + f"{prefix}{table_name}" + f".{self._get_field_name_direction_nulls(field_name=field_name)}" + ) def _split_value_into_parts(self, order_str: str) -> None: if order_str.startswith("-"): @@ -108,10 +111,12 @@ def _split_value_into_parts(self, order_str: str) -> None: self.field_name = parts[-1] self.related_parts = parts[:-1] - def _generate_field_nulls_query(self, result: str) -> str: # pragma: no cover + def _generate_field_nulls_query(self, field_name: str, result: str) -> str: """ Generate the Final Query with handling mysql syntax for nulls value + :param field_name: string name of this field for order + :type field_name: str :param result: query generated in previous stage without nulls value :type result: str :return: result of the final query by field name and direction and nulls value @@ -119,22 +124,24 @@ def _generate_field_nulls_query(self, result: str) -> str: # pragma: no cover """ if not self.is_mysql_bool: - return result + f" nulls {self.nulls}" + return result + f" nulls {self.nulls}" # pragma: no cover - condition: str = "not" if self.nulls == "first" else "" - return f"{self.get_field_name_text()} is {condition} null, {result}" + condition: str = "not" if self.nulls == "first" else "" # pragma: no cover + return f"{field_name} is {condition} null, {result}" # pragma: no cover - def _get_field_direction_nulls(self) -> str: + def _get_field_name_direction_nulls(self, field_name: str) -> str: """ Generate the Query of Order for this field name by direction and nulls value + :param field_name: string name of this field for order + :type field_name: str :return: result of the query by field name and direction and nulls value :rtype: str """ - result: str = f"{self.get_field_name_text()} {self.direction}" + result: str = f"{field_name} {self.direction}" if self.nulls is not None: - return self._generate_field_nulls_query(result=result) + return self._generate_field_nulls_query(field_name=field_name, result=result) return result From 9690176944dfa08bd6ff9fa904c5d325c80701e7 Mon Sep 17 00:00:00 2001 From: Sepehr Date: Tue, 13 Sep 2022 17:59:03 +0430 Subject: [PATCH 21/22] fix: debug number of toys created --- tests/test_queries/test_order_by.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/test_queries/test_order_by.py b/tests/test_queries/test_order_by.py index 317efc55f..aaf447f94 100644 --- a/tests/test_queries/test_order_by.py +++ b/tests/test_queries/test_order_by.py @@ -297,17 +297,17 @@ async def test_sort_order_on_related_model(): akbar = await Owner.objects.create(name="Akbar", age=22) asqar = await Owner.objects.create(name="Asqar", age=18) - await Toy.objects.create(name="Toy 7", owner=akbar) - await Toy.objects.create(name="Toy 8", owner=asqar) + await Toy.objects.create(name="Toy 8", owner=akbar) + await Toy.objects.create(name="Toy 9", owner=asqar) toys = ( await Toy.objects.select_related("owner") .order_by(Toy.owner.age.desc(nulls_ordering=ormar.NullsOrdering.LAST)) .all() ) - assert len(toys) == 10 - assert toys[0].name == "Toy 7" - assert toys[1].name == "Toy 8" + assert len(toys) == 9 + assert toys[0].name == "Toy 8" + assert toys[1].name == "Toy 9" @pytest.mark.asyncio From 93d520d56de4c4911a2f2891a61f6f41cb64f0d9 Mon Sep 17 00:00:00 2001 From: Sepehr Date: Mon, 27 Mar 2023 23:11:26 +0430 Subject: [PATCH 22/22] refactor: rename self.nulls_ordering variable --- ormar/queryset/actions/order_action.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ormar/queryset/actions/order_action.py b/ormar/queryset/actions/order_action.py index 31971d988..b25e1d6c2 100644 --- a/ormar/queryset/actions/order_action.py +++ b/ormar/queryset/actions/order_action.py @@ -35,7 +35,7 @@ def __init__( if self.source_model == self.target_model and "__" not in self.related_str: self.is_source_model_order = True - self.nulls = nulls_ordering if nulls_ordering is not None else None + self.nulls_ordering = nulls_ordering @property def field_alias(self) -> str: @@ -124,9 +124,9 @@ def _generate_field_nulls_query(self, field_name: str, result: str) -> str: """ if not self.is_mysql_bool: - return result + f" nulls {self.nulls}" # pragma: no cover + return result + f" nulls {self.nulls_ordering}" # pragma: no cover - condition: str = "not" if self.nulls == "first" else "" # pragma: no cover + condition: str = "not" if self.nulls_ordering == "first" else "" # pragma: no cover return f"{field_name} is {condition} null, {result}" # pragma: no cover def _get_field_name_direction_nulls(self, field_name: str) -> str: @@ -140,7 +140,7 @@ def _get_field_name_direction_nulls(self, field_name: str) -> str: """ result: str = f"{field_name} {self.direction}" - if self.nulls is not None: + if self.nulls_ordering is not None: return self._generate_field_nulls_query(field_name=field_name, result=result) return result