diff --git a/src/PostgREST/Plan.hs b/src/PostgREST/Plan.hs index f070b1e333..364df2de06 100644 --- a/src/PostgREST/Plan.hs +++ b/src/PostgREST/Plan.hs @@ -380,7 +380,7 @@ initReadRequest ctx@ResolverContext{qi=QualifiedIdentifier{..}} = foldr (treeEntry rootDepth) $ Node defReadPlan{from=qi ctx, relName=qiName, depth=rootDepth} [] where rootDepth = 0 - defReadPlan = ReadPlan [] (QualifiedIdentifier mempty mempty) Nothing [] [] allRange mempty Nothing [] Nothing mempty Nothing Nothing Nothing [] rootDepth + defReadPlan = ReadPlan [] (QualifiedIdentifier mempty mempty) Nothing [] [] allRange mempty Nothing [] Nothing mempty Nothing Nothing Nothing [] [] rootDepth treeEntry :: Depth -> Tree SelectItem -> ReadPlanTree -> ReadPlanTree treeEntry depth (Node si fldForest) (Node q rForest) = let nxtDepth = succ depth in @@ -848,21 +848,46 @@ resolveOrder ctx (OrderTerm fld dir nulls) = CoercibleOrderTerm (resolveTypeOrUn -- and if it's a to-one relationship, it adds the right alias to the OrderRelationTerm so the generated query can succeed. addRelatedOrders :: ReadPlanTree -> Either Error ReadPlanTree addRelatedOrders (Node rp@ReadPlan{order,from} forest) = do - newOrder <- newRelOrder `traverse` order - Node rp{order=newOrder} <$> addRelatedOrders `traverse` forest + (newOrder, newForest) <- foldM addRelOrder ([], forest) (zip [1..] order) + Node rp{order=reverse newOrder} <$> addRelatedOrders `traverse` newForest where - newRelOrder cot@CoercibleOrderTerm{} = Right cot - newRelOrder cot@CoercibleOrderRelationTerm{coRelation} = - let foundRP = rootLabel <$> find (\(Node ReadPlan{relName, relAlias} _) -> coRelation == fromMaybe relName relAlias) forest in - case foundRP of - Just ReadPlan{relName,relAlias,relAggAlias,relToParent} -> - let isToOne = relIsToOne <$> relToParent - name = fromMaybe relName relAlias in - if isToOne == Just True - then Right $ cot{coRelation=relAggAlias} - else Left $ ApiRequestError $ RelatedOrderNotToOne (qiName from) name + addRelOrder (ords, curForest) (_idx, cot@CoercibleOrderTerm{}) = + Right (cot : ords, curForest) + addRelOrder (ords, curForest) (idx, cot@CoercibleOrderRelationTerm{coRelation, coRelTerm, coDirection, coNullOrder}) = + case findTarget coRelation curForest of Nothing -> Left $ ApiRequestError $ NotEmbedded coRelation + Just (Node ReadPlan{relName, relAlias, relAggAlias, relToParent} _) -> + let isToOne = relIsToOne <$> relToParent in + case isToOne of + Just True -> + Right (cot{coRelation=relAggAlias} : ords, curForest) + Just False -> + let + ordAlias = toManyOrderAlias relAggAlias idx + relOrder = RelOrderAgg ordAlias (CoercibleOrderTerm (unknownField (fst coRelTerm) (snd coRelTerm)) coDirection coNullOrder) + newForest = updateTarget coRelation (addRelOrderAgg relOrder) curForest + newOrder = cot{coRelation=relAggAlias, coRelTerm=(ordAlias, [])} + in + Right (newOrder : ords, newForest) + Nothing -> + Left $ ApiRequestError $ RelatedOrderNotToOne (qiName from) (fromMaybe relName relAlias) + + addRelOrderAgg ro rpChild = + rpChild{relOrderAgg = relOrderAgg rpChild <> [ro]} + + findTarget rel = + find (\(Node ReadPlan{relName, relAlias} _) -> rel == fromMaybe relName relAlias) + + updateTarget rel f = + map (\node@(Node rpChild forestChild) -> + if rel == fromMaybe (relName rpChild) (relAlias rpChild) + then Node (f rpChild) forestChild + else node) + + toManyOrderAlias :: Alias -> Integer -> FieldName + toManyOrderAlias relAggAlias idx = + relAggAlias <> "_ord_" <> show idx -- | Searches for null filters on embeds, e.g. `projects=not.is.null` on `GET /clients?select=*,projects(*)&projects=not.is.null` -- diff --git a/src/PostgREST/Plan/ReadPlan.hs b/src/PostgREST/Plan/ReadPlan.hs index 22bd7d62ef..c5956db993 100644 --- a/src/PostgREST/Plan/ReadPlan.hs +++ b/src/PostgREST/Plan/ReadPlan.hs @@ -2,6 +2,7 @@ module PostgREST.Plan.ReadPlan ( ReadPlanTree , ReadPlan(..) , JoinCondition(..) + , RelOrderAgg(..) , SpreadType(..) ) where @@ -30,6 +31,12 @@ data JoinCondition = (QualifiedIdentifier, FieldName) deriving (Eq, Show) +data RelOrderAgg = RelOrderAgg + { roaAlias :: FieldName + , roaOrderTerm :: CoercibleOrderTerm + } + deriving (Eq, Show) + -- TODO: Enforce uniqueness of columns by changing to a Set instead of a List where applicable data ReadPlan = ReadPlan { select :: [CoercibleSelectField] @@ -46,6 +53,7 @@ data ReadPlan = ReadPlan , relHint :: Maybe Hint , relJoinType :: Maybe JoinType , relSpread :: Maybe SpreadType + , relOrderAgg :: [RelOrderAgg] , relSelect :: [RelSelectField] , depth :: Depth -- ^ used for aliasing diff --git a/src/PostgREST/Query/QueryBuilder.hs b/src/PostgREST/Query/QueryBuilder.hs index f84a5e2d7d..c691ce00c6 100644 --- a/src/PostgREST/Query/QueryBuilder.hs +++ b/src/PostgREST/Query/QueryBuilder.hs @@ -95,15 +95,29 @@ getJoins (Node ReadPlan{relSelect} forest) = ) relSelect getJoin :: RelSelectField -> ReadPlanTree -> SQL.Snippet -getJoin fld node@(Node ReadPlan{relJoinType, relSpread} _) = +getJoin fld node@(Node ReadPlan{relJoinType, relSpread, relOrderAgg, order} _) = let correlatedSubquery sub al cond = " " <> (if relJoinType == Just JTInner then "INNER" else "LEFT") <> " JOIN LATERAL ( " <> sub <> " ) AS " <> al <> " ON " <> cond subquery = readPlanToQuery node aggAlias = pgFmtIdent $ rsAggAlias fld - selectSubqAgg = "SELECT json_agg(" <> aggAlias <> ")::jsonb AS " <> aggAlias + orderAggSelects = pgFmtRelOrderAgg (rsAggAlias fld) <$> relOrderAgg + jsonAggOrder = + if all isDirectOrderTerm order + then orderF (QualifiedIdentifier "" (rsAggAlias fld)) order + else mempty + jsonAgg = + "json_agg(" <> aggAlias <> + (if null order then mempty else " " <> jsonAggOrder) <> + ")::jsonb AS " <> aggAlias + selectSubqAgg = + "SELECT " <> jsonAgg <> + (if null orderAggSelects then mempty else ", " <> intercalateSnippet ", " orderAggSelects) fromSubqAgg = " FROM (" <> subquery <> " ) AS " <> aggAlias joinCondition = if relJoinType == Just JTInner then aggAlias <> " IS NOT NULL" else "TRUE" + isDirectOrderTerm ot = case ot of + CoercibleOrderTerm{} -> True + _ -> False in case fld of JsonEmbed{rsEmbedMode = JsonObject} -> @@ -111,13 +125,24 @@ getJoin fld node@(Node ReadPlan{relJoinType, relSpread} _) = Spread{rsSpreadSel, rsAggAlias} -> case relSpread of Just (ToManySpread _ sprOrder) -> - let selSpread = selectSubqAgg <> (if null rsSpreadSel then mempty else ", ") <> intercalateSnippet ", " (pgFmtSpreadJoinSelectItem rsAggAlias sprOrder <$> rsSpreadSel) + 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 +pgFmtRelOrderAgg :: Alias -> RelOrderAgg -> SQL.Snippet +pgFmtRelOrderAgg aggAlias RelOrderAgg{roaAlias, roaOrderTerm} = + "array_agg(" <> fmtField <> " " <> fmtOrder <> ") AS " <> pgFmtIdent roaAlias + where + fmtField = case roaOrderTerm of + CoercibleOrderTerm{coField} -> pgFmtField (QualifiedIdentifier "" aggAlias) coField + CoercibleOrderRelationTerm{} -> pgFmtIdent roaAlias + fmtOrder = orderF (QualifiedIdentifier "" aggAlias) [roaOrderTerm] + mutatePlanToQuery :: MutatePlan -> SQL.Snippet mutatePlanToQuery (Insert mainQi iCols body onConflict putConditions returnings _ applyDefaults) = "INSERT INTO " <> fromQi mainQi <> (if null iCols then " " else "(" <> cols <> ") ") <> diff --git a/src/PostgREST/Query/SqlFragment.hs b/src/PostgREST/Query/SqlFragment.hs index 461d4e8274..f5f3eed6e9 100644 --- a/src/PostgREST/Query/SqlFragment.hs +++ b/src/PostgREST/Query/SqlFragment.hs @@ -24,6 +24,7 @@ module PostgREST.Query.SqlFragment , noLocationF , orderF , pgFmtColumn + , pgFmtField , pgFmtFilter , pgFmtIdent , pgFmtJoinCondition diff --git a/test/spec/Feature/Query/RelatedQueriesSpec.hs b/test/spec/Feature/Query/RelatedQueriesSpec.hs index de0dd8f2d0..56ef644baa 100644 --- a/test/spec/Feature/Query/RelatedQueriesSpec.hs +++ b/test/spec/Feature/Query/RelatedQueriesSpec.hs @@ -106,35 +106,29 @@ spec = describe "related queries" $ do , matchHeaders = [matchContentTypeJson] } - it "fails when is not a to-one relationship" $ do - get "/clients?select=*,projects(*)&order=projects(id)" `shouldRespondWith` - [json|{ - "code":"PGRST118", - "details":"'clients' and 'projects' do not form a many-to-one or one-to-one relationship", - "hint":null, - "message":"A related order on 'projects' is not possible" - }|] - { matchStatus = 400 + it "works on a to-many relationship" $ do + get "/clients?select=*,projects(*)&order=projects(id)&projects.order=id.asc" `shouldRespondWith` + [json|[ + {"id":1,"name":"Microsoft","projects":[{"id":1,"name":"Windows 7","client_id":1},{"id":2,"name":"Windows 10","client_id":1}]}, + {"id":2,"name":"Apple","projects":[{"id":3,"name":"IOS","client_id":2},{"id":4,"name":"OSX","client_id":2}]} + ]|] + { matchStatus = 200 , matchHeaders = [matchContentTypeJson] } - get "/clients?select=*,pros:projects(*)&order=pros(id)" `shouldRespondWith` - [json|{ - "code":"PGRST118", - "details":"'clients' and 'pros' do not form a many-to-one or one-to-one relationship", - "hint":null, - "message":"A related order on 'pros' is not possible" - }|] - { matchStatus = 400 + get "/clients?select=*,pros:projects(*)&order=pros(id)&pros.order=id.asc" `shouldRespondWith` + [json|[ + {"id":1,"name":"Microsoft","pros":[{"id":1,"name":"Windows 7","client_id":1},{"id":2,"name":"Windows 10","client_id":1}]}, + {"id":2,"name":"Apple","pros":[{"id":3,"name":"IOS","client_id":2},{"id":4,"name":"OSX","client_id":2}]} + ]|] + { matchStatus = 200 , matchHeaders = [matchContentTypeJson] } - get "/designers?select=id,computed_videogames(id)&order=computed_videogames(id).desc" `shouldRespondWith` - [json|{ - "code":"PGRST118", - "details":"'designers' and 'computed_videogames' do not form a many-to-one or one-to-one relationship", - "hint":null, - "message":"A related order on 'computed_videogames' is not possible" - }|] - { matchStatus = 400 + get "/designers?select=id,computed_videogames(id)&order=computed_videogames(id).desc&computed_videogames.order=id.asc" `shouldRespondWith` + [json|[ + {"id":2,"computed_videogames":[{"id":3},{"id":4}]}, + {"id":1,"computed_videogames":[{"id":1},{"id":2}]} + ]|] + { matchStatus = 200 , matchHeaders = [matchContentTypeJson] }