Skip to content

Commit

Permalink
feat: add related orders
Browse files Browse the repository at this point in the history
  • Loading branch information
steve-chavez committed Nov 2, 2022
1 parent bafbb6f commit ea7a7a8
Show file tree
Hide file tree
Showing 8 changed files with 132 additions and 21 deletions.
21 changes: 19 additions & 2 deletions src/PostgREST/ApiRequest/QueryParams.hs
Original file line number Diff line number Diff line change
Expand Up @@ -594,17 +594,34 @@ pDelimiter = char '.' <?> "delimiter (.)"
--
-- >>> P.parse pOrder "" "json_col->key.asc.nullslast"
-- Right [OrderTerm {otTerm = ("json_col",[JArrow {jOp = JKey {jVal = "key"}}]), otDirection = Just OrderAsc, otNullOrder = Just OrderNullsLast}]
--
-- >>> P.parse pOrder "" "clients(json_col->key).desc.nullsfirst"
-- Right [OrderRelationTerm {otRelation = "clients", otRelTerm = ("json_col",[JArrow {jOp = JKey {jVal = "key"}}]), otDirection = Just OrderDesc, otNullOrder = Just OrderNullsFirst}]
--
-- >>> P.parse pOrder "" "clients(name,id)"
-- Left (line 1, column 8):
-- unexpected '('
-- expecting letter, digit, "-", "->>", "->", delimiter (.), "," or end of input
--
-- >>> P.parse pOrder "" "name,clients(name),id"
-- Right [OrderTerm {otTerm = ("name",[]), otDirection = Nothing, otNullOrder = Nothing},OrderRelationTerm {otRelation = "clients", otRelTerm = ("name",[]), otDirection = Nothing, otNullOrder = Nothing},OrderTerm {otTerm = ("id",[]), otDirection = Nothing, otNullOrder = Nothing}]
pOrder :: Parser [OrderTerm]
pOrder = lexeme pOrderTerm `sepBy1` char ','
pOrder = lexeme (try pOrderRelationTerm <|> pOrderTerm) `sepBy1` char ','
where
pOrderTerm :: Parser OrderTerm
pOrderTerm = do
fld <- pField
dir <- optionMaybe pOrdDir
nls <- optionMaybe pNulls <* pEnd <|>
pEnd $> Nothing
return $ OrderTerm fld dir nls

pOrderRelationTerm = do
nam <- pFieldName
fld <- between (char '(') (char ')') pField
dir <- optionMaybe pOrdDir
nls <- optionMaybe pNulls <* pEnd <|> pEnd $> Nothing
return $ OrderRelationTerm nam fld dir nls

pNulls :: Parser OrderNulls
pNulls = try (pDelimiter *> string "nullsfirst" $> OrderNullsFirst) <|>
try (pDelimiter *> string "nullslast" $> OrderNullsLast)
Expand Down
20 changes: 14 additions & 6 deletions src/PostgREST/ApiRequest/Types.hs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ data ApiRequestError
| InvalidRpcMethod ByteString
| LimitNoOrderError
| NotFound
| NotToOne Text Text
| NoRelBetween Text Text Text
| NoRpc Text Text [Text] Bool MediaType Bool
| NotEmbedded Text
Expand All @@ -82,12 +83,19 @@ data RangeError
type NodeName = Text
type Depth = Integer

data OrderTerm = OrderTerm
{ otTerm :: Field
, otDirection :: Maybe OrderDirection
, otNullOrder :: Maybe OrderNulls
}
deriving (Eq)
data OrderTerm
= OrderTerm
{ otTerm :: Field
, otDirection :: Maybe OrderDirection
, otNullOrder :: Maybe OrderNulls
}
| OrderRelationTerm
{ otRelation :: FieldName
, otRelTerm :: Field
, otDirection :: Maybe OrderDirection
, otNullOrder :: Maybe OrderNulls
}
deriving Eq

