From 9441cade7039f780fb96411557680bb8d5d605f3 Mon Sep 17 00:00:00 2001 From: nikdedov Date: Mon, 19 Aug 2024 20:27:19 +0100 Subject: [PATCH 1/4] Fixed the 'order' method for 'BaseSelectRequestBuilder' to support multiple sorting criteria and handle sorting on foreign tables. --- postgrest/base_request_builder.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/postgrest/base_request_builder.py b/postgrest/base_request_builder.py index 08679449..f0233408 100644 --- a/postgrest/base_request_builder.py +++ b/postgrest/base_request_builder.py @@ -563,9 +563,18 @@ def order( .. versionchanged:: 0.10.3 Allow ordering results for foreign tables with the foreign_table parameter. """ + + new_order_parameter = (f"{foreign_table + '(' if foreign_table else ''}{column}{')' if foreign_table else ''}" + f"{'.desc' if desc else ''}{'.nullsfirst' if nullsfirst else ''}") + + existing_order_parameter = self.params.get('order') + if existing_order_parameter: + self.params = self.params.remove('order') + new_order_parameter = f"{existing_order_parameter},{new_order_parameter}" + self.params = self.params.add( - f"{foreign_table}.order" if foreign_table else "order", - f"{column}{'.desc' if desc else ''}{'.nullsfirst' if nullsfirst else ''}", + "order", + new_order_parameter, ) return self From e1c5bc31844139752fdf37a2a1273b40b3bbc3cc Mon Sep 17 00:00:00 2001 From: Andrew Smith Date: Tue, 20 Aug 2024 22:30:38 +0000 Subject: [PATCH 2/4] style: format the order method code --- postgrest/base_request_builder.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/postgrest/base_request_builder.py b/postgrest/base_request_builder.py index f0233408..f8c5a324 100644 --- a/postgrest/base_request_builder.py +++ b/postgrest/base_request_builder.py @@ -564,12 +564,14 @@ def order( Allow ordering results for foreign tables with the foreign_table parameter. """ - new_order_parameter = (f"{foreign_table + '(' if foreign_table else ''}{column}{')' if foreign_table else ''}" - f"{'.desc' if desc else ''}{'.nullsfirst' if nullsfirst else ''}") + new_order_parameter = ( + f"{foreign_table + '(' if foreign_table else ''}{column}{')' if foreign_table else ''}" + f"{'.desc' if desc else ''}{'.nullsfirst' if nullsfirst else ''}" + ) - existing_order_parameter = self.params.get('order') + existing_order_parameter = self.params.get("order") if existing_order_parameter: - self.params = self.params.remove('order') + self.params = self.params.remove("order") new_order_parameter = f"{existing_order_parameter},{new_order_parameter}" self.params = self.params.add( From 1811479621b1e9068fdf0d2dd971ae5c3e77b93c Mon Sep 17 00:00:00 2001 From: Andrew Smith Date: Thu, 22 Aug 2024 09:13:43 +0000 Subject: [PATCH 3/4] chore: add tests for order --- ...test_filter_request_builder_integration.py | 17 +++++++++ tests/_async/test_request_builder.py | 14 ++++++++ ...test_filter_request_builder_integration.py | 17 +++++++++ tests/_sync/test_request_builder.py | 36 +++++++++++++------ 4 files changed, 73 insertions(+), 11 deletions(-) diff --git a/tests/_async/test_filter_request_builder_integration.py b/tests/_async/test_filter_request_builder_integration.py index a7341f96..d8ed1553 100644 --- a/tests/_async/test_filter_request_builder_integration.py +++ b/tests/_async/test_filter_request_builder_integration.py @@ -481,3 +481,20 @@ async def test_rpc_with_range(): {"nicename": "Albania", "iso": "AL"}, {"nicename": "Algeria", "iso": "DZ"}, ] + + +async def test_order(): + res = ( + await rest_client() + .from_("countries") + .select("country_name, iso") + .limit(3) + .order("nicename", desc=True) + .execute() + ) + + assert res.data == [ + {"country_name": "ZIMBABWE", "iso": "ZW"}, + {"country_name": "UNITED STATES", "iso": "US"}, + {"country_name": "UNITED KINGDOM", "iso": "GB"}, + ] diff --git a/tests/_async/test_request_builder.py b/tests/_async/test_request_builder.py index fd3c17ed..0970040e 100644 --- a/tests/_async/test_request_builder.py +++ b/tests/_async/test_request_builder.py @@ -190,6 +190,20 @@ def test_explain_options(self, request_builder: AsyncRequestBuilder): assert "options=analyze|verbose|buffers|wal" in str(builder.headers.get("accept")) +class TestOrder: + def test_order(self, request_builder: AsyncRequestBuilder): + builder = request_builder.select().order("country_name", desc=True) + assert str(builder.params) == "order=country_name.desc" + + def test_multiple_orders(self, request_builder: AsyncRequestBuilder): + builder = ( + request_builder.select() + .order("country_name", desc=True) + .order("iso", desc=True) + ) + assert str(builder.params) == "order=country_name.desc%2Ciso.desc" + + class TestRange: def test_range_on_own_table(self, request_builder: AsyncRequestBuilder): builder = request_builder.select("*").range(0, 1) diff --git a/tests/_sync/test_filter_request_builder_integration.py b/tests/_sync/test_filter_request_builder_integration.py index e612a8e6..f508587c 100644 --- a/tests/_sync/test_filter_request_builder_integration.py +++ b/tests/_sync/test_filter_request_builder_integration.py @@ -474,3 +474,20 @@ def test_rpc_with_range(): {"nicename": "Albania", "iso": "AL"}, {"nicename": "Algeria", "iso": "DZ"}, ] + + +def test_order(): + res = ( + rest_client() + .from_("countries") + .select("country_name, iso") + .limit(3) + .order("nicename", desc=True) + .execute() + ) + + assert res.data == [ + {"country_name": "ZIMBABWE", "iso": "ZW"}, + {"country_name": "UNITED STATES", "iso": "US"}, + {"country_name": "UNITED KINGDOM", "iso": "GB"}, + ] diff --git a/tests/_sync/test_request_builder.py b/tests/_sync/test_request_builder.py index 912e8f42..7c37dddc 100644 --- a/tests/_sync/test_request_builder.py +++ b/tests/_sync/test_request_builder.py @@ -71,6 +71,17 @@ def test_insert_with_upsert(self, request_builder: SyncRequestBuilder): assert builder.http_method == "POST" assert builder.json == {"key1": "val1"} + def test_upsert_with_default_single(self, request_builder: SyncRequestBuilder): + builder = request_builder.upsert([{"key1": "val1"}], default_to_null=False) + assert builder.headers.get_list("prefer", True) == [ + "return=representation", + "resolution=merge-duplicates", + "missing=default", + ] + assert builder.http_method == "POST" + assert builder.json == [{"key1": "val1"}] + assert builder.params.get("columns") == '"key1"' + def test_bulk_insert_using_default(self, request_builder: SyncRequestBuilder): builder = request_builder.insert( [{"key1": "val1", "key2": "val2"}, {"key3": "val3"}], default_to_null=False @@ -95,17 +106,6 @@ def test_upsert(self, request_builder: SyncRequestBuilder): assert builder.http_method == "POST" assert builder.json == {"key1": "val1"} - def test_upsert_with_default_single(self, request_builder: SyncRequestBuilder): - builder = request_builder.upsert([{"key1": "val1"}], default_to_null=False) - assert builder.headers.get_list("prefer", True) == [ - "return=representation", - "resolution=merge-duplicates", - "missing=default", - ] - assert builder.http_method == "POST" - assert builder.json == [{"key1": "val1"}] - assert builder.params.get("columns") == '"key1"' - def test_bulk_upsert_with_default(self, request_builder: SyncRequestBuilder): builder = request_builder.upsert( [{"key1": "val1", "key2": "val2"}, {"key3": "val3"}], default_to_null=False @@ -190,6 +190,20 @@ def test_explain_options(self, request_builder: SyncRequestBuilder): assert "options=analyze|verbose|buffers|wal" in str(builder.headers.get("accept")) +class TestOrder: + def test_order(self, request_builder: SyncRequestBuilder): + builder = request_builder.select().order("country_name", desc=True) + assert str(builder.params) == "order=country_name.desc" + + def test_multiple_orders(self, request_builder: SyncRequestBuilder): + builder = ( + request_builder.select() + .order("country_name", desc=True) + .order("iso", desc=True) + ) + assert str(builder.params) == "order=country_name.desc%2Ciso.desc" + + class TestRange: def test_range_on_own_table(self, request_builder: SyncRequestBuilder): builder = request_builder.select("*").range(0, 1) From c50ec9f2f7ba0038ec420168b838184e91229b74 Mon Sep 17 00:00:00 2001 From: Andrew Smith Date: Thu, 22 Aug 2024 10:05:31 +0000 Subject: [PATCH 4/4] chore: add test for order on foreign table --- tests/_async/test_request_builder.py | 12 ++++++++++++ tests/_sync/test_request_builder.py | 12 ++++++++++++ 2 files changed, 24 insertions(+) diff --git a/tests/_async/test_request_builder.py b/tests/_async/test_request_builder.py index 0970040e..bd55e140 100644 --- a/tests/_async/test_request_builder.py +++ b/tests/_async/test_request_builder.py @@ -203,6 +203,18 @@ def test_multiple_orders(self, request_builder: AsyncRequestBuilder): ) assert str(builder.params) == "order=country_name.desc%2Ciso.desc" + def test_multiple_orders_on_foreign_table(self, request_builder: AsyncRequestBuilder): + foreign_table = "cities" + builder = ( + request_builder.select() + .order("city_name", desc=True, foreign_table=foreign_table) + .order("id", desc=True, foreign_table=foreign_table) + ) + assert ( + str(builder.params) + == "order=cities%28city_name%29.desc%2Ccities%28id%29.desc" + ) + class TestRange: def test_range_on_own_table(self, request_builder: AsyncRequestBuilder): diff --git a/tests/_sync/test_request_builder.py b/tests/_sync/test_request_builder.py index 7c37dddc..45657ed9 100644 --- a/tests/_sync/test_request_builder.py +++ b/tests/_sync/test_request_builder.py @@ -203,6 +203,18 @@ def test_multiple_orders(self, request_builder: SyncRequestBuilder): ) assert str(builder.params) == "order=country_name.desc%2Ciso.desc" + def test_multiple_orders_on_foreign_table(self, request_builder: SyncRequestBuilder): + foreign_table = "cities" + builder = ( + request_builder.select() + .order("city_name", desc=True, foreign_table=foreign_table) + .order("id", desc=True, foreign_table=foreign_table) + ) + assert ( + str(builder.params) + == "order=cities%28city_name%29.desc%2Ccities%28id%29.desc" + ) + class TestRange: def test_range_on_own_table(self, request_builder: SyncRequestBuilder):