fix: a nested spread embedding now correctly groups by the fields of …

…its top parent relationship
PostgREST · Aug 21, 2024 · e9244f1 · e9244f1
1 parent 3539aaf
commit e9244f1
Show file tree

Hide file tree

Showing 5 changed files with 158 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 ### Fixed
 
  - #3693, Prevent spread embedding to allow aggregates when they are disabled - @laurenceisla
+ - #3693, A nested spread embedding now correctly groups by the fields of its top parent relationship - @laurenceisla
 
 ### Changed
 

diff --git a/src/PostgREST/Plan.hs b/src/PostgREST/Plan.hs
@@ -679,15 +679,23 @@ applySpreadAggHoistingToNode (Node rp@ReadPlan{relAggAlias, relToParent, relIsSp
                                 then (select rp, [])
                                 else hoistFromSelectFields relAggAlias (select rp)
 
-      newRelSelects = if null children
+      -- If the current `ReadPlan` is a spread rel and it has aggregates hoisted from
+      -- child relationships, then it must hoist those aggregates to its parent rel.
+      -- So we update them with the current `relAggAlias`.
+      hoistAgg ((_, fieldName), hoistFunc) = ((relAggAlias, fieldName), hoistFunc)
+      hoistedAggList = if relIsSpread
+                       then aggList ++ map hoistAgg allChildAggLists
+                       else aggList
+
+      newRelSelects = if null children || relIsSpread
                       then relSelect rp
                       else map (hoistIntoRelSelectFields allChildAggLists) $ relSelect rp
-  in  (Node rp { select = newSelects, relSelect = newRelSelects } newChildren, aggList)
+  in  (Node rp { select = newSelects, relSelect = newRelSelects } newChildren, hoistedAggList)
 
 -- Hoist aggregate functions from the select list of a ReadPlan, and return the
 -- updated select list and the list of hoisted aggregates.
 hoistFromSelectFields :: Alias -> [CoercibleSelectField] -> ([CoercibleSelectField], [HoistedAgg])
-hoistFromSelectFields aggAlias fields =
+hoistFromSelectFields relAggAlias fields =
     let (newFields, maybeAggs) = foldr processField ([], []) fields
     in (newFields, catMaybes maybeAggs)
   where
@@ -699,19 +707,19 @@ hoistFromSelectFields aggAlias fields =
       case csAggFunction field of
         Just aggFunc ->
           ( field { csAggFunction = Nothing, csAggCast = Nothing },
-            Just ((aggAlias, determineFieldName field), (aggFunc, csAggCast field, csAlias field)))
+            Just ((relAggAlias, determineFieldName field), (aggFunc, csAggCast field, csAlias field)))
         Nothing -> (field, Nothing)
 
     determineFieldName field = fromMaybe (cfName $ csField field) (csAlias field)
 
 -- Taking the hoisted aggregates, modify the rel selects to apply the aggregates,
 -- and any applicable casts or aliases.
 hoistIntoRelSelectFields :: [HoistedAgg] -> RelSelectField -> RelSelectField
-hoistIntoRelSelectFields aggList r@(Spread {rsSpreadSel = spreadSelects, rsAggAlias = aggAlias}) =
+hoistIntoRelSelectFields aggList r@(Spread {rsSpreadSel = spreadSelects, rsAggAlias = relAggAlias}) =
     r { rsSpreadSel = map updateSelect spreadSelects }
   where
     updateSelect s =
-        case lookup (aggAlias, ssSelName s) aggList of
+        case lookup (relAggAlias, ssSelName s) aggList of
             Just (aggFunc, aggCast, fldAlias) ->
                 s { ssSelAggFunction = Just aggFunc,
                     ssSelAggCast     = aggCast,

diff --git a/test/spec/Feature/Query/AggregateFunctionsSpec.hs b/test/spec/Feature/Query/AggregateFunctionsSpec.hs
@@ -152,6 +152,77 @@ allowed =
         get "/budget_expenses?select=...budget_categories(total_budget:budget_amount.sum())" `shouldRespondWith`
           [json|[{"total_budget": 9501.06}]|]
           { matchHeaders = [matchContentTypeJson] }
+      it "supports aggregates from a spread relationships grouped by spreaded fields from other relationships" $ do
+        get "/processes?select=...process_costs(cost.sum()),...process_categories(name)" `shouldRespondWith`
+          [json|[
+            {"sum": 400.00, "name": "Batch"},
+            {"sum": 320.00, "name": "Mass"}]|]
+          { matchHeaders = [matchContentTypeJson] }
+        get "/processes?select=...process_costs(cost_sum:cost.sum()),...process_categories(category:name)" `shouldRespondWith`
+          [json|[
+            {"cost_sum": 400.00, "category": "Batch"},
+            {"cost_sum": 320.00, "category": "Mass"}]|]
+          { matchHeaders = [matchContentTypeJson] }
+      it "supports aggregates on spreaded fields from nested relationships" $ do
+        get "/process_supervisor?select=...processes(factory_id,...process_costs(cost.sum()))" `shouldRespondWith`
+          [json|[
+            {"factory_id": 3, "sum": 120.00},
+            {"factory_id": 2, "sum": 500.00},
+            {"factory_id": 1, "sum": 350.00}]|]
+          { matchHeaders = [matchContentTypeJson] }
+        get "/process_supervisor?select=...processes(factory_id,...process_costs(cost_sum:cost.sum()))" `shouldRespondWith`
+          [json|[
+            {"factory_id": 3, "cost_sum": 120.00},
+            {"factory_id": 2, "cost_sum": 500.00},
+            {"factory_id": 1, "cost_sum": 350.00}]|]
+          { matchHeaders = [matchContentTypeJson] }
+      it "supports aggregates on spreaded fields from nested relationships, grouped by a regular nested relationship" $ do
+        get "/process_supervisor?select=...processes(factories(name),...process_costs(cost.sum()))" `shouldRespondWith`
+          [json|[
+            {"factories": {"name": "Factory A"}, "sum": 350.00},
+            {"factories": {"name": "Factory B"}, "sum": 500.00},
+            {"factories": {"name": "Factory C"}, "sum": 120.00}]|]
+          { matchHeaders = [matchContentTypeJson] }
+        get "/process_supervisor?select=...processes(factory:factories(name),...process_costs(cost_sum:cost.sum()))" `shouldRespondWith`
+          [json|[
+            {"factory": {"name": "Factory A"}, "cost_sum": 350.00},
+            {"factory": {"name": "Factory B"}, "cost_sum": 500.00},
+            {"factory": {"name": "Factory C"}, "cost_sum": 120.00}]|]
+          { matchHeaders = [matchContentTypeJson] }
+      it "supports aggregates on spreaded fields from nested relationships, grouped by spreaded fields from other nested relationships" $ do
+        get "/process_supervisor?select=supervisor_id,...processes(...process_costs(cost.sum()),...process_categories(name))&order=supervisor_id" `shouldRespondWith`
+          [json|[
+            {"supervisor_id": 1, "sum": 220.00, "name": "Batch"},
+            {"supervisor_id": 2, "sum": 70.00, "name": "Batch"},
+            {"supervisor_id": 2, "sum": 200.00, "name": "Mass"},
+            {"supervisor_id": 3, "sum": 180.00, "name": "Batch"},
+            {"supervisor_id": 3, "sum": 120.00, "name": "Mass"},
+            {"supervisor_id": 4, "sum": 180.00, "name": "Batch"}]|]
+          { matchHeaders = [matchContentTypeJson] }
+        get "/process_supervisor?select=supervisor_id,...processes(...process_costs(cost_sum:cost.sum()),...process_categories(category:name))&order=supervisor_id" `shouldRespondWith`
+          [json|[
+            {"supervisor_id": 1, "cost_sum": 220.00, "category": "Batch"},
+            {"supervisor_id": 2, "cost_sum": 70.00, "category": "Batch"},
+            {"supervisor_id": 2, "cost_sum": 200.00, "category": "Mass"},
+            {"supervisor_id": 3, "cost_sum": 180.00, "category": "Batch"},
+            {"supervisor_id": 3, "cost_sum": 120.00, "category": "Mass"},
+            {"supervisor_id": 4, "cost_sum": 180.00, "category": "Batch"}]|]
+          { matchHeaders = [matchContentTypeJson] }
+      it "supports aggregates on spreaded fields from nested relationships, grouped by spreaded fields from other nested relationships, using a nested relationship as top parent" $ do
+        get "/supervisors?select=name,process_supervisor(...processes(...process_costs(cost.sum()),...process_categories(name)))" `shouldRespondWith`
+          [json|[
+            {"name": "Mary", "process_supervisor": [{"name": "Batch", "sum": 220.00}]},
+            {"name": "John", "process_supervisor": [{"name": "Batch", "sum": 70.00}, {"name": "Mass", "sum": 200.00}]},
+            {"name": "Peter", "process_supervisor": [{"name": "Batch", "sum": 180.00}, {"name": "Mass", "sum": 120.00}]},
+            {"name": "Sarah", "process_supervisor": [{"name": "Batch", "sum": 180.00}]}]|]
+          { matchHeaders = [matchContentTypeJson] }
+        get "/supervisors?select=name,process_supervisor(...processes(...process_costs(cost_sum:cost.sum()),...process_categories(category:name)))" `shouldRespondWith`
+          [json|[
+            {"name": "Mary", "process_supervisor": [{"category": "Batch", "cost_sum": 220.00}]},
+            {"name": "John", "process_supervisor": [{"category": "Batch", "cost_sum": 70.00}, {"category": "Mass", "cost_sum": 200.00}]},
+            {"name": "Peter", "process_supervisor": [{"category": "Batch", "cost_sum": 180.00}, {"category": "Mass", "cost_sum": 120.00}]},
+            {"name": "Sarah", "process_supervisor": [{"category": "Batch", "cost_sum": 180.00}]}]|]
+          { matchHeaders = [matchContentTypeJson] }
 
 disallowed :: SpecWith ((), Application)
 disallowed =

diff --git a/test/spec/fixtures/data.sql b/test/spec/fixtures/data.sql
@@ -883,3 +883,42 @@ INSERT INTO budget_expenses VALUES (1, 200.26, 1);
 INSERT INTO budget_expenses VALUES (2, 400.26, 3);
 INSERT INTO budget_expenses VALUES (3, 100.22, 4);
 INSERT INTO budget_expenses VALUES (5, 900.27, 5);
+
+TRUNCATE TABLE factories CASCADE;
+INSERT INTO factories VALUES (1, 'Factory A');
+INSERT INTO factories VALUES (2, 'Factory B');
+INSERT INTO factories VALUES (3, 'Factory C');
+INSERT INTO factories VALUES (4, 'Factory D');
+
+TRUNCATE TABLE process_categories CASCADE;
+INSERT INTO process_categories VALUES (1, 'Batch');
+INSERT INTO process_categories VALUES (2, 'Mass');
+
+TRUNCATE TABLE processes CASCADE;
+INSERT INTO processes VALUES (1, 'Process A1', 1, 1);
+INSERT INTO processes VALUES (2, 'Process A2', 1, 2);
+INSERT INTO processes VALUES (3, 'Process B1', 2, 1);
+INSERT INTO processes VALUES (4, 'Process B2', 2, 1);
+INSERT INTO processes VALUES (5, 'Process C1', 3, 2);
+
+TRUNCATE TABLE process_costs CASCADE;
+INSERT INTO process_costs VALUES (1, 150.00);
+INSERT INTO process_costs VALUES (2, 200.00);
+INSERT INTO process_costs VALUES (3, 180.00);
+INSERT INTO process_costs VALUES (4, 70.00);
+INSERT INTO process_costs VALUES (5, 120.00);
+
+TRUNCATE TABLE supervisors CASCADE;
+INSERT INTO supervisors VALUES (1, 'Mary');
+INSERT INTO supervisors VALUES (2, 'John');
+INSERT INTO supervisors VALUES (3, 'Peter');
+INSERT INTO supervisors VALUES (4, 'Sarah');
+
+TRUNCATE TABLE process_supervisor CASCADE;
+INSERT INTO process_supervisor VALUES (1, 1);
+INSERT INTO process_supervisor VALUES (2, 2);
+INSERT INTO process_supervisor VALUES (3, 3);
+INSERT INTO process_supervisor VALUES (3, 4);
+INSERT INTO process_supervisor VALUES (4, 1);
+INSERT INTO process_supervisor VALUES (4, 2);
+INSERT INTO process_supervisor VALUES (5, 3);
diff --git a/test/spec/fixtures/schema.sql b/test/spec/fixtures/schema.sql
@@ -3749,3 +3749,36 @@ after insert
 on infinite_inserts
 for each row
 execute procedure infinite_inserts();
+
+create table factories (
+  id int primary key,
+  name text
+);
+
+create table process_categories (
+  id int primary key,
+  name text
+);
+
+create table processes (
+  id int primary key,
+  name text,
+  factory_id int references factories(id),
+  category_id int references process_categories(id)
+);
+
+create table process_costs (
+  process_id int references processes(id) primary key,
+  cost numeric
+);
+
+create table supervisors (
+  id int primary key,
+  name text
+);
+
+create table process_supervisor (
+  process_id int references processes(id),
+  supervisor_id int references supervisors(id),
+  primary key (process_id, supervisor_id)
+);