Skip to content

Commit

Permalink
feat: WIP allow spread operators in to-many relationships
Browse files Browse the repository at this point in the history
  • Loading branch information
laurenceisla committed Sep 13, 2024
1 parent 678103b commit cfec0d8
Show file tree
Hide file tree
Showing 10 changed files with 227 additions and 53 deletions.
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 | JsonAgg
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
69 changes: 44 additions & 25 deletions src/PostgREST/Plan.hs
Original file line number Diff line number Diff line change
Expand Up @@ -334,12 +334,12 @@ readPlan qi@QualifiedIdentifier{..} AppConfig{configDbMaxRows, configDbAggregate
treeRestrictRange configDbMaxRows (iAction apiRequest) =<<
hoistSpreadAggFunctions =<<
validateAggFunctions configDbAggregates =<<
addRelSelects =<<
addRelSelects False =<<
addNullEmbedFilters =<<
validateSpreadEmbeds =<<
addRelatedOrders =<<
addAliases =<<
expandStars ctx =<<
addJsonAggToManySpread False =<<
addRels qiSchema (iAction apiRequest) dbRelationships Nothing =<<
addLogicTrees ctx apiRequest =<<
addRanges apiRequest =<<
Expand Down Expand Up @@ -616,32 +616,52 @@ findRel schema allRels origin target hint =
)
) $ fromMaybe mempty $ HM.lookup (QualifiedIdentifier schema origin, schema) allRels


addRelSelects :: ReadPlanTree -> Either ApiRequestError ReadPlanTree
addRelSelects node@(Node rp forest)
-- Add JsonAgg 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
addJsonAggToManySpread :: Bool -> ReadPlanTree -> Either ApiRequestError ReadPlanTree
addJsonAggToManySpread isNested (Node rp@ReadPlan{select} forest) =
let newForest = addJsonAggToManySpread shouldAddJsonAgg `traverse` forest
newSelects
| shouldAddJsonAgg = fieldToJsonAgg <$> select
| otherwise = select
in Node rp { select = newSelects } <$> newForest
where
shouldAddJsonAgg = spreadRelIsInToMany isNested rp
fieldToJsonAgg field
| isJust $ csAggFunction field = field
| otherwise = field { csAggFunction = Just JsonAgg, csAlias = newAlias (csAlias field) (cfName $ csField field) }
newAlias :: Maybe Alias -> FieldName -> Maybe Alias
newAlias _ "*" = Nothing
newAlias alias fieldName = maybe (Just fieldName) pure alias