data OrderDirection
= OrderAsc
Expand Down
13 changes: 12 additions & 1 deletion src/PostgREST/Error.hs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ instance PgrstError ApiRequestError where
status InvalidRpcMethod{} = HTTP.status405
status InvalidRange{} = HTTP.status416
status NotFound = HTTP.status404
status NotToOne{} = HTTP.status400

status NoRelBetween{} = HTTP.status400
status NoRpc{} = HTTP.status404
status NotEmbedded{} = HTTP.status400
Expand Down Expand Up @@ -123,7 +125,7 @@ instance JSON.ToJSON ApiRequestError where
toJSON NotFound = JSON.object []
toJSON (NotEmbedded resource) = JSON.object [
"code" .= ApiRequestErrorCode08,
"message" .= ("Cannot apply filter because '" <> resource <> "' is not an embedded resource in this request" :: Text),
"message" .= ("Cannot comply because '" <> resource <> "' is not an embedded resource in this request" :: Text),
"details" .= JSON.Null,
"hint" .= ("Verify that '" <> resource <> "' is included in the 'select' query parameter." :: Text)]

Expand Down Expand Up @@ -151,11 +153,18 @@ instance JSON.ToJSON ApiRequestError where
"details" .= JSON.Null,
"hint" .= JSON.Null]

toJSON (NotToOne origin target) = JSON.object [
"code" .= ApiRequestErrorCode18,
"message" .= ("'" <> origin <> "' and '" <> target <> "' do not form a many-to-one or one-to-one relationship" :: Text),
"details" .= JSON.Null,
"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),
"details" .= JSON.Null,
"hint" .= ("Verify that '" <> parent <> "' and '" <> child <> "' exist in the schema '" <> schema <> "' and that there is a foreign key relationship between them. If a new relationship was created, try reloading the schema cache." :: Text)]

toJSON (AmbiguousRelBetween parent child rels) = JSON.object [
"code" .= SchemaCacheErrorCode01,
"message" .= ("Could not embed because more than one relationship was found for '" <> parent <> "' and '" <> child <> "'" :: Text),
Expand Down Expand Up @@ -461,6 +470,7 @@ data ErrorCode
| ApiRequestErrorCode15
| ApiRequestErrorCode16
| ApiRequestErrorCode17
| ApiRequestErrorCode18
-- Schema Cache errors
| SchemaCacheErrorCode00
| SchemaCacheErrorCode01
Expand Down Expand Up @@ -502,6 +512,7 @@ buildErrorCode code = "PGRST" <> case code of
ApiRequestErrorCode15 -> "115"
ApiRequestErrorCode16 -> "116"
ApiRequestErrorCode17 -> "117"
ApiRequestErrorCode18 -> "118"

SchemaCacheErrorCode00 -> "200"
SchemaCacheErrorCode01 -> "201"
Expand Down
22 changes: 21 additions & 1 deletion src/PostgREST/Plan.hs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ import PostgREST.SchemaCache.Proc (ProcDescription (..),
import PostgREST.SchemaCache.Relationship (Cardinality (..),
Junction (..),
Relationship (..),
RelationshipsMap)
RelationshipsMap,
relIsToOne)
import PostgREST.SchemaCache.Table (tablePKCols)

import PostgREST.Plan.CallPlan
Expand Down Expand Up @@ -95,6 +96,7 @@ readPlan :: QualifiedIdentifier -> AppConfig -> SchemaCache -> ApiRequest -> Eit
readPlan qi@QualifiedIdentifier{..} AppConfig{configDbMaxRows} SchemaCache{dbRelationships} apiRequest =
mapLeft ApiRequestError $
treeRestrictRange configDbMaxRows (iAction apiRequest) =<<
addRelatedOrders =<<
addRels qiSchema (iAction apiRequest) dbRelationships Nothing =<<
addLogicTrees apiRequest =<<
addRanges apiRequest =<<
Expand Down Expand Up @@ -298,6 +300,24 @@ addOrders ApiRequest{..} rReq =
addOrderToNode :: (EmbedPath, [OrderTerm]) -> Either ApiRequestError ReadPlanTree -> Either ApiRequestError ReadPlanTree
addOrderToNode = updateNode (\o (Node q f) -> Node q{order=o} f)

addRelatedOrders :: ReadPlanTree -> Either ApiRequestError ReadPlanTree
addRelatedOrders (Node rp@ReadPlan{order,from} forest) = do
newOrder <- getRelOrder `traverse` order
Node rp{order=newOrder} <$> addRelatedOrders `traverse` forest
where
getRelOrder ot@OrderTerm{} = Right ot
getRelOrder ot@OrderRelationTerm{otRelation} =
let foundRP = rootLabel <$> find (\(Node ReadPlan{relName, relAlias} _) -> Just otRelation `elem` [Just 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 $ ot{otRelation=relAggAlias}
else Left $ NotToOne (qiName from) name
Nothing ->
Left $ NotEmbedded otRelation

addRanges :: ApiRequest -> ReadPlanTree -> Either ApiRequestError ReadPlanTree
addRanges ApiRequest{..} rReq =
case iAction of
Expand Down
10 changes: 3 additions & 7 deletions src/PostgREST/Query/QueryBuilder.hs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ import PostgREST.SchemaCache.Identifiers (QualifiedIdentifier (..))
import PostgREST.SchemaCache.Proc (ProcParam (..))
import PostgREST.SchemaCache.Relationship (Cardinality (..),
Junction (..),
Relationship (..))
Relationship (..),
relIsToOne)

import PostgREST.ApiRequest.Types
import PostgREST.Plan.CallPlan
Expand Down Expand Up @@ -63,12 +64,7 @@ getSelectsJoins rr@(Node ReadPlan{relName, relToParent=Just rel, relAggAlias, re
aggAlias = pgFmtIdent relAggAlias
correlatedSubquery sub al cond =
(if joinType == Just JTInner then "INNER" else "LEFT") <> " JOIN LATERAL ( " <> sub <> " ) AS " <> SQL.sql al <> " ON " <> cond
isToOne = case rel of
Relationship{relCardinality=M2O _ _} -> True
Relationship{relCardinality=O2O _ _} -> True
ComputedRelationship{relToOne=True} -> True
_ -> False
(sel, joi) = if isToOne
(sel, joi) = if relIsToOne rel
then
( SQL.sql ("row_to_json(" <> aggAlias <> ".*) AS " <> aliasOrName)
, correlatedSubquery subquery aggAlias "TRUE")
Expand Down
6 changes: 5 additions & 1 deletion src/PostgREST/Query/SqlFragment.hs
Original file line number Diff line number Diff line change
Expand Up @@ -244,11 +244,15 @@ pgFmtSelectItem table (f@(fName, jp), Just cast, alias) = "CAST (" <> pgFmtField

pgFmtOrderTerm :: QualifiedIdentifier -> OrderTerm -> SQL.Snippet
pgFmtOrderTerm qi ot =
pgFmtField qi (otTerm ot) <> " " <>
fmtOTerm ot <> " " <>
SQL.sql (BS.unwords [
maybe mempty direction $ otDirection ot,
maybe mempty nullOrder $ otNullOrder ot])
where
fmtOTerm = \case
OrderTerm{otTerm} -> pgFmtField qi otTerm
OrderRelationTerm{otRelation, otRelTerm} -> pgFmtField (QualifiedIdentifier mempty otRelation) otRelTerm

direction OrderAsc = "ASC"
direction OrderDesc = "DESC"

Expand Down
8 changes: 8 additions & 0 deletions src/PostgREST/SchemaCache/Relationship.hs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ module PostgREST.SchemaCache.Relationship
, Relationship(..)
, Junction(..)
, RelationshipsMap
, relIsToOne
) where

import qualified Data.Aeson as JSON
Expand Down Expand Up @@ -62,3 +63,10 @@ data Junction = Junction

-- | Key based on the source table and the foreign table schema
type RelationshipsMap = HM.HashMap (QualifiedIdentifier, Schema) [Relationship]

relIsToOne :: Relationship -> Bool
relIsToOne rel = case rel of
Relationship{relCardinality=M2O _ _} -> True
Relationship{relCardinality=O2O _ _} -> True
ComputedRelationship{relToOne=True} -> True
_ -> False
53 changes: 50 additions & 3 deletions test/spec/Feature/Query/QuerySpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ spec actualPgVersion = do
{"hint":"Verify that 'non_existent_projects' is included in the 'select' query parameter.",
"details":null,
"code":"PGRST108",
"message":"Cannot apply filter because 'non_existent_projects' is not an embedded resource in this request"}|]
"message":"Cannot comply because 'non_existent_projects' is not an embedded resource in this request"}|]
{ matchStatus = 400
, matchHeaders = [matchContentTypeJson]
}
Expand All @@ -300,7 +300,7 @@ spec actualPgVersion = do
{"hint":"Verify that 'amiga_projectsss' is included in the 'select' query parameter.",
"details":null,
"code":"PGRST108",
"message":"Cannot apply filter because 'amiga_projectsss' is not an embedded resource in this request"}|]
"message":"Cannot comply because 'amiga_projectsss' is not an embedded resource in this request"}|]
{ matchStatus = 400
, matchHeaders = [matchContentTypeJson]
}
Expand All @@ -309,7 +309,7 @@ spec actualPgVersion = do
{"hint":"Verify that 'tasks2' is included in the 'select' query parameter.",
"details":null,
"code":"PGRST108",
"message":"Cannot apply filter because 'tasks2' is not an embedded resource in this request"}|]
"message":"Cannot comply because 'tasks2' is not an embedded resource in this request"}|]
{ matchStatus = 400
, matchHeaders = [matchContentTypeJson]
}
Expand Down Expand Up @@ -966,6 +966,53 @@ spec actualPgVersion = do
, matchHeaders = [matchContentTypeJson]
}

