From cb99270a8f14cfe3838408a5a3f9f9205e740323 Mon Sep 17 00:00:00 2001 From: steve-chavez Date: Fri, 18 Nov 2022 15:57:34 -0500 Subject: [PATCH] fix: bad M2M embed on RPC --- CHANGELOG.md | 2 ++ src/PostgREST/Error.hs | 2 +- src/PostgREST/Plan.hs | 15 ++++++++------- src/PostgREST/Plan/ReadPlan.hs | 7 ------- src/PostgREST/SchemaCache/Relationship.hs | 4 ++-- test/spec/Feature/Query/RpcSpec.hs | 6 +++++- test/spec/fixtures/schema.sql | 21 +++++++++++++++++++++ 7 files changed, 39 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 357fcc9ea..c3667ae6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### Fixed + - #2565, Fix bad M2M embedding on RPC - @steve-chavez + ## [10.1.1] - 2022-11-08 ### Fixed diff --git a/src/PostgREST/Error.hs b/src/PostgREST/Error.hs index 07accc74b..fa75a3bbc 100644 --- a/src/PostgREST/Error.hs +++ b/src/PostgREST/Error.hs @@ -209,7 +209,7 @@ compressedRel Relationship{..} = : case relCardinality of M2M Junction{..} -> [ "cardinality" .= ("many-to-many" :: Text) - , "relationship" .= (qiName junTable <> " using " <> junConstraint1 <> fmtEls (snd <$> junColumns1) <> " and " <> junConstraint2 <> fmtEls (snd <$> junColumns2)) + , "relationship" .= (qiName junTable <> " using " <> junConstraint1 <> fmtEls (snd <$> junColsSource) <> " and " <> junConstraint2 <> fmtEls (snd <$> junColsTarget)) ] M2O cons relColumns -> [ "cardinality" .= ("many-to-one" :: Text) diff --git a/src/PostgREST/Plan.hs b/src/PostgREST/Plan.hs index 54c69f103..86e61bbf9 100644 --- a/src/PostgREST/Plan.hs +++ b/src/PostgREST/Plan.hs @@ -399,7 +399,7 @@ mutatePlan mutation qi ApiRequest{..} sCache readReq = mapLeft ApiRequestError $ returnings = if iPreferRepresentation == None then [] - else returningCols readReq pkCols + else inferColsEmbedNeeds readReq pkCols pkCols = maybe mempty tablePKCols $ HM.lookup qi $ dbTables sCache logic = map snd qsLogic rootOrder = maybe [] snd $ find (\(x, _) -> null x) qsOrder @@ -413,7 +413,7 @@ callPlan proc apiReq readReq = FunctionCall { , funCArgs = payRaw <$> iPayload apiReq , funCScalar = procReturnsScalar proc , funCMultipleCall = iPreferParameters apiReq == Just MultipleObjects -, funCReturning = returningCols readReq [] +, funCReturning = inferColsEmbedNeeds readReq [] } where paramsAsSingleObject = iPreferParameters apiReq == Just SingleObject @@ -424,14 +424,15 @@ callPlan proc apiReq readReq = FunctionCall { prms -> KeyParams $ specifiedParams prms specifiedParams = filter (\x -> ppName x `S.member` iColumns apiReq) -returningCols :: ReadPlanTree -> [FieldName] -> [FieldName] -returningCols rr@(Node _ forest) pkCols +-- | Infers the columns needed for an embed to be successful after a mutation or a function call. +inferColsEmbedNeeds :: ReadPlanTree -> [FieldName] -> [FieldName] +inferColsEmbedNeeds (Node ReadPlan{select} forest) pkCols -- if * is part of the select, we must not add pk or fk columns manually - -- otherwise those would be selected and output twice | "*" `elem` fldNames = ["*"] | otherwise = returnings where - fldNames = fstFieldNames rr + fldNames = (\((fld, _), _, _) -> fld) <$> select -- Without fkCols, when a mutatePlan to -- /projects?select=name,clients(name) occurs, the RETURNING SQL part would -- be `RETURNING name`(see QueryBuilder). This would make the embedding @@ -445,8 +446,8 @@ returningCols rr@(Node _ forest) pkCols Just $ fst <$> cols Node ReadPlan{relToParent=Just Relationship{relCardinality=O2O _ cols}} _ -> Just $ fst <$> cols - Node ReadPlan{relToParent=Just Relationship{relCardinality=M2M Junction{junColumns1, junColumns2}}} _ -> - Just $ (fst <$> junColumns1) ++ (fst <$> junColumns2) + Node ReadPlan{relToParent=Just Relationship{relCardinality=M2M Junction{junColsSource=cols}}} _ -> + Just $ fst <$> cols Node ReadPlan{relToParent=Just ComputedRelationship{}} _ -> Nothing Node ReadPlan{relToParent=Nothing} _ -> diff --git a/src/PostgREST/Plan/ReadPlan.hs b/src/PostgREST/Plan/ReadPlan.hs index e51170cdf..94e181ca2 100644 --- a/src/PostgREST/Plan/ReadPlan.hs +++ b/src/PostgREST/Plan/ReadPlan.hs @@ -1,8 +1,6 @@ -{-# LANGUAGE NamedFieldPuns #-} module PostgREST.Plan.ReadPlan ( ReadPlanTree , ReadPlan(..) - , fstFieldNames , JoinCondition(..) ) where @@ -46,8 +44,3 @@ data ReadPlan = ReadPlan -- ^ used for aliasing } deriving (Eq) - --- First level FieldNames(e.g get a,b from /table?select=a,b,other(c,d)) -fstFieldNames :: ReadPlanTree -> [FieldName] -fstFieldNames (Node ReadPlan{select} _) = - fst . (\(f, _, _) -> f) <$> select diff --git a/src/PostgREST/SchemaCache/Relationship.hs b/src/PostgREST/SchemaCache/Relationship.hs index e8b18a642..db912faac 100644 --- a/src/PostgREST/SchemaCache/Relationship.hs +++ b/src/PostgREST/SchemaCache/Relationship.hs @@ -56,8 +56,8 @@ data Junction = Junction { junTable :: QualifiedIdentifier , junConstraint1 :: FKConstraint , junConstraint2 :: FKConstraint - , junColumns1 :: [(FieldName, FieldName)] - , junColumns2 :: [(FieldName, FieldName)] + , junColsSource :: [(FieldName, FieldName)] + , junColsTarget :: [(FieldName, FieldName)] } deriving (Eq, Ord, Generic, JSON.ToJSON) diff --git a/test/spec/Feature/Query/RpcSpec.hs b/test/spec/Feature/Query/RpcSpec.hs index 66cb1b98a..fcd90f442 100644 --- a/test/spec/Feature/Query/RpcSpec.hs +++ b/test/spec/Feature/Query/RpcSpec.hs @@ -246,13 +246,17 @@ spec actualPgVersion = `shouldRespondWith` [json|{"id": 2, "articleStars": [{"userId": 3}]}|] - it "can embed an M2M relationship table" $ + it "can embed an M2M relationship table" $ do get "/rpc/getallusers?select=name,tasks(name)&id=gt.1" `shouldRespondWith` [json|[ {"name":"Michael Scott", "tasks":[{"name":"Design IOS"}, {"name":"Code IOS"}, {"name":"Design OSX"}]}, {"name":"Dwight Schrute","tasks":[{"name":"Design w7"}, {"name":"Design IOS"}]} ]|] { matchHeaders = [matchContentTypeJson] } + -- https://github.com/PostgREST/postgrest/issues/2565 + get "/rpc/get_yards?select=groups(*)" + `shouldRespondWith` [json|[]|] + { matchHeaders = [matchContentTypeJson] } it "can embed an M2M relationship table that has a parent relationship table" $ get "/rpc/getallusers?select=name,tasks(name,project:projects(name))&id=gt.1" diff --git a/test/spec/fixtures/schema.sql b/test/spec/fixtures/schema.sql index 8fec1f12a..ab4489929 100644 --- a/test/spec/fixtures/schema.sql +++ b/test/spec/fixtures/schema.sql @@ -3005,3 +3005,24 @@ create table test.trash_details( id int primary key references test.trash(id), jsonb_col jsonb ); + +CREATE TABLE test.groups ( + name text PRIMARY KEY +); + +CREATE TABLE test.yards ( + id bigint PRIMARY KEY +); + +CREATE TABLE test.group_yard ( + id bigint NOT NULL, + group_id text NOT NULL REFERENCES test.groups(name), + yard_id bigint NOT NULL REFERENCES test.yards(id), + PRIMARY KEY (id, group_id, yard_id) +); + +CREATE FUNCTION test.get_yards() RETURNS SETOF test.yards +LANGUAGE sql +AS $$ + select * from test.yards; +$$;