From 2aa0e091bb92669360bdcedf00a9f66fb99d88f9 Mon Sep 17 00:00:00 2001 From: steve-chavez Date: Tue, 8 Nov 2022 20:03:50 -0500 Subject: [PATCH] feat: add spread embeds --- CHANGELOG.md | 4 +- postgrest.cabal | 1 + src/PostgREST/ApiRequest/QueryParams.hs | 76 +++++++++++++---- src/PostgREST/ApiRequest/Types.hs | 10 ++- src/PostgREST/Error.hs | 9 ++ src/PostgREST/Plan.hs | 32 ++++++-- src/PostgREST/Plan/ReadPlan.hs | 1 + src/PostgREST/Query/QueryBuilder.hs | 10 ++- test/spec/Feature/Query/SpreadQueriesSpec.hs | 86 ++++++++++++++++++++ test/spec/Main.hs | 2 + 10 files changed, 200 insertions(+), 31 deletions(-) create mode 100644 test/spec/Feature/Query/SpreadQueriesSpec.hs diff --git a/CHANGELOG.md b/CHANGELOG.md index cf30d6b67..357fcc9ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,9 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### Added - #1414, Add related orders - @steve-chavez - + On a many-to-one or one-to-one relationship, you can order a parent by a child column `/projects?select=*,clients(*)&order=clients(name).desc.nullsfirst` + + On a many-to-one or one-to-one relationship, you can order a parent by a child column `/projects?select=*,clients(*)&order=clients(name).desc.nullsfirst` + - #1233, Allow spreading embedded resources - @steve-chavez + + On a many-to-one or one-to-one relationship, you can unnest a json object with `/projects?select=*,..clients(client_name:name)` ### Fixed diff --git a/postgrest.cabal b/postgrest.cabal index 06c99a7d0..7a884f097 100644 --- a/postgrest.cabal +++ b/postgrest.cabal @@ -212,6 +212,7 @@ test-suite spec Feature.Query.RelatedQueriesSpec Feature.Query.RpcSpec Feature.Query.SingularSpec + Feature.Query.SpreadQueriesSpec Feature.Query.UnicodeSpec Feature.Query.UpdateSpec Feature.Query.UpsertSpec diff --git a/src/PostgREST/ApiRequest/QueryParams.hs b/src/PostgREST/ApiRequest/QueryParams.hs index 54cdcb2b3..07513c57c 100644 --- a/src/PostgREST/ApiRequest/QueryParams.hs +++ b/src/PostgREST/ApiRequest/QueryParams.hs @@ -46,7 +46,8 @@ import PostgREST.SchemaCache.Identifiers (FieldName) import PostgREST.ApiRequest.Types (EmbedParam (..), EmbedPath, Field, Filter (..), FtsOperator (..), - JoinType (..), JsonOperand (..), + Hint, JoinType (..), + JsonOperand (..), JsonOperation (..), JsonPath, ListVal, LogicOperator (..), LogicTree (..), OpExpr (..), @@ -317,6 +318,9 @@ pTreePath = do -- >>> P.parse pFieldForest "" "*,client(*,nested(*))" -- Right [Node {rootLabel = SelectField {selField = ("*",[]), selCast = Nothing, selAlias = Nothing}, subForest = []},Node {rootLabel = SelectRelation {selRelation = "client", selAlias = Nothing, selHint = Nothing, selJoinType = Nothing}, subForest = [Node {rootLabel = SelectField {selField = ("*",[]), selCast = Nothing, selAlias = Nothing}, subForest = []},Node {rootLabel = SelectRelation {selRelation = "nested", selAlias = Nothing, selHint = Nothing, selJoinType = Nothing}, subForest = [Node {rootLabel = SelectField {selField = ("*",[]), selCast = Nothing, selAlias = Nothing}, subForest = []}]}]}] -- +-- >>> P.parse pFieldForest "" "*,..client(*),other(*)" +-- Right [Node {rootLabel = SelectField {selField = ("*",[]), selCast = Nothing, selAlias = Nothing}, subForest = []},Node {rootLabel = SpreadRelation {selRelation = "client", selHint = Nothing, selJoinType = Nothing}, subForest = [Node {rootLabel = SelectField {selField = ("*",[]), selCast = Nothing, selAlias = Nothing}, subForest = []}]},Node {rootLabel = SelectRelation {selRelation = "other", selAlias = Nothing, selHint = Nothing, selJoinType = Nothing}, subForest = [Node {rootLabel = SelectField {selField = ("*",[]), selCast = Nothing, selAlias = Nothing}, subForest = []}]}] +-- -- >>> P.parse pFieldForest "" "id,clients(name[])" -- Left (line 1, column 16): -- unexpected '[' @@ -325,7 +329,8 @@ pFieldForest :: Parser [Tree SelectItem] pFieldForest = pFieldTree `sepBy1` lexeme (char ',') where pFieldTree :: Parser (Tree SelectItem) - pFieldTree = try (Node <$> pRelationSelect <*> between (char '(') (char ')') pFieldForest) <|> + pFieldTree = try (Node <$> pSpreadRelationSelect <*> between (char '(') (char ')') pFieldForest) <|> + try (Node <$> pRelationSelect <*> between (char '(') (char ')') pFieldForest) <|> Node <$> pFieldSelect <*> pure [] pStar :: Parser Text @@ -454,24 +459,10 @@ pRelationSelect :: Parser SelectItem pRelationSelect = lexeme $ try ( do alias <- optionMaybe ( try(pFieldName <* aliasSeparator) ) name <- pFieldName - prm1 <- optionMaybe pEmbedParam - prm2 <- optionMaybe pEmbedParam + (hint, jType) <- pEmbedParams try (void $ lookAhead (string "(")) - return $ SelectRelation name alias (embedParamHint prm1 <|> embedParamHint prm2) (embedParamJoin prm1 <|> embedParamJoin prm2) + return $ SelectRelation name alias hint jType ) - where - pEmbedParam :: Parser EmbedParam - pEmbedParam = - char '!' *> ( - try (string "left" $> EPJoinType JTLeft) <|> - try (string "inner" $> EPJoinType JTInner) <|> - try (EPHint <$> pFieldName)) - embedParamHint prm = case prm of - Just (EPHint hint) -> Just hint - _ -> Nothing - embedParamJoin prm = case prm of - Just (EPJoinType jt) -> Just jt - _ -> Nothing -- | -- Parse regular fields in select @@ -527,6 +518,55 @@ pFieldSelect = lexeme $ try (void $ lookAhead (string ",")) <|> try eof +-- | +-- Parse spread relations in select +-- +-- >>> P.parse pSpreadRelationSelect "" "..rel(*)" +-- Right (SpreadRelation {selRelation = "rel", selHint = Nothing, selJoinType = Nothing}) +-- +-- >>> P.parse pSpreadRelationSelect "" "..rel!hint!inner(*)" +-- Right (SpreadRelation {selRelation = "rel", selHint = Just "hint", selJoinType = Just JTInner}) +-- +-- >>> P.parse pSpreadRelationSelect "" "rel(*)" +-- Left (line 1, column 1): +-- unexpected "r" +-- expecting "..." +-- +-- >>> P.parse pSpreadRelationSelect "" "alias:..rel(*)" +-- Left (line 1, column 1): +-- unexpected "a" +-- expecting ".." +-- +-- >>> P.parse pSpreadRelationSelect "" "..rel->jsonpath(*)" +-- Left (line 1, column 8): +-- unexpected '>' +pSpreadRelationSelect :: Parser SelectItem +pSpreadRelationSelect = lexeme $ try ( do + name <- string ".." >> pFieldName + (hint, jType) <- pEmbedParams + try (void $ lookAhead (string "(")) + return $ SpreadRelation name hint jType + ) + +pEmbedParams :: Parser (Maybe Hint, Maybe JoinType) +pEmbedParams = do + prm1 <- optionMaybe pEmbedParam + prm2 <- optionMaybe pEmbedParam + return (embedParamHint prm1 <|> embedParamHint prm2, embedParamJoin prm1 <|> embedParamJoin prm2) + where + pEmbedParam :: Parser EmbedParam + pEmbedParam = + char '!' *> ( + try (string "left" $> EPJoinType JTLeft) <|> + try (string "inner" $> EPJoinType JTInner) <|> + try (EPHint <$> pFieldName)) + embedParamHint prm = case prm of + Just (EPHint hint) -> Just hint + _ -> Nothing + embedParamJoin prm = case prm of + Just (EPJoinType jt) -> Just jt + _ -> Nothing + -- | -- Parse operator expression used in horizontal filtering -- diff --git a/src/PostgREST/ApiRequest/Types.hs b/src/PostgREST/ApiRequest/Types.hs index cc1a17ac5..cbc1231ca 100644 --- a/src/PostgREST/ApiRequest/Types.hs +++ b/src/PostgREST/ApiRequest/Types.hs @@ -38,19 +38,26 @@ import PostgREST.SchemaCache.Relationship (Relationship) import Protolude --- | The select value in `/tbl?select=alias:field::cast` +-- | The value in `/tbl?select=alias:field::cast` data SelectItem = SelectField { selField :: Field , selCast :: Maybe Cast , selAlias :: Maybe Alias } +-- | The value in `/tbl?select=alias:another_tbl(*)` | SelectRelation { selRelation :: FieldName , selAlias :: Maybe Alias , selHint :: Maybe Hint , selJoinType :: Maybe JoinType } +-- | The value in `/tbl?select=...another_tbl(*)` + | SpreadRelation + { selRelation :: FieldName + , selHint :: Maybe Hint + , selJoinType :: Maybe JoinType + } deriving (Eq) data ApiRequestError @@ -71,6 +78,7 @@ data ApiRequestError | ParseRequestError Text Text | PutRangeNotAllowedError | QueryParamError QPError + | SpreadNotToOne Text Text | UnacceptableSchema [Text] | UnsupportedMethod ByteString diff --git a/src/PostgREST/Error.hs b/src/PostgREST/Error.hs index d7aaeb00e..caad95b38 100644 --- a/src/PostgREST/Error.hs +++ b/src/PostgREST/Error.hs @@ -72,6 +72,7 @@ instance PgrstError ApiRequestError where status ParseRequestError{} = HTTP.status400 status PutRangeNotAllowedError = HTTP.status400 status QueryParamError{} = HTTP.status400 + status SpreadNotToOne{} = HTTP.status400 status UnacceptableSchema{} = HTTP.status406 status UnsupportedMethod{} = HTTP.status405 status LimitNoOrderError = HTTP.status400 @@ -159,6 +160,12 @@ instance JSON.ToJSON ApiRequestError where "details" .= JSON.Null, "hint" .= JSON.Null] + toJSON (SpreadNotToOne origin target) = JSON.object [ + "code" .= ApiRequestErrorCode19, + "message" .= ("A spread operation on '" <> target <> "' is not possible" :: Text), + "details" .= ("'" <> origin <> "' and '" <> target <> "' do not form a many-to-one or one-to-one relationship" :: Text), + "hint" .= JSON.Null] + toJSON (NoRelBetween parent child schema) = JSON.object [ "code" .= SchemaCacheErrorCode00, "message" .= ("Could not find a relationship between '" <> parent <> "' and '" <> child <> "' in the schema cache" :: Text), @@ -471,6 +478,7 @@ data ErrorCode | ApiRequestErrorCode16 | ApiRequestErrorCode17 | ApiRequestErrorCode18 + | ApiRequestErrorCode19 -- Schema Cache errors | SchemaCacheErrorCode00 | SchemaCacheErrorCode01 @@ -513,6 +521,7 @@ buildErrorCode code = "PGRST" <> case code of ApiRequestErrorCode16 -> "116" ApiRequestErrorCode17 -> "117" ApiRequestErrorCode18 -> "118" + ApiRequestErrorCode19 -> "119" SchemaCacheErrorCode00 -> "200" SchemaCacheErrorCode01 -> "201" diff --git a/src/PostgREST/Plan.hs b/src/PostgREST/Plan.hs index 99c906d38..a79acace5 100644 --- a/src/PostgREST/Plan.hs +++ b/src/PostgREST/Plan.hs @@ -97,6 +97,7 @@ readPlan :: QualifiedIdentifier -> AppConfig -> SchemaCache -> ApiRequest -> Eit readPlan qi@QualifiedIdentifier{..} AppConfig{configDbMaxRows} SchemaCache{dbRelationships} apiRequest = mapLeft ApiRequestError $ treeRestrictRange configDbMaxRows (iAction apiRequest) =<< + validateSpreadEmbeds =<< addRelatedOrders =<< addRels qiSchema (iAction apiRequest) dbRelationships Nothing =<< addLogicTrees apiRequest =<< @@ -110,15 +111,23 @@ initReadRequest qi@QualifiedIdentifier{..} = foldr (treeEntry rootDepth) $ Node defReadPlan{from=qi, relName=qiName, depth=rootDepth} [] where rootDepth = 0 - defReadPlan = ReadPlan [] (QualifiedIdentifier mempty mempty) Nothing [] [] allRange mempty Nothing [] Nothing mempty Nothing Nothing rootDepth + defReadPlan = ReadPlan [] (QualifiedIdentifier mempty mempty) Nothing [] [] allRange mempty Nothing [] Nothing mempty Nothing Nothing False rootDepth treeEntry :: Depth -> Tree SelectItem -> ReadPlanTree -> ReadPlanTree - treeEntry depth (Node SelectRelation{..} fldForest) (Node q rForest) = + treeEntry depth (Node si fldForest) (Node q rForest) = let nxtDepth = succ depth in - Node q $ - foldr (treeEntry nxtDepth) - (Node defReadPlan{from=QualifiedIdentifier qiSchema selRelation, relName=selRelation, relAlias=selAlias, relHint=selHint, relJoinType=selJoinType, depth=nxtDepth} []) - fldForest:rForest - treeEntry _ (Node SelectField{..} _) (Node q rForest) = Node q{select=(selField, selCast, selAlias):select q} rForest + case si of + SelectRelation{..} -> + Node q $ + foldr (treeEntry nxtDepth) + (Node defReadPlan{from=QualifiedIdentifier qiSchema selRelation, relName=selRelation, relAlias=selAlias, relHint=selHint, relJoinType=selJoinType, depth=nxtDepth} []) + fldForest:rForest + SpreadRelation{..} -> + Node q $ + foldr (treeEntry nxtDepth) + (Node defReadPlan{from=QualifiedIdentifier qiSchema selRelation, relName=selRelation, relHint=selHint, relJoinType=selJoinType, depth=nxtDepth, relIsSpread=True} []) + fldForest:rForest + SelectField{..} -> + Node q{select=(selField, selCast, selAlias):select q} rForest -- | Enforces the `max-rows` config on the result treeRestrictRange :: Maybe Integer -> Action -> ReadPlanTree -> Either ApiRequestError ReadPlanTree @@ -344,6 +353,15 @@ addLogicTrees ApiRequest{..} rReq = addLogicTreeToNode :: (EmbedPath, LogicTree) -> Either ApiRequestError ReadPlanTree -> Either ApiRequestError ReadPlanTree addLogicTreeToNode = updateNode (\t (Node q@ReadPlan{where_=lf} f) -> Node q{ReadPlan.where_=t:lf} f) +-- 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 diff --git a/src/PostgREST/Plan/ReadPlan.hs b/src/PostgREST/Plan/ReadPlan.hs index 95566576c..e51170cdf 100644 --- a/src/PostgREST/Plan/ReadPlan.hs +++ b/src/PostgREST/Plan/ReadPlan.hs @@ -41,6 +41,7 @@ data ReadPlan = ReadPlan , relAggAlias :: Alias , relHint :: Maybe Hint , relJoinType :: Maybe JoinType + , relIsSpread :: Bool , depth :: Depth -- ^ used for aliasing } diff --git a/src/PostgREST/Query/QueryBuilder.hs b/src/PostgREST/Query/QueryBuilder.hs index c72ad4518..fa8d624a9 100644 --- a/src/PostgREST/Query/QueryBuilder.hs +++ b/src/PostgREST/Query/QueryBuilder.hs @@ -57,23 +57,25 @@ readPlanToQuery (Node ReadPlan{select,from=mainQi,fromAlias,where_=logicForest,o getSelectsJoins :: ReadPlanTree -> ([SQL.Snippet], [SQL.Snippet]) -> ([SQL.Snippet], [SQL.Snippet]) getSelectsJoins (Node ReadPlan{relToParent=Nothing} _) _ = ([], []) -getSelectsJoins rr@(Node ReadPlan{relName, relToParent=Just rel, relAggAlias, relAlias, relJoinType=joinType} _) (selects,joins) = +getSelectsJoins rr@(Node ReadPlan{relName, relToParent=Just rel, relAggAlias, relAlias, relJoinType, relIsSpread} _) (selects,joins) = let subquery = readPlanToQuery rr aliasOrName = pgFmtIdent $ fromMaybe relName relAlias aggAlias = pgFmtIdent relAggAlias correlatedSubquery sub al cond = - (if joinType == Just JTInner then "INNER" else "LEFT") <> " JOIN LATERAL ( " <> sub <> " ) AS " <> SQL.sql al <> " ON " <> cond + (if relJoinType == Just JTInner then "INNER" else "LEFT") <> " JOIN LATERAL ( " <> sub <> " ) AS " <> SQL.sql al <> " ON " <> cond (sel, joi) = if relIsToOne rel then - ( SQL.sql ("row_to_json(" <> aggAlias <> ".*) AS " <> aliasOrName) + ( if relIsSpread + then SQL.sql aggAlias <> ".*" + else SQL.sql ("row_to_json(" <> aggAlias <> ".*) AS " <> aliasOrName) , correlatedSubquery subquery aggAlias "TRUE") else ( SQL.sql $ "COALESCE( " <> aggAlias <> "." <> aggAlias <> ", '[]') AS " <> aliasOrName , correlatedSubquery ( "SELECT json_agg(" <> SQL.sql aggAlias <> ") AS " <> SQL.sql aggAlias <> "FROM (" <> subquery <> " ) AS " <> SQL.sql aggAlias - ) aggAlias $ if joinType == Just JTInner then SQL.sql aggAlias <> " IS NOT NULL" else "TRUE") + ) aggAlias $ if relJoinType == Just JTInner then SQL.sql aggAlias <> " IS NOT NULL" else "TRUE") in (sel:selects, joi:joins) diff --git a/test/spec/Feature/Query/SpreadQueriesSpec.hs b/test/spec/Feature/Query/SpreadQueriesSpec.hs new file mode 100644 index 000000000..ea519c1fe --- /dev/null +++ b/test/spec/Feature/Query/SpreadQueriesSpec.hs @@ -0,0 +1,86 @@ +module Feature.Query.SpreadQueriesSpec where + +import Network.Wai (Application) + +import Test.Hspec +import Test.Hspec.Wai +import Test.Hspec.Wai.JSON + +import Protolude hiding (get) +import SpecHelper + +spec :: SpecWith ((), Application) +spec = + describe "spread embeds" $ do + it "works on a many-to-one relationship" $ do + get "/projects?select=id,..clients(client_name:name)" `shouldRespondWith` + [json|[ + {"id":1,"client_name":"Microsoft"}, + {"id":2,"client_name":"Microsoft"}, + {"id":3,"client_name":"Apple"}, + {"id":4,"client_name":"Apple"}, + {"id":5,"client_name":null} + ]|] + { matchStatus = 200 + , matchHeaders = [matchContentTypeJson] + } + get "/grandchild_entities?select=name,..child_entities(parent_name:name,..entities(grandparent_name:name))&limit=3" `shouldRespondWith` + [json|[ + {"name":"grandchild entity 1","parent_name":"child entity 1","grandparent_name":"entity 1"}, + {"name":"grandchild entity 2","parent_name":"child entity 1","grandparent_name":"entity 1"}, + {"name":"grandchild entity 3","parent_name":"child entity 2","grandparent_name":"entity 1"} + ]|] + { matchStatus = 200 + , matchHeaders = [matchContentTypeJson] + } + get "/videogames?select=name,..computed_designers(designer_name:name)" `shouldRespondWith` + [json|[ + {"name":"Civilization I","designer_name":"Sid Meier"}, + {"name":"Civilization II","designer_name":"Sid Meier"}, + {"name":"Final Fantasy I","designer_name":"Hironobu Sakaguchi"}, + {"name":"Final Fantasy II","designer_name":"Hironobu Sakaguchi"} + ]|] + { matchStatus = 200 + , matchHeaders = [matchContentTypeJson] + } + + it "works inside a normal embed" $ + get "/grandchild_entities?select=name,child_entity:child_entities(name,..entities(parent_name:name))&limit=1" `shouldRespondWith` + [json|[ + {"name":"grandchild entity 1","child_entity":{"name":"child entity 1","parent_name":"entity 1"}} + ]|] + { matchStatus = 200 + , matchHeaders = [matchContentTypeJson] + } + + it "works on a one-to-one relationship" $ + get "/country?select=name,..capital(capital:name)" `shouldRespondWith` + [json|[ + {"name":"Afghanistan","capital":"Kabul"}, + {"name":"Algeria","capital":"Algiers"} + ]|] + { matchStatus = 200 + , matchHeaders = [matchContentTypeJson] + } + + it "fails when is not a to-one relationship" $ do + get "/clients?select=*,..projects(*)" `shouldRespondWith` + [json|{ + "code":"PGRST119", + "details":"'clients' and 'projects' do not form a many-to-one or one-to-one relationship", + "hint":null, + "message":"A spread operation on 'projects' is not possible" + }|] + { matchStatus = 400 + , matchHeaders = [matchContentTypeJson] + } + get "/designers?select=*,..computed_videogames(*)" `shouldRespondWith` + [json|{ + "code":"PGRST119", + "details":"'designers' and 'computed_videogames' do not form a many-to-one or one-to-one relationship", + "hint":null, + "message":"A spread operation on 'computed_videogames' is not possible" + }|] + { matchStatus = 400 + , matchHeaders = [matchContentTypeJson] + } diff --git a/test/spec/Main.hs b/test/spec/Main.hs index 72306bf56..aa443b821 100644 --- a/test/spec/Main.hs +++ b/test/spec/Main.hs @@ -56,6 +56,7 @@ import qualified Feature.Query.RawOutputTypesSpec import qualified Feature.Query.RelatedQueriesSpec import qualified Feature.Query.RpcSpec import qualified Feature.Query.SingularSpec +import qualified Feature.Query.SpreadQueriesSpec import qualified Feature.Query.UnicodeSpec import qualified Feature.Query.UpdateSpec import qualified Feature.Query.UpsertSpec @@ -151,6 +152,7 @@ main = do , ("Feature.Query.UpsertSpec" , Feature.Query.UpsertSpec.spec actualPgVersion) , ("Feature.Query.ComputedRelsSpec" , Feature.Query.ComputedRelsSpec.spec) , ("Feature.Query.RelatedQueriesSpec" , Feature.Query.RelatedQueriesSpec.spec) + , ("Feature.Query.SpreadQueriesSpec" , Feature.Query.SpreadQueriesSpec.spec) ] hspec $ do