addRelSelects :: Bool -> ReadPlanTree -> Either ApiRequestError ReadPlanTree
addRelSelects isNestedInToManySpread node@(Node rp forest)
| null forest = Right node
| otherwise =
let newForest = rights $ addRelSelects <$> forest
newRelSelects = mapMaybe generateRelSelectField newForest
let spreadInToMany = spreadRelIsInToMany isNestedInToManySpread rp
newForest = rights $ addRelSelects spreadInToMany <$> forest
newRelSelects = mapMaybe (generateRelSelectField spreadInToMany) newForest
in Right $ Node rp { relSelect = newRelSelects } newForest

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 :: Bool -> ReadPlanTree -> Maybe RelSelectField
generateRelSelectField isNestedInToManySpread (Node rp@ReadPlan{relToParent=Just _, relAggAlias, relIsSpread = True} _) =
Just $ Spread { rsSpreadSel = generateSpreadSelectFields isNestedInToManySpread rp, rsAggAlias = relAggAlias }
generateRelSelectField isNestedInToManySpread (Node ReadPlan{relToParent=Just rel, select, relName, relAlias, relAggAlias, relIsSpread = False} 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 || isNestedInToManySpread then JsonObject else JsonArray
rsEmptyEmbed = hasOnlyNullEmbed (null select) forest
hasOnlyNullEmbed = foldr checkIfNullEmbed
checkIfNullEmbed :: ReadPlanTree -> Bool -> Bool
checkIfNullEmbed (Node ReadPlan{select=s} f) isNullEmbed =
isNullEmbed && hasOnlyNullEmbed (null s) f
generateRelSelectField _ = Nothing
generateRelSelectField _ _ = Nothing

Check warning on line 661 in src/PostgREST/Plan.hs

View check run for this annotation

Codecov / codecov/patch

src/PostgREST/Plan.hs#L661

Added line #L661 was not covered by tests

generateSpreadSelectFields :: ReadPlan -> [SpreadSelectField]
generateSpreadSelectFields ReadPlan{select, relSelect} =
generateSpreadSelectFields :: Bool -> ReadPlan -> [SpreadSelectField]
generateSpreadSelectFields isNestedInToManySpread rp@ReadPlan{select, relSelect} =
-- We combine the select and relSelect fields into a single list of SpreadSelectField.
selectSpread ++ relSelectSpread
where
Expand All @@ -653,10 +673,18 @@ 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 usin `json_agg()`
let shouldAddJsonAgg = spreadRelIsInToMany isNestedInToManySpread rp
(aggFun, alias) = if shouldAddJsonAgg then (Just JsonAgg, Just rsSelName) else (Nothing, Nothing) in
[SpreadSelectField { ssSelName = rsSelName, ssSelAggFunction = aggFun, ssSelAggCast = Nothing, ssSelAlias = alias }]
relSelectToSpread (Spread{rsSpreadSel}) =
rsSpreadSel

-- TODO: perhaps this can be done in addRels instead? ReadPlan could have a "nestedInToManySpread" attribute
spreadRelIsInToMany :: Bool -> ReadPlan -> Bool
spreadRelIsInToMany isNested ReadPlan{relIsSpread, relToParent} =
relIsSpread && (isNested || Just False == (relIsToOne <$> relToParent))

-- When aggregates are present in a ReadPlan that will be spread, we "hoist"
-- to the highest level possible so that their semantics make sense. For instance,
-- imagine the user performs the following request:
Expand Down Expand Up @@ -906,15 +934,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
1 change: 1 addition & 0 deletions src/PostgREST/Query/QueryBuilder.hs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ getJoinSelects (Node ReadPlan{relSelect} _) =
case fld of
JsonEmbed{rsEmptyEmbed = True} ->
Nothing
-- Maybe there needs to be a flag here or treat as a Spread?
JsonEmbed{rsSelName, rsEmbedMode = JsonObject} ->
Just $ "row_to_json(" <> aggAlias <> ".*)::jsonb AS " <> pgFmtIdent rsSelName
JsonEmbed{rsSelName, rsEmbedMode = JsonArray} ->
Expand Down
7 changes: 4 additions & 3 deletions src/PostgREST/Query/SqlFragment.hs
Original file line number Diff line number Diff line change
Expand Up @@ -284,10 +284,11 @@ pgFmtApplyAggregate Nothing _ snippet = snippet
pgFmtApplyAggregate (Just agg) aggCast snippet =
pgFmtApplyCast aggCast aggregatedSnippet
where
convertAggFunction :: AggregateFunction -> SQL.Snippet
aggregatedSnippet = aggFunction <> "(" <> aggArgument <> ")" <> aggFilter
-- Convert from e.g. Sum (the data type) to SUM
convertAggFunction = SQL.sql . BS.map toUpper . BS.pack . show
aggregatedSnippet = convertAggFunction agg <> "(" <> snippet <> ")"
(aggFunction, aggArgument, aggFilter) = case agg of
JsonAgg -> (SQL.sql "json_agg", snippet, mempty)
a -> (SQL.sql . BS.map toUpper . BS.pack $ show a, snippet, mempty)

pgFmtApplyCast :: Maybe Cast -> SQL.Snippet -> SQL.Snippet
pgFmtApplyCast Nothing snippet = snippet
Expand Down
6 changes: 4 additions & 2 deletions test/spec/Feature/Query/AggregateFunctionsSpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -214,14 +214,16 @@ allowed =
{"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}]}]|]
{"name": "Sarah", "process_supervisor": [{"name": "Batch", "sum": 180.00}]},
{"name": "Jane", "process_supervisor": []}]|]
{ 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}]}]|]
{"name": "Sarah", "process_supervisor": [{"category": "Batch", "cost_sum": 180.00}]},
{"name": "Jane", "process_supervisor": []}]|]
{ matchHeaders = [matchContentTypeJson] }

context "supports count() aggregate without specifying a field" $ do
Expand Down
Loading

0 comments on commit cfec0d8

Please sign in to comment.