fix: order using foreign table (#581)

Author: grdsdev

infra/init.sql

@@ -76,3 +76,24 @@ create or replace function public.list_stored_countries()
 as $function$
     select * from countries;
 $function$;
+
+
+create table
+  orchestral_sections (id int8 primary key, name text);
+create table
+  instruments (
+    id int8 primary key,
+    section_id int8 not null references orchestral_sections,
+    name text
+  );
+
+insert into
+  orchestral_sections (id, name)
+values
+  (1, 'strings'),
+  (2, 'woodwinds');
+insert into
+  instruments (id, section_id, name)
+values
+  (1, 1, 'harp'),
+  (2, 1, 'violin');

postgrest/base_request_builder.py

@@ -566,7 +566,7 @@ def order(
         column: str,
         *,
         desc: bool = False,
-        nullsfirst: bool = False,
+        nullsfirst: Optional[bool] = None,
         foreign_table: Optional[str] = None,
     ) -> Self:
         """Sort the returned rows in some specific order.
@@ -579,20 +579,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(
-            "order",
-            new_order_parameter,
+        key = f"{foreign_table}.order" if foreign_table else "order"
+        existing_order = self.params.get(key)
+
+        self.params = self.params.set(
+            key,
+            f"{existing_order + ',' if existing_order else ''}"
+            + f"{column}.{'desc' if desc else 'asc'}"
+            + (
+                f".{'nullsfirst' if nullsfirst else 'nullslast'}"
+                if nullsfirst is not None
+                else ""
+            ),
         )
         return self
 

tests/_async/test_filter_request_builder_integration.py

@@ -498,3 +498,18 @@ async def test_order():
         {"country_name": "UNITED STATES", "iso": "US"},
         {"country_name": "UNITED KINGDOM", "iso": "GB"},
     ]
+
+
+async def test_order_on_foreign_table():
+    res = (
+        await rest_client()
+        .from_("orchestral_sections")
+        .select("name, instruments(name)")
+        .order("name", desc=True, foreign_table="instruments")
+        .execute()
+    )
+
+    assert res.data == [
+        {"name": "strings", "instruments": [{"name": "violin"}, {"name": "harp"}]},
+        {"name": "woodwinds", "instruments": []},
+    ]

tests/_async/test_request_builder.py

@@ -218,10 +218,7 @@ def test_multiple_orders_on_foreign_table(self, request_builder: AsyncRequestBui
             .order("city_name", desc=True, foreign_table=foreign_table)
             .order("id", desc=True, foreign_table=foreign_table)
         )
-        assert (
-            str(builder.params)
-            == "select=%2A&order=cities%28city_name%29.desc%2Ccities%28id%29.desc"
-        )
+        assert str(builder.params) == "select=%2A&cities.order=city_name.desc%2Cid.desc"
 
 
 class TestRange:

tests/_sync/test_filter_request_builder_integration.py

@@ -491,3 +491,18 @@ def test_order():
         {"country_name": "UNITED STATES", "iso": "US"},
         {"country_name": "UNITED KINGDOM", "iso": "GB"},
     ]
+
+
+def test_order_on_foreign_table():
+    res = (
+        rest_client()
+        .from_("orchestral_sections")
+        .select("name, instruments(name)")
+        .order("name", desc=True, foreign_table="instruments")
+        .execute()
+    )
+
+    assert res.data == [
+        {"name": "strings", "instruments": [{"name": "violin"}, {"name": "harp"}]},
+        {"name": "woodwinds", "instruments": []},
+    ]

tests/_sync/test_request_builder.py

@@ -218,10 +218,7 @@ def test_multiple_orders_on_foreign_table(self, request_builder: SyncRequestBuil
             .order("city_name", desc=True, foreign_table=foreign_table)
             .order("id", desc=True, foreign_table=foreign_table)
         )
-        assert (
-            str(builder.params)
-            == "select=%2A&order=cities%28city_name%29.desc%2Ccities%28id%29.desc"
-        )
+        assert str(builder.params) == "select=%2A&cities.order=city_name.desc%2Cid.desc"
 
 
 class TestRange: