From 337f821e00424957f1e8f36f762847182c1a073b Mon Sep 17 00:00:00 2001 From: steve-chavez Date: Thu, 3 Oct 2019 19:29:25 -0500 Subject: [PATCH] refactor: remove parent embed workaround(in #647) This workaround is no more necessary since the addition of #978. * Also add a test for proving parent embeds offset is consistent with other types of embeds. --- src/PostgREST/App.hs | 6 +-- src/PostgREST/QueryBuilder.hs | 63 +++++++++++++++----------------- test/Feature/QueryLimitedSpec.hs | 9 ++++- 3 files changed, 41 insertions(+), 37 deletions(-) diff --git a/src/PostgREST/App.hs b/src/PostgREST/App.hs index e4e0a99b6..c853b91ab 100644 --- a/src/PostgREST/App.hs +++ b/src/PostgREST/App.hs @@ -335,22 +335,22 @@ app dbStructure proc cols conf apiRequest = topLevelRange = iTopLevelRange apiRequest returnsScalar = maybe False procReturnsScalar proc - selectQuery = readRequestToQuery False readSqlParts s t = let readReq = readRequest s t maxRows (dbRelations dbStructure) apiRequest in (,,) <$> - (selectQuery <$> readReq) <*> + (readRequestToQuery <$> readReq) <*> (readRequestToCountQuery <$> readReq) <*> (binaryField contentType rawContentTypes returnsScalar =<< readReq) + mutateSqlParts s t = let readReq = readRequest s t maxRows (dbRelations dbStructure) apiRequest mutReq = mutateRequest s t apiRequest cols (tablePKCols dbStructure s t) =<< readReq in (,) <$> - (selectQuery <$> readReq) <*> + (readRequestToQuery <$> readReq) <*> (mutateRequestToQuery <$> mutReq) responseContentTypeOrError :: [ContentType] -> [ContentType] -> Action -> Target -> Either Response ContentType diff --git a/src/PostgREST/QueryBuilder.hs b/src/PostgREST/QueryBuilder.hs index 96ceee54c..9a6c78255 100644 --- a/src/PostgREST/QueryBuilder.hs +++ b/src/PostgREST/QueryBuilder.hs @@ -34,8 +34,8 @@ import PostgREST.Types import Protolude hiding (cast, intercalate, replace) -readRequestToQuery :: Bool -> ReadRequest -> SqlQuery -readRequestToQuery isParent (Node (Select colSelects mainQi tblAlias implJoins logicForest joinConditions_ ordts range, _) forest) = +readRequestToQuery :: ReadRequest -> SqlQuery +readRequestToQuery (Node (Select colSelects mainQi tblAlias implJoins logicForest joinConditions_ ordts range, _) forest) = unwords [ "SELECT " <> intercalate ", " (map (pgFmtSelectItem qi) colSelects ++ selects), "FROM " <> intercalate ", " (tabl : implJs), @@ -43,42 +43,39 @@ readRequestToQuery isParent (Node (Select colSelects mainQi tblAlias implJoins l ("WHERE " <> intercalate " AND " (map (pgFmtLogicTree qi) logicForest ++ map pgFmtJoinCondition joinConditions_)) `emptyOnFalse` (null logicForest && null joinConditions_), ("ORDER BY " <> intercalate ", " (map (pgFmtOrderTerm qi) ordts)) `emptyOnFalse` null ordts, - ("LIMIT " <> maybe "ALL" show (rangeLimit range) <> " OFFSET " <> show (rangeOffset range)) `emptyOnFalse` (isParent || range == allRange) ] - + ("LIMIT " <> maybe "ALL" show (rangeLimit range) <> " OFFSET " <> show (rangeOffset range)) `emptyOnFalse` (range == allRange) + ] where implJs = fromQi <$> implJoins tabl = fromQi mainQi <> maybe mempty (\a -> " AS " <> pgFmtIdent a) tblAlias qi = maybe mainQi (QualifiedIdentifier mempty) tblAlias + (joins, selects) = foldr getJoinsSelects ([],[]) forest - (joins, selects) = foldr getQueryParts ([],[]) forest - - getQueryParts :: Tree ReadNode -> ([SqlFragment], [SqlFragment]) -> ([SqlFragment], [SqlFragment]) - getQueryParts (Node n@(_, (name, Just Relation{relType=Child,relTable=Table{tableName=table}}, alias, _, _)) forst) (j,s) = (j,sel:s) - where - sel = "COALESCE((" - <> "SELECT json_agg(" <> pgFmtIdent table <> ".*) " - <> "FROM (" <> subquery <> ") " <> pgFmtIdent table - <> "), '[]') AS " <> pgFmtIdent (fromMaybe name alias) - where subquery = readRequestToQuery False (Node n forst) - getQueryParts (Node n@(_, (name, Just Relation{relType=Parent,relTable=Table{tableName=table}}, alias, _, _)) forst) (j,s) = (joi:j,sel:s) - where - aliasOrName = fromMaybe name alias - localTableName = pgFmtIdent $ table <> "_" <> aliasOrName - sel = "row_to_json(" <> localTableName <> ".*) AS " <> pgFmtIdent aliasOrName - joi = " LEFT JOIN LATERAL( " <> subquery <> " ) AS " <> localTableName <> " ON TRUE " - where subquery = readRequestToQuery True (Node n forst) - getQueryParts (Node n@(_, (name, Just Relation{relType=Many,relTable=Table{tableName=table}}, alias, _, _)) forst) (j,s) = (j,sel:s) - where - sel = "COALESCE ((" - <> "SELECT json_agg(" <> pgFmtIdent table <> ".*) " - <> "FROM (" <> subquery <> ") " <> pgFmtIdent table - <> "), '[]') AS " <> pgFmtIdent (fromMaybe name alias) - where subquery = readRequestToQuery False (Node n forst) - --the following is just to remove the warning - --getQueryParts is not total but readRequestToQuery is called only after addJoinConditions which ensures the only - --posible relations are Child Parent Many - getQueryParts _ _ = witness - +getJoinsSelects :: ReadRequest -> ([SqlFragment], [SqlFragment]) -> ([SqlFragment], [SqlFragment]) +getJoinsSelects rr@(Node (_, (name, Just Relation{relType=relTyp,relTable=Table{tableName=table}}, alias, _, _)) _) (j,s) = + let subquery = readRequestToQuery rr in + case relTyp of + Child -> + let sel = "COALESCE((" + <> "SELECT json_agg(" <> pgFmtIdent table <> ".*) " + <> "FROM (" <> subquery <> ") " <> pgFmtIdent table + <> "), '[]') AS " <> pgFmtIdent (fromMaybe name alias) in + (j, sel:s) + Parent -> + let aliasOrName = fromMaybe name alias + localTableName = pgFmtIdent $ table <> "_" <> aliasOrName + sel = "row_to_json(" <> localTableName <> ".*) AS " <> pgFmtIdent aliasOrName + joi = " LEFT JOIN LATERAL( " <> subquery <> " ) AS " <> localTableName <> " ON TRUE " in + (joi:j,sel:s) + Many -> + let sel = "COALESCE ((" + <> "SELECT json_agg(" <> pgFmtIdent table <> ".*) " + <> "FROM (" <> subquery <> ") " <> pgFmtIdent table + <> "), '[]') AS " <> pgFmtIdent (fromMaybe name alias) in + (j,sel:s) +--readRequestToQuery is called only after addJoinConditions which ensures the only posible relations are Child Parent Many + Root -> witness +getJoinsSelects _ _ = witness mutateRequestToQuery :: MutateRequest -> SqlQuery mutateRequestToQuery (Insert mainQi iCols onConflct putConditions returnings) = diff --git a/test/Feature/QueryLimitedSpec.hs b/test/Feature/QueryLimitedSpec.hs index 1ab4dc5af..e4b1d5202 100644 --- a/test/Feature/QueryLimitedSpec.hs +++ b/test/Feature/QueryLimitedSpec.hs @@ -36,13 +36,20 @@ spec = , matchHeaders = ["Content-Range" <:> "0-1/*"] } - it "is not applied to parent embeds" $ + it "succeeds in getting parent embeds despite the limit, see #647" $ get "/tasks?select=id,project(id)&id=gt.5" `shouldRespondWith` [json|[{"id":6,"project":{"id":3}},{"id":7,"project":{"id":4}}]|] { matchStatus = 200 , matchHeaders = ["Content-Range" <:> "0-1/*"] } + it "can offset the parent embed, being consistent with the other embed types" $ + get "/tasks?select=id,project:projects(id)&id=gt.5&project.offset=1" + `shouldRespondWith` [json|[{"id":6,"project":null}, {"id":7,"project":null}]|] + { matchStatus = 200 + , matchHeaders = ["Content-Range" <:> "0-1/*"] + } + context "count=estimated" $ do it "uses the query planner guess when query rows > maxRows" $ request methodHead "/getallprojects_view" [("Prefer", "count=estimated")] ""