From cc2d480e9a5823dda4a900ba6050a169a15c187c Mon Sep 17 00:00:00 2001 From: Laurence Isla Date: Fri, 15 Sep 2023 16:45:06 -0500 Subject: [PATCH 1/2] fix count query for null embed filters --- src/PostgREST/Query/QueryBuilder.hs | 13 ++++++- src/PostgREST/Query/SqlFragment.hs | 1 + test/spec/Feature/Query/RelatedQueriesSpec.hs | 35 +++++++++++++++++++ 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/PostgREST/Query/QueryBuilder.hs b/src/PostgREST/Query/QueryBuilder.hs index fb6408a2a4..397eca3c1f 100644 --- a/src/PostgREST/Query/QueryBuilder.hs +++ b/src/PostgREST/Query/QueryBuilder.hs @@ -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 ) @@ -216,6 +216,17 @@ 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 + + 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) diff --git a/src/PostgREST/Query/SqlFragment.hs b/src/PostgREST/Query/SqlFragment.hs index a56abdce35..f2d21efce6 100644 --- a/src/PostgREST/Query/SqlFragment.hs +++ b/src/PostgREST/Query/SqlFragment.hs @@ -15,6 +15,7 @@ module PostgREST.Query.SqlFragment , mutRangeF , orderF , pgFmtColumn + , pgFmtFilter , pgFmtIdent , pgFmtJoinCondition , pgFmtLogicTree diff --git a/test/spec/Feature/Query/RelatedQueriesSpec.hs b/test/spec/Feature/Query/RelatedQueriesSpec.hs index 0b83cbb680..1c67e64769 100644 --- a/test/spec/Feature/Query/RelatedQueriesSpec.hs +++ b/test/spec/Feature/Query/RelatedQueriesSpec.hs @@ -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 @@ -275,3 +276,37 @@ 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" ] + } From a62d1bdf52780b1ea1b9dcd25a15bfba8c818c75 Mon Sep 17 00:00:00 2001 From: Laurence Isla Date: Fri, 15 Sep 2023 18:36:52 -0500 Subject: [PATCH 2/2] =?UTF-8?q?Add=20tests=20for=20count=3Dp=C4=BAanned=20?= =?UTF-8?q?and=20estimated?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/PostgREST/Query/QueryBuilder.hs | 1 + test/spec/Feature/Query/RelatedQueriesSpec.hs | 68 +++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/src/PostgREST/Query/QueryBuilder.hs b/src/PostgREST/Query/QueryBuilder.hs index 397eca3c1f..02a4a8926f 100644 --- a/src/PostgREST/Query/QueryBuilder.hs +++ b/src/PostgREST/Query/QueryBuilder.hs @@ -218,6 +218,7 @@ readPlanToCountQuery (Node ReadPlan{from=mainQi, fromAlias=tblAlias, where_=logi 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 diff --git a/test/spec/Feature/Query/RelatedQueriesSpec.hs b/test/spec/Feature/Query/RelatedQueriesSpec.hs index 1c67e64769..de0dd8f2d0 100644 --- a/test/spec/Feature/Query/RelatedQueriesSpec.hs +++ b/test/spec/Feature/Query/RelatedQueriesSpec.hs @@ -310,3 +310,71 @@ spec = describe "related queries" $ do , 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" ] + }