Skip to content

Commit

Permalink
feat: allow spreading one-to-many and many-to-many embedded resources
Browse files Browse the repository at this point in the history
The selected columns in the embedded resources are aggregated into arrays
  • Loading branch information
laurenceisla committed Sep 18, 2024
1 parent a064d0d commit 9002110
Show file tree
Hide file tree
Showing 11 changed files with 760 additions and 195 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
- #3607, Log to stderr when the JWT secret is less than 32 characters long - @laurenceisla
- #2858, Performance improvements when calling RPCs via GET using indexes in more cases - @wolfgangwalther
- #3560, Log resolved host in "Listening on ..." messages - @develop7
- #3041, Allow spreading one-to-many and many-to-many embedded resources - @laurenceisla
+ The selected columns in the embedded resources are aggregated into arrays

### Fixed

Expand Down
3 changes: 1 addition & 2 deletions src/PostgREST/ApiRequest/Types.hs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ data ApiRequestError
| PutLimitNotAllowedError
| QueryParamError QPError
| RelatedOrderNotToOne Text Text
| SpreadNotToOne Text Text
| UnacceptableFilter Text
| UnacceptableSchema [Text]
| UnsupportedMethod ByteString
Expand Down Expand Up @@ -145,7 +144,7 @@ type Cast = Text
type Alias = Text
type Hint = Text

data AggregateFunction = Sum | Avg | Max | Min | Count
data AggregateFunction = Sum | Avg | Max | Min | Count | ArrayAgg
deriving (Show, Eq)

data EmbedParam
Expand Down
10 changes: 1 addition & 9 deletions src/PostgREST/Error.hs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ instance PgrstError ApiRequestError where
status PutLimitNotAllowedError = HTTP.status400
status QueryParamError{} = HTTP.status400
status RelatedOrderNotToOne{} = HTTP.status400
status SpreadNotToOne{} = HTTP.status400
status UnacceptableFilter{} = HTTP.status400
status UnacceptableSchema{} = HTTP.status406
status UnsupportedMethod{} = HTTP.status405
Expand Down Expand Up @@ -176,12 +175,6 @@ instance JSON.ToJSON ApiRequestError where
(Just $ JSON.String $ "'" <> origin <> "' and '" <> target <> "' do not form a many-to-one or one-to-one relationship")
Nothing

toJSON (SpreadNotToOne origin target) = toJsonPgrstError
ApiRequestErrorCode19
("A spread operation on '" <> target <> "' is not possible")
(Just $ JSON.String $ "'" <> origin <> "' and '" <> target <> "' do not form a many-to-one or one-to-one relationship")
Nothing

toJSON (UnacceptableFilter target) = toJsonPgrstError
ApiRequestErrorCode20
("Bad operator on the '" <> target <> "' embedded resource")
Expand Down Expand Up @@ -629,7 +622,7 @@ data ErrorCode
| ApiRequestErrorCode16
| ApiRequestErrorCode17
| ApiRequestErrorCode18
| ApiRequestErrorCode19
-- | ApiRequestErrorCode19 -- no longer used (used to be mapped to SpreadNotToOne)
| ApiRequestErrorCode20
| ApiRequestErrorCode21
| ApiRequestErrorCode22
Expand Down Expand Up @@ -678,7 +671,6 @@ buildErrorCode code = case code of
ApiRequestErrorCode16 -> "PGRST116"
ApiRequestErrorCode17 -> "PGRST117"
ApiRequestErrorCode18 -> "PGRST118"
ApiRequestErrorCode19 -> "PGRST119"
ApiRequestErrorCode20 -> "PGRST120"
ApiRequestErrorCode21 -> "PGRST121"
ApiRequestErrorCode22 -> "PGRST122"
Expand Down
59 changes: 37 additions & 22 deletions src/PostgREST/Plan.hs
Original file line number Diff line number Diff line change
Expand Up @@ -336,9 +336,9 @@ readPlan qi@QualifiedIdentifier{..} AppConfig{configDbMaxRows, configDbAggregate
validateAggFunctions configDbAggregates =<<
addRelSelects =<<
addNullEmbedFilters =<<
validateSpreadEmbeds =<<
addRelatedOrders =<<
addAliases =<<
addArrayAggToManySpread =<<
expandStars ctx =<<
addRels qiSchema (iAction apiRequest) dbRelationships Nothing =<<
addLogicTrees ctx apiRequest =<<
Expand All @@ -352,7 +352,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 False [] rootDepth
defReadPlan = ReadPlan [] (QualifiedIdentifier mempty mempty) Nothing [] [] allRange mempty Nothing [] Nothing mempty Nothing Nothing False False [] rootDepth
treeEntry :: Depth -> Tree SelectItem -> ReadPlanTree -> ReadPlanTree
treeEntry depth (Node si fldForest) (Node q rForest) =
let nxtDepth = succ depth in
Expand Down Expand Up @@ -417,13 +417,14 @@ knownColumnsInContext ResolverContext{..} =
-- | Expand "select *" into explicit field names of the table in the following situations:
-- * When there are data representations present.
-- * When there is an aggregate function in a given ReadPlan or its parent.
-- * When the ReadPlan is a spread embed nested inside a to-many spread relationship (array aggregate).
expandStars :: ResolverContext -> ReadPlanTree -> Either ApiRequestError ReadPlanTree
expandStars ctx rPlanTree = Right $ expandStarsForReadPlan False rPlanTree
where
expandStarsForReadPlan :: Bool -> ReadPlanTree -> ReadPlanTree
expandStarsForReadPlan hasAgg (Node rp@ReadPlan{select, from=fromQI, fromAlias=alias} children) =
let
newHasAgg = hasAgg || any (isJust . csAggFunction) select
newHasAgg = hasAgg || any (isJust . csAggFunction) select || spreadRelIsNestedInToMany rp
newCtx = adjustContext ctx fromQI alias
newRPlan = expandStarsForTable newCtx newHasAgg rp
in Node newRPlan (map (expandStarsForReadPlan newHasAgg) children)
Expand Down Expand Up @@ -474,18 +475,18 @@ treeRestrictRange maxRows _ request = pure $ nodeRestrictRange maxRows <$> reque
addRels :: Schema -> Action -> RelationshipsMap -> Maybe ReadPlanTree -> ReadPlanTree -> Either ApiRequestError ReadPlanTree
addRels schema action allRels parentNode (Node rPlan@ReadPlan{relName,relHint,relAlias,depth} forest) =
case parentNode of
Just (Node ReadPlan{from=parentNodeQi, fromAlias=parentAlias} _) ->
Just (Node pr@ReadPlan{from=parentNodeQi, fromAlias=parentAlias} _) ->
let
newReadPlan = (\r ->
let newAlias = Just (qiName (relForeignTable r) <> "_" <> show depth)
aggAlias = qiName (relTable r) <> "_" <> fromMaybe relName relAlias <> "_" <> show depth 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}
rPlan{from=relForeignTable r, relToParent=Just r, relAggAlias=aggAlias, relJoinConds=getJoinConditions Nothing parentAlias r, relIsInToManySpread=spreadRelIsNestedInToMany pr}
ComputedRelationship{} ->
rPlan{from=relForeignTable r, relToParent=Just r{relTableAlias=maybe (relTable r) (QualifiedIdentifier mempty) parentAlias}, relAggAlias=aggAlias, fromAlias=newAlias}
rPlan{from=relForeignTable r, relToParent=Just r{relTableAlias=maybe (relTable r) (QualifiedIdentifier mempty) parentAlias}, relAggAlias=aggAlias, fromAlias=newAlias, relIsInToManySpread=spreadRelIsNestedInToMany pr}
_ ->
rPlan{from=relForeignTable r, relToParent=Just r, relAggAlias=aggAlias, fromAlias=newAlias, relJoinConds=getJoinConditions newAlias parentAlias r}
rPlan{from=relForeignTable r, relToParent=Just r, relAggAlias=aggAlias, fromAlias=newAlias, relJoinConds=getJoinConditions newAlias parentAlias r, relIsInToManySpread=spreadRelIsNestedInToMany pr}
) <$> rel
origin = if depth == 1 -- Only on depth 1 we check if the root(depth 0) has an alias so the sourceCTEName alias can be found as a relationship
then fromMaybe (qiName parentNodeQi) parentAlias
Expand All @@ -509,6 +510,10 @@ addRels schema action allRels parentNode (Node rPlan@ReadPlan{relName,relHint,re
updateForest :: Maybe ReadPlanTree -> Either ApiRequestError [ReadPlanTree]
updateForest rq = addRels schema action allRels rq `traverse` forest

spreadRelIsNestedInToMany :: ReadPlan -> Bool
spreadRelIsNestedInToMany ReadPlan{relIsSpread, relToParent, relIsInToManySpread} =
relIsSpread && (relIsInToManySpread || Just False == (relIsToOne <$> relToParent))

getJoinConditions :: Maybe Alias -> Maybe Alias -> Relationship -> [JoinCondition]
getJoinConditions _ _ ComputedRelationship{} = []
getJoinConditions tblAlias parentAlias Relationship{relTable=qi,relForeignTable=fQi,relCardinality=card} =
Expand Down Expand Up @@ -616,6 +621,22 @@ findRel schema allRels origin target hint =
)
) $ fromMaybe mempty $ HM.lookup (QualifiedIdentifier schema origin, schema) allRels

-- Add ArrayAgg aggregates to selected fields that do not have other aggregates and:
-- * Are selected inside a to-many spread relationship
-- * Are selected inside a to-one spread relationship but are nested inside a to-many spread relationship at any level
addArrayAggToManySpread :: ReadPlanTree -> Either ApiRequestError ReadPlanTree
addArrayAggToManySpread (Node rp@ReadPlan{select} forest) =
let newForest = addArrayAggToManySpread `traverse` forest
newSelects
| shouldAddArrayAgg = fieldToArrayAgg <$> select
| otherwise = select
in Node rp { select = newSelects } <$> newForest
where
shouldAddArrayAgg = spreadRelIsNestedInToMany rp
fieldToArrayAgg field
| isJust $ csAggFunction field = field
| otherwise = field { csAggFunction = Just ArrayAgg, csAlias = newAlias (csAlias field) (cfName $ csField field) }
newAlias alias fieldName = maybe (Just fieldName) pure alias

addRelSelects :: ReadPlanTree -> Either ApiRequestError ReadPlanTree
addRelSelects node@(Node rp forest)
Expand All @@ -628,11 +649,12 @@ addRelSelects node@(Node rp forest)
generateRelSelectField :: ReadPlanTree -> Maybe RelSelectField
generateRelSelectField (Node rp@ReadPlan{relToParent=Just _, relAggAlias, relIsSpread = True} _) =
Just $ Spread { rsSpreadSel = generateSpreadSelectFields rp, rsAggAlias = relAggAlias }
generateRelSelectField (Node ReadPlan{relToParent=Just rel, select, relName, relAlias, relAggAlias, relIsSpread = False} forest) =
generateRelSelectField (Node ReadPlan{relToParent=Just rel, select, relName, relAlias, relAggAlias, relIsSpread = False, relIsInToManySpread} forest) =
Just $ JsonEmbed { rsEmbedMode, rsSelName, rsAggAlias = relAggAlias, rsEmptyEmbed }
where
rsSelName = fromMaybe relName relAlias
rsEmbedMode = if relIsToOne rel then JsonObject else JsonArray
-- If the JsonEmbed is nested in a to-many spread relationship, it will be aggregated at the top. That's why we treat it as `JsonObject`.
rsEmbedMode = if relIsToOne rel || relIsInToManySpread then JsonObject else JsonArray
rsEmptyEmbed = hasOnlyNullEmbed (null select) forest
hasOnlyNullEmbed = foldr checkIfNullEmbed
checkIfNullEmbed :: ReadPlanTree -> Bool -> Bool
Expand All @@ -641,7 +663,7 @@ generateRelSelectField (Node ReadPlan{relToParent=Just rel, select, relName, rel
generateRelSelectField _ = Nothing

generateSpreadSelectFields :: ReadPlan -> [SpreadSelectField]
generateSpreadSelectFields ReadPlan{select, relSelect} =
generateSpreadSelectFields rp@ReadPlan{select, relSelect} =
-- We combine the select and relSelect fields into a single list of SpreadSelectField.
selectSpread ++ relSelectSpread
where
Expand All @@ -653,7 +675,9 @@ generateSpreadSelectFields ReadPlan{select, relSelect} =
relSelectSpread = concatMap relSelectToSpread relSelect
relSelectToSpread :: RelSelectField -> [SpreadSelectField]
relSelectToSpread (JsonEmbed{rsSelName}) =
[SpreadSelectField { ssSelName = rsSelName, ssSelAggFunction = Nothing, ssSelAggCast = Nothing, ssSelAlias = Nothing }]
-- The regular embeds that are nested inside spread to-many relationships are also aggregated in an array
let (aggFun, alias) = if spreadRelIsNestedInToMany rp then (Just ArrayAgg, Just rsSelName) else (Nothing, Nothing) in
[SpreadSelectField { ssSelName = rsSelName, ssSelAggFunction = aggFun, ssSelAggCast = Nothing, ssSelAlias = alias }]
relSelectToSpread (Spread{rsSpreadSel}) =
rsSpreadSel

Expand Down Expand Up @@ -815,7 +839,7 @@ addRelatedOrders (Node rp@ReadPlan{order,from} forest) = do
-- relName = "projects",
-- relToParent = Nothing,
-- relJoinConds = [],
-- relAlias = Nothing, relAggAlias = "clients_projects_1", relHint = Nothing, relJoinType = Nothing, relIsSpread = False, depth = 1,
-- relAlias = Nothing, relAggAlias = "clients_projects_1", relHint = Nothing, relJoinType = Nothing, relIsSpread = False, relIsInToManySpread = False, depth = 1,
-- relSelect = []
-- },
-- subForest = []
Expand All @@ -841,7 +865,7 @@ addRelatedOrders (Node rp@ReadPlan{order,from} forest) = do
-- )
-- ],
-- order = [], range_ = fullRange, relName = "clients", relToParent = Nothing, relJoinConds = [], relAlias = Nothing, relAggAlias = "", relHint = Nothing,
-- relJoinType = Nothing, relIsSpread = False, depth = 0,
-- relJoinType = Nothing, relIsSpread = False, relIsInToManySpread = False, depth = 0,
-- relSelect = []
-- },
-- subForest = subForst
Expand Down Expand Up @@ -906,15 +930,6 @@ resolveLogicTree ctx (Expr b op lts) = CoercibleExpr b op (map (resolveLogicTree
resolveFilter :: ResolverContext -> Filter -> CoercibleFilter
resolveFilter ctx (Filter fld opExpr) = CoercibleFilter{field=resolveQueryInputField ctx fld, opExpr=opExpr}

-- Validates that spread embeds are only done on to-one relationships
validateSpreadEmbeds :: ReadPlanTree -> Either ApiRequestError ReadPlanTree
validateSpreadEmbeds (Node rp@ReadPlan{relToParent=Nothing} forest) = Node rp <$> validateSpreadEmbeds `traverse` forest
validateSpreadEmbeds (Node rp@ReadPlan{relIsSpread,relToParent=Just rel,relName} forest) = do
validRP <- if relIsSpread && not (relIsToOne rel)
then Left $ SpreadNotToOne (qiName $ relTable rel) relName -- TODO using relTable is not entirely right because ReadPlan might have an alias, need to store the parent alias on ReadPlan
else Right rp
Node validRP <$> validateSpreadEmbeds `traverse` forest

-- Find a Node of the Tree and apply a function to it
updateNode :: (a -> ReadPlanTree -> ReadPlanTree) -> (EmbedPath, a) -> Either ApiRequestError ReadPlanTree -> Either ApiRequestError ReadPlanTree
updateNode f ([], a) rr = f a <$> rr
Expand Down
33 changes: 17 additions & 16 deletions src/PostgREST/Plan/ReadPlan.hs
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,23 @@ data JoinCondition =
deriving (Eq, Show)

data ReadPlan = ReadPlan
{ select :: [CoercibleSelectField]
, from :: QualifiedIdentifier
, fromAlias :: Maybe Alias
, where_ :: [CoercibleLogicTree]
, order :: [CoercibleOrderTerm]
, range_ :: NonnegRange
, relName :: NodeName
, relToParent :: Maybe Relationship
, relJoinConds :: [JoinCondition]
, relAlias :: Maybe Alias
, relAggAlias :: Alias
, relHint :: Maybe Hint
, relJoinType :: Maybe JoinType
, relIsSpread :: Bool
, relSelect :: [RelSelectField]
, depth :: Depth
{ select :: [CoercibleSelectField]
, from :: QualifiedIdentifier
, fromAlias :: Maybe Alias
, where_ :: [CoercibleLogicTree]
, order :: [CoercibleOrderTerm]
, range_ :: NonnegRange
, relName :: NodeName
, relToParent :: Maybe Relationship
, relJoinConds :: [JoinCondition]
, relAlias :: Maybe Alias
, relAggAlias :: Alias
, relHint :: Maybe Hint
, relJoinType :: Maybe JoinType
, relIsSpread :: Bool
, relIsInToManySpread :: Bool

Check warning on line 46 in src/PostgREST/Plan/ReadPlan.hs

View check run for this annotation

Codecov / codecov/patch

src/PostgREST/Plan/ReadPlan.hs#L34-L46

Added lines #L34 - L46 were not covered by tests
, relSelect :: [RelSelectField]
, depth :: Depth
-- ^ used for aliasing
}
deriving (Eq, Show)
10 changes: 6 additions & 4 deletions src/PostgREST/Query/SqlFragment.hs
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ pgFmtCallUnary :: Text -> SQL.Snippet -> SQL.Snippet
pgFmtCallUnary f x = SQL.sql (encodeUtf8 f) <> "(" <> x <> ")"

pgFmtField :: QualifiedIdentifier -> CoercibleField -> SQL.Snippet
pgFmtField table CoercibleField{cfFullRow=True} = fromQi table
pgFmtField table CoercibleField{cfFullRow=True} = pgFmtIdent (qiName table)
pgFmtField table CoercibleField{cfName=fn, cfJsonPath=[]} = pgFmtColumn table fn
pgFmtField table CoercibleField{cfName=fn, cfToJson=doToJson, cfJsonPath=jp} | doToJson = "to_jsonb(" <> pgFmtColumn table fn <> ")" <> pgFmtJsonPath jp
| otherwise = pgFmtColumn table fn <> pgFmtJsonPath jp
Expand Down Expand Up @@ -284,10 +284,12 @@ pgFmtApplyAggregate Nothing _ snippet = snippet
pgFmtApplyAggregate (Just agg) aggCast snippet =
pgFmtApplyCast aggCast aggregatedSnippet
where
convertAggFunction :: AggregateFunction -> SQL.Snippet
-- Convert from e.g. Sum (the data type) to SUM
convertAggFunction = SQL.sql . BS.map toUpper . BS.pack . show
aggregatedSnippet = convertAggFunction agg <> "(" <> snippet <> ")"
aggregatedSnippet = case agg of
-- TODO: NULLIF(...,'{null}') does not take into consideration a case with a single element with a null value.
-- See https://github.com/PostgREST/postgrest/pull/3640#issuecomment-2334996466
ArrayAgg -> "COALESCE(NULLIF(array_agg(" <> snippet <> "),'{null}'),'{}')"
a -> convertAggFunction a <> "(" <> snippet <> ")"

pgFmtApplyCast :: Maybe Cast -> SQL.Snippet -> SQL.Snippet
pgFmtApplyCast Nothing snippet = snippet
Expand Down
Loading

0 comments on commit 9002110

Please sign in to comment.