diff --git a/CHANGELOG.md b/CHANGELOG.md index 56a3d3d8b..8167decf8 100644 --- 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 index 8f9323f46..2e3587e98 100644 --- 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,7 +707,7 @@ 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) @@ -707,11 +715,11 @@ hoistFromSelectFields aggAlias fields = -- 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 index 7c8025d58..f505ec3ad 100644 --- 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 index 3a5af3862..8d9a32459 100644 --- 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 index 67cfd7f6f..a3b6edda1 100644 --- 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) +);