Skip to content

Commit

Permalink
fix: error when requesting "Prefer: count=<type>" with null filters o…
Browse files Browse the repository at this point in the history
…n embedded resources
  • Loading branch information
laurenceisla authored Sep 16, 2023
1 parent c3169b7 commit 37a3f81
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 1 deletion.
14 changes: 13 additions & 1 deletion src/PostgREST/Query/QueryBuilder.hs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ readPlanToCountQuery (Node ReadPlan{from=mainQi, fromAlias=tblAlias, where_=logi
then mempty
else " WHERE " ) <>
intercalateSnippet " AND " (
map (pgFmtLogicTree qi) logicForest ++
map (pgFmtLogicTreeCount qi) logicForest ++
map pgFmtJoinCondition relJoinConds ++
subQueries
)
Expand All @@ -216,6 +216,18 @@ readPlanToCountQuery (Node ReadPlan{from=mainQi, fromAlias=tblAlias, where_=logi
if joinType == Just JTInner
then ("EXISTS (" <> readPlanToCountQuery readReq <> " )"):rest
else rest
findNullEmbedRel fld = find (\(Node ReadPlan{relAggAlias} _) -> fld == relAggAlias) forest

-- https://github.com/PostgREST/postgrest/pull/2930#discussion_r1325293698
pgFmtLogicTreeCount :: QualifiedIdentifier -> CoercibleLogicTree -> SQL.Snippet
pgFmtLogicTreeCount qiCount (CoercibleExpr hasNot op frst) = SQL.sql notOp <> " (" <> intercalateSnippet (opSql op) (pgFmtLogicTreeCount qiCount <$> frst) <> ")"
where
notOp = if hasNot then "NOT" else mempty
opSql And = " AND "
opSql Or = " OR "
pgFmtLogicTreeCount _ (CoercibleStmnt (CoercibleFilterNullEmbed hasNot fld)) =
maybe mempty (\x -> (if not hasNot then "NOT " else mempty) <> "EXISTS (" <> readPlanToCountQuery x <> ")") (findNullEmbedRel fld)
pgFmtLogicTreeCount qiCount (CoercibleStmnt flt) = pgFmtFilter qiCount flt

limitedQuery :: SQL.Snippet -> Maybe Integer -> SQL.Snippet
limitedQuery query maxRows = query <> SQL.sql (maybe mempty (\x -> " LIMIT " <> BS.pack (show x)) maxRows)
Expand Down
1 change: 1 addition & 0 deletions src/PostgREST/Query/SqlFragment.hs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ module PostgREST.Query.SqlFragment
, mutRangeF
, orderF
, pgFmtColumn
, pgFmtFilter
, pgFmtIdent
, pgFmtJoinCondition
, pgFmtLogicTree
Expand Down
103 changes: 103 additions & 0 deletions test/spec/Feature/Query/RelatedQueriesSpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ module Feature.Query.RelatedQueriesSpec where

import Network.Wai (Application)

import Network.HTTP.Types
import Test.Hspec
import Test.Hspec.Wai
import Test.Hspec.Wai.JSON
Expand Down Expand Up @@ -275,3 +276,105 @@ spec = describe "related queries" $ do
{ matchStatus = 200
, matchHeaders = [matchContentTypeJson]
}

it "works with count=exact" $ do
request methodGet "/projects?select=name,clients(name)&clients=not.is.null"
[("Prefer", "count=exact")] ""
`shouldRespondWith`
[json|[
{"name":"Windows 7", "clients":{"name":"Microsoft"}},
{"name":"Windows 10", "clients":{"name":"Microsoft"}},
{"name":"IOS", "clients":{"name":"Apple"}},
{"name":"OSX", "clients":{"name":"Apple"}}
]|]
{ matchStatus = 200
, matchHeaders = [ matchContentTypeJson
, "Content-Range" <:> "0-3/4" ]
}
request methodGet "/projects?select=name,clients()&clients=is.null"
[("Prefer", "count=exact")] ""
`shouldRespondWith`
[json|[{"name":"Orphan"}]|]
{ matchStatus = 200
, matchHeaders = [ matchContentTypeJson
, "Content-Range" <:> "0-0/1" ]
}
request methodGet "/client?select=*,clientinfo(),contact()&clientinfo.other=ilike.*main*&contact.name=ilike.*tabby*&or=(clientinfo.not.is.null,contact.not.is.null)"
[("Prefer", "count=exact")] ""
`shouldRespondWith`
[json|[
{"id":1,"name":"Walmart"},
{"id":2,"name":"Target"}
]|]
{ matchStatus = 200
, matchHeaders = [ matchContentTypeJson
, "Content-Range" <:> "0-1/2" ]
}

it "works with count=planned" $ do
request methodGet "/projects?select=name,clients(name)&clients=not.is.null"
[("Prefer", "count=planned")] ""
`shouldRespondWith`
[json|[
{"name":"Windows 7", "clients":{"name":"Microsoft"}},
{"name":"Windows 10", "clients":{"name":"Microsoft"}},
{"name":"IOS", "clients":{"name":"Apple"}},
{"name":"OSX", "clients":{"name":"Apple"}}
]|]
{ matchStatus = 206
, matchHeaders = [ matchContentTypeJson
, "Content-Range" <:> "0-3/1200" ]
}
request methodGet "/projects?select=name,clients()&clients=is.null"
[("Prefer", "count=planned")] ""
`shouldRespondWith`
[json|[{"name":"Orphan"}]|]
{ matchStatus = 200
, matchHeaders = [ matchContentTypeJson
, "Content-Range" <:> "0-0/1" ]
}
request methodGet "/client?select=*,clientinfo(),contact()&clientinfo.other=ilike.*main*&contact.name=ilike.*tabby*&or=(clientinfo.not.is.null,contact.not.is.null)"
[("Prefer", "count=planned")] ""
`shouldRespondWith`
[json|[
{"id":1,"name":"Walmart"},
{"id":2,"name":"Target"}
]|]
{ matchStatus = 206
, matchHeaders = [ matchContentTypeJson
, "Content-Range" <:> "0-1/952" ]
}

it "works with count=estimated" $ do
request methodGet "/projects?select=name,clients(name)&clients=not.is.null"
[("Prefer", "count=estimated")] ""
`shouldRespondWith`
[json|[
{"name":"Windows 7", "clients":{"name":"Microsoft"}},
{"name":"Windows 10", "clients":{"name":"Microsoft"}},
{"name":"IOS", "clients":{"name":"Apple"}},
{"name":"OSX", "clients":{"name":"Apple"}}
]|]
{ matchStatus = 206
, matchHeaders = [ matchContentTypeJson
, "Content-Range" <:> "0-3/1200" ]
}
request methodGet "/projects?select=name,clients()&clients=is.null"
[("Prefer", "count=estimated")] ""
`shouldRespondWith`
[json|[{"name":"Orphan"}]|]
{ matchStatus = 200
, matchHeaders = [ matchContentTypeJson
, "Content-Range" <:> "0-0/1" ]
}
request methodGet "/client?select=*,clientinfo(),contact()&clientinfo.other=ilike.*main*&contact.name=ilike.*tabby*&or=(clientinfo.not.is.null,contact.not.is.null)"
[("Prefer", "count=estimated")] ""
`shouldRespondWith`
[json|[
{"id":1,"name":"Walmart"},
{"id":2,"name":"Target"}
]|]
{ matchStatus = 206
, matchHeaders = [ matchContentTypeJson
, "Content-Range" <:> "0-1/952" ]
}

0 comments on commit 37a3f81

Please sign in to comment.