context "order parent by child" $ do
it "works when it's a many-to-one relationship" $ do
get "/projects?select=id,clients(name)&order=clients(name).nullsfirst" `shouldRespondWith`
[json|[
{"id":5,"clients":null},
{"id":3,"clients":{"name":"Apple"}},
{"id":4,"clients":{"name":"Apple"}},
{"id":1,"clients":{"name":"Microsoft"}},
{"id":2,"clients":{"name":"Microsoft"}} ]|]
{ matchStatus = 200
, matchHeaders = [matchContentTypeJson]
}
get "/projects?select=id,client:clients(name)&order=client(name).asc" `shouldRespondWith`
[json|[
{"id":3,"client":{"name":"Apple"}},
{"id":4,"client":{"name":"Apple"}},
{"id":1,"client":{"name":"Microsoft"}},
{"id":2,"client":{"name":"Microsoft"}},
{"id":5,"client":null} ]|]
{ matchStatus = 200
, matchHeaders = [matchContentTypeJson]
}

it "fails when is not a to-one relationship" $ do
get "/clients?select=*,projects(*)&order=projects(id)" `shouldRespondWith`
[json|{"code":"PGRST118","details":null,"hint":null,"message":"'clients' and 'projects' do not form a many-to-one or one-to-one relationship"}|]
{ matchStatus = 400
, matchHeaders = [matchContentTypeJson]
}
get "/clients?select=*,pros:projects(*)&order=pros(id)" `shouldRespondWith`
[json|{"code":"PGRST118","details":null,"hint":null,"message":"'clients' and 'pros' do not form a many-to-one or one-to-one relationship"}|]
{ matchStatus = 400
, matchHeaders = [matchContentTypeJson]
}

it "fails when the resource is not embedded" $
get "/projects?select=id,clients(name)&order=clientsx(name).nullsfirst" `shouldRespondWith`
[json|{
"code":"PGRST108",
"details":null,
"hint":"Verify that 'clientsx' is included in the 'select' query parameter.",
"message":"Cannot comply because 'clientsx' is not an embedded resource in this request"
}|]
{ matchStatus = 400
, matchHeaders = [matchContentTypeJson]
}

describe "Accept headers" $ do
it "should respond an unknown accept type with 415" $
request methodGet "/simple_pk"
Expand Down

0 comments on commit ea7a7a8

Please sign in to comment.