add ordering on spread embeds without the need of selecting them

docs/references/api/resource_embedding.rst

@@ -1270,5 +1270,3 @@ You can still order all the resulting arrays explicitly. For example, to order b
        ]
      }
    ]
-
-Note that the field must be selected in the spread relationship for the order to work.

src/PostgREST/Plan.hs

@@ -330,6 +330,7 @@ readPlan qi@QualifiedIdentifier{..} AppConfig{configDbMaxRows, configDbAggregate
     mapLeft ApiRequestError $
     treeRestrictRange configDbMaxRows (iAction apiRequest) =<<
     hoistSpreadAggFunctions =<<
+    addToManySpreadOrderSelects =<<
     validateAggFunctions configDbAggregates =<<
     addRelSelects =<<
     addNullEmbedFilters =<<
@@ -429,7 +430,7 @@ expandStars ctx rPlanTree = Right $ expandStarsForReadPlan False rPlanTree
     expandStarsForReadPlan :: Bool -> ReadPlanTree -> ReadPlanTree
     expandStarsForReadPlan hasAgg (Node rp@ReadPlan{select, from=fromQI, fromAlias=alias, relSpread=spread} children) =
       let
-        newHasAgg = hasAgg || any (isJust . csAggFunction) select || spread == Just ToManySpread
+        newHasAgg = hasAgg || any (isJust . csAggFunction) select || case spread of Just ToManySpread{} -> True; _ -> False
         newCtx = adjustContext ctx fromQI alias
         newRPlan = expandStarsForTable newCtx newHasAgg rp
       in Node newRPlan (map (expandStarsForReadPlan newHasAgg) children)
@@ -485,7 +486,7 @@ addRels schema action allRels parentNode (Node rPlan@ReadPlan{relName,relHint,re
         newReadPlan = (\r ->
           let newAlias = Just (qiName (relForeignTable r) <> "_" <> show depth)
               aggAlias = qiName (relTable r) <> "_" <> fromMaybe relName relAlias <> "_" <> show depth
-              updSpread = if isJust relSpread && not (relIsToOne r) then Just ToManySpread else relSpread in
+              updSpread = if isJust relSpread && not (relIsToOne r) then Just $ ToManySpread [] [] else relSpread in
           case r of
             Relationship{relCardinality=M2M _} -> -- m2m does internal implicit joins that don't need aliasing
               rPlan{from=relForeignTable r, relToParent=Just r, relAggAlias=aggAlias, relJoinConds=getJoinConditions Nothing parentAlias r, relSpread=updSpread}
@@ -748,6 +749,29 @@ hoistIntoRelSelectFields aggList r@(Spread {rsSpreadSel = spreadSelects, rsAggAl
             Nothing -> s
 hoistIntoRelSelectFields _ r = r
 
+-- | Handle ordering in a To-Many Spread Relationship
+-- In case of a To-Many Spread, it removes the ordering done in the ReadPlan and moves it to the SpreadType.
+-- We also select the ordering columns and alias them to avoid collisions. This is because it would be impossible
+-- to order once it's aggregated if it's not selected in the inner query beforehand.
+addToManySpreadOrderSelects :: ReadPlanTree -> Either ApiRequestError ReadPlanTree
+addToManySpreadOrderSelects (Node rp@ReadPlan { order, relAggAlias, relSpread = Just ToManySpread {}} forest) =
+  Node rp { order = [], relSpread = newRelSpread } <$> addToManySpreadOrderSelects `traverse` forest
+  where
+    newRelSpread = Just ToManySpread { stExtraSelect = addSprExtraSelects, stOrder = addSprOrder}
+    (addSprExtraSelects, addSprOrder) = unzip $ zipWith ordToExtraSelsAndSprOrds [1..] order
+    ordToExtraSelsAndSprOrds i = \case
+      CoercibleOrderTerm fld dir ordr -> (
+          (Nothing, CoercibleSelectField fld Nothing Nothing Nothing (Just $ selOrdAlias (cfName fld) i)),
+          CoercibleOrderTerm (unknownField (selOrdAlias (cfName fld) i) []) dir ordr
+        )
+      CoercibleOrderRelationTerm rel (fld,jp) dir ordr -> (
+          (Just rel, CoercibleSelectField (unknownField fld jp) Nothing Nothing Nothing (Just $ selOrdAlias fld i)),
+          CoercibleOrderTerm (unknownField (selOrdAlias fld i) []) dir ordr
+        )
+    selOrdAlias :: Alias -> Integer -> Alias
+    selOrdAlias name i = relAggAlias <> "_" <> name <> "_" <> show i -- add index to avoid collisions in aliases
+addToManySpreadOrderSelects (Node rp forest) = Node rp <$> addToManySpreadOrderSelects `traverse` forest
+
 validateAggFunctions :: Bool -> ReadPlanTree -> Either ApiRequestError ReadPlanTree
 validateAggFunctions aggFunctionsAllowed (Node rp@ReadPlan {select} forest)
   | not aggFunctionsAllowed && any (isJust . csAggFunction) select = Left AggregatesNotAllowed

src/PostgREST/Plan/ReadPlan.hs

@@ -12,7 +12,8 @@ import PostgREST.ApiRequest.Types         (Alias, Depth, Hint,
 import PostgREST.Plan.Types               (CoercibleLogicTree,
                                            CoercibleOrderTerm,
                                            CoercibleSelectField (..),
-                                           RelSelectField (..))
+                                           RelSelectField (..),
+                                           SpreadType (..))
 import PostgREST.RangeQuery               (NonnegRange)
 import PostgREST.SchemaCache.Identifiers  (FieldName,
                                            QualifiedIdentifier)
@@ -29,9 +30,6 @@ data JoinCondition =
     (QualifiedIdentifier, FieldName)
   deriving (Eq, Show)
 
-data SpreadType = ToOneSpread | ToManySpread
-  deriving (Eq, Show)
-
 data ReadPlan = ReadPlan
   { select       :: [CoercibleSelectField]
   , from         :: QualifiedIdentifier

src/PostgREST/Plan/Types.hs

@@ -9,6 +9,7 @@ module PostgREST.Plan.Types
   , RelSelectField(..)
   , RelJsonEmbedMode(..)
   , SpreadSelectField(..)
+  , SpreadType(..)
   ) where
 
 import PostgREST.ApiRequest.Types (AggregateFunction, Alias, Cast,
@@ -105,3 +106,11 @@ data SpreadSelectField =
   , ssSelAlias       :: Maybe Alias
   }
   deriving (Eq, Show)
+
+data SpreadType
+  = ToOneSpread
+  | ToManySpread
+    { stExtraSelect :: [(Maybe FieldName, CoercibleSelectField)]
+    , stOrder       :: [CoercibleOrderTerm]
+    }
+  deriving (Eq, Show)

src/PostgREST/Query/QueryBuilder.hs

@@ -47,23 +47,26 @@ import Protolude
 readPlanToQuery :: ReadPlanTree -> SQL.Snippet
 readPlanToQuery node@(Node ReadPlan{select,from=mainQi,fromAlias,where_=logicForest,order, range_=readRange, relToParent, relJoinConds, relSelect, relSpread} forest) =
   "SELECT " <>
-  intercalateSnippet ", " ((pgFmtSelectItem qi <$> (if null select && null forest then defSelect else select)) ++ joinsSelects) <> " " <>
+  intercalateSnippet ", " (selects ++ sprExtraSelects ++ joinsSelects) <> " " <>
   fromFrag <> " " <>
   intercalateSnippet " " joins <> " " <>
   (if null logicForest && null relJoinConds
     then mempty
     else "WHERE " <> intercalateSnippet " AND " (map (pgFmtLogicTree qi) logicForest ++ map pgFmtJoinCondition relJoinConds)) <> " " <>
   groupF qi select relSelect <> " " <>
-  orderFrag <> " " <>
+  orderF qi order <> " " <>
   limitOffsetF readRange
   where
     fromFrag = fromF relToParent mainQi fromAlias
     qi = getQualifiedIdentifier relToParent mainQi fromAlias
     -- gets all the columns in case of an empty select, ignoring/obtaining these columns is done at the aggregation stage
     defSelect = [CoercibleSelectField (unknownField "*" []) Nothing Nothing Nothing Nothing]
     joins = getJoins node
+    selects = pgFmtSelectItem qi <$> (if null select && null forest then defSelect else select)
     joinsSelects = getJoinSelects node
-    orderFrag = if relSpread == Just ToManySpread then mempty else orderF qi order
+    sprExtraSelects = case relSpread of
+      Just (ToManySpread sels _) -> (\s -> pgFmtSelectItem (maybe qi (QualifiedIdentifier "") $ fst s) $ snd s) <$> sels
+      _ -> mempty
 
 getJoinSelects :: ReadPlanTree -> [SQL.Snippet]
 getJoinSelects (Node ReadPlan{relSelect} _) =
@@ -93,7 +96,7 @@ getJoins (Node ReadPlan{relSelect} forest) =
       ) relSelect
 
 getJoin :: RelSelectField -> ReadPlanTree -> SQL.Snippet
-getJoin fld node@(Node ReadPlan{order, relJoinType, relSpread} _) =
+getJoin fld node@(Node ReadPlan{relJoinType, relSpread} _) =
   let
     correlatedSubquery sub al cond =
       (if relJoinType == Just JTInner then "INNER" else "LEFT") <> " JOIN LATERAL ( " <> sub <> " ) AS " <> al <> " ON " <> cond
@@ -107,11 +110,12 @@ getJoin fld node@(Node ReadPlan{order, relJoinType, relSpread} _) =
       JsonEmbed{rsEmbedMode = JsonObject} ->
         correlatedSubquery subquery aggAlias "TRUE"
       Spread{rsSpreadSel, rsAggAlias} ->
-        if relSpread == Just ToManySpread then
-          let selSpread = selectSubqAgg <> (if null rsSpreadSel then mempty else ", ") <> intercalateSnippet ", " (pgFmtSpreadJoinSelectItem rsAggAlias order <$> rsSpreadSel)
-          in correlatedSubquery (selSpread <> fromSubqAgg) aggAlias joinCondition
-        else
-          correlatedSubquery subquery aggAlias "TRUE"
+        case relSpread of
+          Just (ToManySpread _ sprOrder) ->
+            let selSpread = selectSubqAgg <> (if null rsSpreadSel then mempty else ", ") <> intercalateSnippet ", " (pgFmtSpreadJoinSelectItem rsAggAlias sprOrder <$> rsSpreadSel)
+            in correlatedSubquery (selSpread <> fromSubqAgg) aggAlias joinCondition
+          _ ->
+            correlatedSubquery subquery aggAlias "TRUE"
       JsonEmbed{rsEmbedMode = JsonArray} ->
         correlatedSubquery (selectSubqAgg <> fromSubqAgg) aggAlias joinCondition
 

test/spec/Feature/Query/SpreadQueriesSpec.hs

@@ -290,7 +290,7 @@ spec =
           { matchStatus  = 200
           , matchHeaders = [matchContentTypeJson]
           }
-      it "orders ALL the resulting arrays according to the specified order in the spread relationship" $
+      it "orders all the resulting arrays according to the spread relationship ordering columns" $ do
         get "/factories?select=factory:name,...processes(*)&processes.order=category_id.asc,name.desc&order=name" `shouldRespondWith`
           [json|[
             {"factory":"Factory A","id":[1, 2],"name":["Process A1", "Process A2"],"factory_id":[1, 1],"category_id":[1, 2]},
@@ -301,6 +301,38 @@ spec =
           { matchStatus  = 200
           , matchHeaders = [matchContentTypeJson]
           }
+        get "/factories?select=factory:name,...factory_buildings(*)&factory_buildings.order=inspections->pending.asc.nullsfirst,inspections->ins.desc&order=name" `shouldRespondWith`
+          [json|[
+            {"factory":"Factory A","id":[2, 1],"code":["A002", "A001"],"size":[200, 150],"type":["A", "A"],"factory_id":[1, 1],"inspections":[{"ins": "2025A", "pending": true}, {"ins": "2024C", "pending": true}]},
+            {"factory":"Factory B","id":[4, 3],"code":["B002", "B001"],"size":[120, 50],"type":["C", "B"],"factory_id":[2, 2],"inspections":[{"ins": "2023A"}, {"ins": "2025A", "pending": true}]},
+            {"factory":"Factory C","id":[5],"code":["C001"],"size":[240],"type":["B"],"factory_id":[3],"inspections":[{"ins": "2022B"}]},
+            {"factory":"Factory D","id":[6],"code":["D001"],"size":[310],"type":["A"],"factory_id":[4],"inspections":[{"ins": "2024C", "pending": true}]}
+          ]|]
+          { matchStatus  = 200
+          , matchHeaders = [matchContentTypeJson]
+          }
+      it "orders all the resulting arrays according to the spread relationship ordering columns even if they aren't selected" $
+        get "/factories?select=factory:name,...processes(name)&processes.order=category_id.asc,name.desc&order=name" `shouldRespondWith`
+          [json|[
+            {"factory":"Factory A","name":["Process A1", "Process A2"]},
+            {"factory":"Factory B","name":["Process B2", "Process B1"]},
+            {"factory":"Factory C","name":["Process YY", "Process XX", "Process C2", "Process C1"]},
+            {"factory":"Factory D","name":[]}
+          ]|]
+          { matchStatus  = 200
+          , matchHeaders = [matchContentTypeJson]
+          }
+      it "orders all the resulting arrays according to the related ordering columns in the spread relationship" $
+        get "/factories?select=factory:name,...processes(name,...process_costs(cost))&processes.order=process_costs(cost)&order=name" `shouldRespondWith`
+          [json|[
+            {"factory":"Factory A","name":["Process A1", "Process A2"],"cost":[150.00, 200.00]},
+            {"factory":"Factory B","name":["Process B2", "Process B1"],"cost":[70.00, 180.00]},
+            {"factory":"Factory C","name":["Process C1", "Process YY", "Process C2", "Process XX"],"cost":[40.00, 40.00, 70.00, null]},
+            {"factory":"Factory D","name":[],"cost":[]}
+           ]|]
+          { matchStatus  = 200
+          , matchHeaders = [matchContentTypeJson]
+          }
 
     context "many-to-many relationships" $ do
       it "should spread a column as a json array" $ do
@@ -518,7 +550,7 @@ spec =
           { matchStatus  = 200
           , matchHeaders = [matchContentTypeJson]
           }
-      it "orders ALL the resulting arrays according to the specified order in the spread relationship" $
+      it "orders all the resulting arrays according to the spread relationship ordering columns" $ do
         get "/supervisors?select=supervisor:name,...processes(*)&processes.order=category_id.asc,name.desc&order=name" `shouldRespondWith`
           [json|[
             {"supervisor":"Jane","id":[],"name":[],"factory_id":[],"category_id":[]},
@@ -530,3 +562,41 @@ spec =
           { matchStatus  = 200
           , matchHeaders = [matchContentTypeJson]
           }
+        get "/processes?select=process:name,...operators(*)&operators.order=status->afk.asc.nullsfirst,status->id.desc&order=name" `shouldRespondWith`
+          [json|[
+            {"process":"Process A1","id":[2, 1],"name":["Louis", "Anne"],"status":[{"id": "012345"}, {"id": "543210", "afk": true}]},
+            {"process":"Process A2","id":[2, 3, 1],"name":["Louis", "Jeff", "Anne"],"status":[{"id": "012345"}, {"id": "666666", "afk": true}, {"id": "543210", "afk": true}]},
+            {"process":"Process B1","id":[3],"name":["Jeff"],"status":[{"id": "666666", "afk": true}]},
+            {"process":"Process B2","id":[3, 1],"name":["Jeff", "Anne"],"status":[{"id": "666666", "afk": true}, {"id": "543210", "afk": true}]},
+            {"process":"Process C1","id":[],"name":[],"status":[]},
+            {"process":"Process C2","id":[5, 3],"name":["Alfred", "Jeff"],"status":[{"id": "000000"}, {"id": "666666", "afk": true}]},
+            {"process":"Process XX","id":[5],"name":["Alfred"],"status":[{"id": "000000"}]},
+            {"process":"Process YY","id":[],"name":[],"status":[]}
+          ]|]
+          { matchStatus  = 200
+          , matchHeaders = [matchContentTypeJson]
+          }
+      it "orders all the resulting arrays according to the spread relationship ordering columns even if they aren't selected" $
+        get "/supervisors?select=supervisor:name,...processes(name)&processes.order=category_id.asc,name.desc&order=name" `shouldRespondWith`
+          [json|[
+            {"supervisor":"Jane","name":[]},
+            {"supervisor":"John","name":["Process B2", "Process A2"]},
+            {"supervisor":"Mary","name":["Process B2", "Process A1"]},
+            {"supervisor":"Peter","name":["Process B1", "Process C2", "Process C1"]},
+            {"supervisor":"Sarah","name":["Process B1"]}
+          ]|]
+          { matchStatus  = 200
+          , matchHeaders = [matchContentTypeJson]
+          }
+      it "orders all the resulting arrays according to the related ordering columns in the spread relationship" $
+        get "/supervisors?select=supervisor:name,...processes(name,...process_costs(cost))&processes.order=process_costs(cost)&order=name" `shouldRespondWith`
+          [json|[
+            {"supervisor":"Jane","name":[],"cost":[]},
+            {"supervisor":"John","name":["Process B2", "Process A2"],"cost":[70.00, 200.00]},
+            {"supervisor":"Mary","name":["Process B2", "Process A1"],"cost":[70.00, 150.00]},
+            {"supervisor":"Peter","name":["Process C1", "Process C2", "Process B1"],"cost":[40.00, 70.00, 180.00]},
+            {"supervisor":"Sarah","name":["Process B1"],"cost":[180.00]}
+          ]|]
+          { matchStatus  = 200
+          , matchHeaders = [matchContentTypeJson]
+          }

test/spec/fixtures/data.sql

@@ -931,11 +931,11 @@ INSERT INTO process_supervisor VALUES (5, 3);
 INSERT INTO process_supervisor VALUES (6, 3);
 
 TRUNCATE TABLE operators CASCADE;
-INSERT INTO operators VALUES (1, 'Anne');
-INSERT INTO operators VALUES (2, 'Louis');
-INSERT INTO operators VALUES (3, 'Jeff');
-INSERT INTO operators VALUES (4, 'Liz');
-INSERT INTO operators VALUES (5, 'Alfred');
+INSERT INTO operators VALUES (1, 'Anne', '{"id": "543210", "afk": true}');
+INSERT INTO operators VALUES (2, 'Louis', '{"id": "012345"}');
+INSERT INTO operators VALUES (3, 'Jeff', '{"id": "666666", "afk": true}');
+INSERT INTO operators VALUES (4, 'Liz', '{"id": "999999"}');
+INSERT INTO operators VALUES (5, 'Alfred', '{"id": "000000"}');
 
 TRUNCATE TABLE process_operator CASCADE;
 INSERT INTO process_operator VALUES (1,1);
@@ -951,12 +951,12 @@ INSERT INTO process_operator VALUES (6,5);
 INSERT INTO process_operator VALUES (7,5);
 
 TRUNCATE TABLE factory_buildings CASCADE;
-INSERT INTO factory_buildings VALUES (1, 'A001', 150, 'A', 1);
-INSERT INTO factory_buildings VALUES (2, 'A002', 200, 'A', 1);
-INSERT INTO factory_buildings VALUES (3, 'B001', 50, 'B', 2);
-INSERT INTO factory_buildings VALUES (4, 'B002', 120, 'C', 2);
-INSERT INTO factory_buildings VALUES (5, 'C001', 240, 'B', 3);
-INSERT INTO factory_buildings VALUES (6, 'D001', 310, 'A', 4);
+INSERT INTO factory_buildings VALUES (1, 'A001', 150, 'A', 1, '{"ins": "2024C", "pending": true}');
+INSERT INTO factory_buildings VALUES (2, 'A002', 200, 'A', 1, '{"ins": "2025A", "pending": true}');
+INSERT INTO factory_buildings VALUES (3, 'B001', 50, 'B', 2, '{"ins": "2025A", "pending": true}');
+INSERT INTO factory_buildings VALUES (4, 'B002', 120, 'C', 2, '{"ins": "2023A"}');
+INSERT INTO factory_buildings VALUES (5, 'C001', 240, 'B', 3, '{"ins": "2022B"}' );
+INSERT INTO factory_buildings VALUES (6, 'D001', 310, 'A', 4, '{"ins": "2024C", "pending": true}');
 
 TRUNCATE TABLE surr_serial_upsert CASCADE;
 INSERT INTO surr_serial_upsert(name, extra) VALUES ('value', 'existing value');

test/spec/fixtures/schema.sql

@@ -3797,7 +3797,8 @@ create table surr_gen_default_upsert (
 
 create table operators (
   id int primary key,
-  name text
+  name text,
+  status jsonb
 );
 
 create table process_operator (
@@ -3811,5 +3812,6 @@ create table factory_buildings (
   code char(4),
   size numeric,
   "type" char(1),
-  factory_id int references factories(id)
+  factory_id int references factories(id),
+  inspections jsonb
 );