From 35c15c7f8b48bf074690ca8ef366c15028f8ba8a Mon Sep 17 00:00:00 2001 From: steve-chavez Date: Thu, 24 Nov 2022 19:24:43 -0500 Subject: [PATCH 1/3] feat: Allow embedding without selecting any column --- CHANGELOG.md | 1 + src/PostgREST/ApiRequest/QueryParams.hs | 6 ++--- src/PostgREST/Query/QueryBuilder.hs | 4 ++-- test/spec/Feature/Query/QuerySpec.hs | 32 +++++++++++++++++++++++++ 4 files changed, 38 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0966bf1669..7b6fb0e8df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). + Allows including the join table columns when resource embedding + Allows disambiguating a recursive m2m embed + Allows disambiguating an embed that has a many-to-many relationship using two foreign keys on a junction + - #2340, Allow embedding without selecting any column - @steve-chavez ### Fixed diff --git a/src/PostgREST/ApiRequest/QueryParams.hs b/src/PostgREST/ApiRequest/QueryParams.hs index 95e69907c2..087fe2c825 100644 --- a/src/PostgREST/ApiRequest/QueryParams.hs +++ b/src/PostgREST/ApiRequest/QueryParams.hs @@ -36,8 +36,8 @@ import Text.ParserCombinators.Parsec (GenParser, ParseError, Parser, eof, errorPos, letter, lookAhead, many1, noneOf, notFollowedBy, oneOf, - optionMaybe, sepBy1, string, - try, ()) + optionMaybe, sepBy, sepBy1, + string, try, ()) import PostgREST.RangeQuery (NonnegRange, allRange, rangeGeq, rangeLimit, @@ -333,7 +333,7 @@ pTreePath = do -- unexpected 'x' -- expecting digit, "->", "::", ".", "," or end of input pFieldForest :: Parser [Tree SelectItem] -pFieldForest = pFieldTree `sepBy1` lexeme (char ',') +pFieldForest = pFieldTree `sepBy` lexeme (char ',') where pFieldTree :: Parser (Tree SelectItem) pFieldTree = try (Node <$> pSpreadRelationSelect <*> between (char '(') (char ')') pFieldForest) <|> diff --git a/src/PostgREST/Query/QueryBuilder.hs b/src/PostgREST/Query/QueryBuilder.hs index fa8d624a97..93b04c8021 100644 --- a/src/PostgREST/Query/QueryBuilder.hs +++ b/src/PostgREST/Query/QueryBuilder.hs @@ -57,7 +57,7 @@ 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, relIsSpread} _) (selects,joins) = +getSelectsJoins rr@(Node ReadPlan{select, relName, relToParent=Just rel, relAggAlias, relAlias, relJoinType, relIsSpread} forest) (selects,joins) = let subquery = readPlanToQuery rr aliasOrName = pgFmtIdent $ fromMaybe relName relAlias @@ -77,7 +77,7 @@ getSelectsJoins rr@(Node ReadPlan{relName, relToParent=Just rel, relAggAlias, re "FROM (" <> subquery <> " ) AS " <> SQL.sql aggAlias ) aggAlias $ if relJoinType == Just JTInner then SQL.sql aggAlias <> " IS NOT NULL" else "TRUE") in - (sel:selects, joi:joins) + (if null select && null forest then selects else sel:selects, joi:joins) mutatePlanToQuery :: MutatePlan -> SQL.Snippet mutatePlanToQuery (Insert mainQi iCols body onConflct putConditions returnings _) = diff --git a/test/spec/Feature/Query/QuerySpec.hs b/test/spec/Feature/Query/QuerySpec.hs index c076f11eef..d5cab0961f 100644 --- a/test/spec/Feature/Query/QuerySpec.hs +++ b/test/spec/Feature/Query/QuerySpec.hs @@ -1224,3 +1224,35 @@ spec actualPgVersion = do liftIO $ do let respHeaders = simpleHeaders r respHeaders `shouldSatisfy` noProfileHeader + + context "empty embed" $ do + it "works on a many-to-one relationship" $ do + get "/projects?select=id,name,clients()" `shouldRespondWith` + [json| [ + {"id":1,"name":"Windows 7"}, + {"id":2,"name":"Windows 10"}, + {"id":3,"name":"IOS"}, + {"id":4,"name":"OSX"}, + {"id":5,"name":"Orphan"}]|] + { matchHeaders = [matchContentTypeJson] } + get "/projects?select=id,name,clients!inner()&clients.id=eq.2" `shouldRespondWith` + [json|[ + {"id":3,"name":"IOS"}, + {"id":4,"name":"OSX"}]|] + { matchHeaders = [matchContentTypeJson] } + + it "works on a one-to-many relationship" $ do + get "/clients?select=id,name,projects()" `shouldRespondWith` + [json| [{"id":1,"name":"Microsoft"}, {"id":2,"name":"Apple"}]|] + { matchHeaders = [matchContentTypeJson] } + get "/clients?select=id,name,projects!inner()&projects.name=eq.IOS" `shouldRespondWith` + [json|[{"id":2,"name":"Apple"}]|] + { matchHeaders = [matchContentTypeJson] } + + it "works on a many-to-many relationship" $ do + get "/users?select=*,tasks!inner()" `shouldRespondWith` + [json| [{"id":1,"name":"Angela Martin"}, {"id":2,"name":"Michael Scott"}, {"id":3,"name":"Dwight Schrute"}]|] + { matchHeaders = [matchContentTypeJson] } + get "/users?select=*,tasks!inner()&tasks.id=eq.3" `shouldRespondWith` + [json|[{"id":1,"name":"Angela Martin"}]|] + { matchHeaders = [matchContentTypeJson] } From 6e200812709f97ee4cf2fb522770d2c6a837c683 Mon Sep 17 00:00:00 2001 From: steve-chavez Date: Thu, 24 Nov 2022 23:45:29 -0500 Subject: [PATCH 2/3] fix parsers --- src/PostgREST/ApiRequest/QueryParams.hs | 32 ++++++++++--------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/src/PostgREST/ApiRequest/QueryParams.hs b/src/PostgREST/ApiRequest/QueryParams.hs index 087fe2c825..a38c287b6d 100644 --- a/src/PostgREST/ApiRequest/QueryParams.hs +++ b/src/PostgREST/ApiRequest/QueryParams.hs @@ -326,7 +326,7 @@ pTreePath = do -- >>> P.parse pFieldForest "" "id,clients(name[])" -- Left (line 1, column 16): -- unexpected '[' --- expecting letter, digit, "-", "!", "(", "->>", "->", "::", ")", "," or end of input +-- expecting letter, digit, "-", "->>", "->", "::", ")", "," or end of input -- -- >>> P.parse pFieldForest "" "data->>-78xy" -- Left (line 1, column 11): @@ -335,9 +335,8 @@ pTreePath = do pFieldForest :: Parser [Tree SelectItem] pFieldForest = pFieldTree `sepBy` lexeme (char ',') where - pFieldTree :: Parser (Tree SelectItem) - pFieldTree = try (Node <$> pSpreadRelationSelect <*> between (char '(') (char ')') pFieldForest) <|> - try (Node <$> pRelationSelect <*> between (char '(') (char ')') pFieldForest) <|> + pFieldTree = Node <$> try pSpreadRelationSelect <*> between (char '(') (char ')') pFieldForest <|> + Node <$> try pRelationSelect <*> between (char '(') (char ')') pFieldForest <|> Node <$> pFieldSelect <*> pure [] pStar :: Parser Text @@ -480,13 +479,12 @@ aliasSeparator = char ':' >> notFollowedBy (char ':') -- Left (line 1, column 6): -- unexpected '>' pRelationSelect :: Parser SelectItem -pRelationSelect = lexeme $ try ( do +pRelationSelect = lexeme $ do alias <- optionMaybe ( try(pFieldName <* aliasSeparator) ) name <- pFieldName (hint, jType) <- pEmbedParams try (void $ lookAhead (string "(")) return $ SelectRelation name alias hint jType - ) -- | -- Parse regular fields in select @@ -524,19 +522,16 @@ pRelationSelect = lexeme $ try ( do -- unexpected end of input -- expecting letter or digit pFieldSelect :: Parser SelectItem -pFieldSelect = lexeme $ - try ( - do - alias <- optionMaybe ( try(pFieldName <* aliasSeparator) ) - fld <- pField - cast' <- optionMaybe (string "::" *> pIdentifier) - pEnd - return $ SelectField fld (toS <$> cast') alias - ) - <|> do +pFieldSelect = lexeme $ try (do s <- pStar pEnd - return $ SelectField (s, []) Nothing Nothing + return $ SelectField (s, []) Nothing Nothing) + <|> do + alias <- optionMaybe ( try(pFieldName <* aliasSeparator) ) + fld <- pField + cast' <- optionMaybe (string "::" *> pIdentifier) + pEnd + return $ SelectField fld (toS <$> cast') alias where pEnd = try (void $ lookAhead (string ")")) <|> try (void $ lookAhead (string ",")) <|> @@ -565,12 +560,11 @@ pFieldSelect = lexeme $ -- Left (line 1, column 8): -- unexpected '>' pSpreadRelationSelect :: Parser SelectItem -pSpreadRelationSelect = lexeme $ try ( do +pSpreadRelationSelect = lexeme $ do name <- string ".." >> pFieldName (hint, jType) <- pEmbedParams try (void $ lookAhead (string "(")) return $ SpreadRelation name hint jType - ) pEmbedParams :: Parser (Maybe Hint, Maybe JoinType) pEmbedParams = do From d9d50f160b67d37bc998d6f22d13adfd4d19e0ab Mon Sep 17 00:00:00 2001 From: steve-chavez Date: Fri, 25 Nov 2022 14:02:03 -0500 Subject: [PATCH 3/3] fix empty select on root --- src/PostgREST/ApiRequest/QueryParams.hs | 8 +++++--- src/PostgREST/Query.hs | 6 +++--- src/PostgREST/Query/QueryBuilder.hs | 9 +++++---- test/spec/Feature/Query/QuerySpec.hs | 19 +++++++++++++++++++ 4 files changed, 32 insertions(+), 10 deletions(-) diff --git a/src/PostgREST/ApiRequest/QueryParams.hs b/src/PostgREST/ApiRequest/QueryParams.hs index a38c287b6d..9bc0a299e4 100644 --- a/src/PostgREST/ApiRequest/QueryParams.hs +++ b/src/PostgREST/ApiRequest/QueryParams.hs @@ -323,6 +323,9 @@ pTreePath = do -- >>> 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 "" "" +-- Right [] +-- -- >>> P.parse pFieldForest "" "id,clients(name[])" -- Left (line 1, column 16): -- unexpected '[' @@ -339,9 +342,6 @@ pFieldForest = pFieldTree `sepBy` lexeme (char ',') Node <$> try pRelationSelect <*> between (char '(') (char ')') pFieldForest <|> Node <$> pFieldSelect <*> pure [] -pStar :: Parser Text -pStar = string "*" $> "*" - -- | -- Parse field names -- @@ -536,6 +536,8 @@ pFieldSelect = lexeme $ try (do pEnd = try (void $ lookAhead (string ")")) <|> try (void $ lookAhead (string ",")) <|> try eof + pStar = string "*" $> "*" + -- | -- Parse spread relations in select diff --git a/src/PostgREST/Query.hs b/src/PostgREST/Query.hs index 022a3af3d7..edb98f6364 100644 --- a/src/PostgREST/Query.hs +++ b/src/PostgREST/Query.hs @@ -76,7 +76,7 @@ readQuery req conf@AppConfig{..} apiReq@ApiRequest{..} = do resultSet <- lift . SQL.statement mempty $ Statements.prepareRead - (QueryBuilder.readPlanToQuery req) + (QueryBuilder.readPlanToQuery True req) (if iPreferCount == Just EstimatedCount then -- LIMIT maxRows + 1 so we can determine below that maxRows was surpassed QueryBuilder.limitedQuery countQuery ((+ 1) <$> configDbMaxRows) @@ -163,7 +163,7 @@ invokeQuery proc CallReadPlan{crReadPlan, crCallPlan} apiReq@ApiRequest{..} conf (Proc.procReturnsScalar proc) (Proc.procReturnsSingle proc) (QueryBuilder.callPlanToQuery crCallPlan) - (QueryBuilder.readPlanToQuery crReadPlan) + (QueryBuilder.readPlanToQuery True crReadPlan) (QueryBuilder.readPlanToCountQuery crReadPlan) (shouldCount iPreferCount) iAcceptMediaType @@ -217,7 +217,7 @@ writeQuery MutateReadPlan{mrReadPlan, mrMutatePlan} apiReq conf = in lift . SQL.statement mempty $ Statements.prepareWrite - (QueryBuilder.readPlanToQuery mrReadPlan) + (QueryBuilder.readPlanToQuery True mrReadPlan) (QueryBuilder.mutatePlanToQuery mrMutatePlan) isInsert (iAcceptMediaType apiReq) diff --git a/src/PostgREST/Query/QueryBuilder.hs b/src/PostgREST/Query/QueryBuilder.hs index 93b04c8021..4fb689de66 100644 --- a/src/PostgREST/Query/QueryBuilder.hs +++ b/src/PostgREST/Query/QueryBuilder.hs @@ -39,10 +39,10 @@ import PostgREST.RangeQuery (allRange) import Protolude -readPlanToQuery :: ReadPlanTree -> SQL.Snippet -readPlanToQuery (Node ReadPlan{select,from=mainQi,fromAlias,where_=logicForest,order, range_=readRange, relToParent, relJoinConds} forest) = +readPlanToQuery :: Bool -> ReadPlanTree -> SQL.Snippet +readPlanToQuery isRoot (Node ReadPlan{select,from=mainQi,fromAlias,where_=logicForest,order, range_=readRange, relToParent, relJoinConds} forest) = "SELECT " <> - intercalateSnippet ", " ((pgFmtSelectItem qi <$> select) ++ selects) <> " " <> + intercalateSnippet ", " ((pgFmtSelectItem qi <$> (if isRoot && null select && null forest then defRootSelect else select)) ++ selects) <> " " <> fromFrag <> " " <> intercalateSnippet " " joins <> " " <> (if null logicForest && null relJoinConds @@ -53,13 +53,14 @@ readPlanToQuery (Node ReadPlan{select,from=mainQi,fromAlias,where_=logicForest,o where fromFrag = fromF relToParent mainQi fromAlias qi = getQualifiedIdentifier relToParent mainQi fromAlias + defRootSelect = [(("*", []), Nothing, Nothing)] -- gets all columns in case an empty select, e.g. `/tbl?select=`, is done. (selects, joins) = foldr getSelectsJoins ([],[]) forest getSelectsJoins :: ReadPlanTree -> ([SQL.Snippet], [SQL.Snippet]) -> ([SQL.Snippet], [SQL.Snippet]) getSelectsJoins (Node ReadPlan{relToParent=Nothing} _) _ = ([], []) getSelectsJoins rr@(Node ReadPlan{select, relName, relToParent=Just rel, relAggAlias, relAlias, relJoinType, relIsSpread} forest) (selects,joins) = let - subquery = readPlanToQuery rr + subquery = readPlanToQuery False rr aliasOrName = pgFmtIdent $ fromMaybe relName relAlias aggAlias = pgFmtIdent relAggAlias correlatedSubquery sub al cond = diff --git a/test/spec/Feature/Query/QuerySpec.hs b/test/spec/Feature/Query/QuerySpec.hs index d5cab0961f..eedf0d703b 100644 --- a/test/spec/Feature/Query/QuerySpec.hs +++ b/test/spec/Feature/Query/QuerySpec.hs @@ -1256,3 +1256,22 @@ spec actualPgVersion = do get "/users?select=*,tasks!inner()&tasks.id=eq.3" `shouldRespondWith` [json|[{"id":1,"name":"Angela Martin"}]|] { matchHeaders = [matchContentTypeJson] } + + context "empty root select" $ + it "gives all columns" $ do + get "/projects?select=" `shouldRespondWith` + [json|[ + {"id":1,"name":"Windows 7","client_id":1}, + {"id":2,"name":"Windows 10","client_id":1}, + {"id":3,"name":"IOS","client_id":2}, + {"id":4,"name":"OSX","client_id":2}, + {"id":5,"name":"Orphan","client_id":null}]|] + { matchHeaders = [matchContentTypeJson] } + get "/rpc/getallprojects?select=" `shouldRespondWith` + [json|[ + {"id":1,"name":"Windows 7","client_id":1}, + {"id":2,"name":"Windows 10","client_id":1}, + {"id":3,"name":"IOS","client_id":2}, + {"id":4,"name":"OSX","client_id":2}, + {"id":5,"name":"Orphan","client_id":null}]|] + { matchHeaders = [matchContentTypeJson] }