diff --git a/CHANGELOG.md b/CHANGELOG.md index a01cd7954..868668074 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ This project adheres to [Semantic Versioning](http://semver.org/). ## Unreleased +### 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 14e1cb87e..e5cf3eaca 100644 --- a/src/PostgREST/Error.hs +++ b/src/PostgREST/Error.hs @@ -193,7 +193,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 851c707f3..b94764fe1 100644 --- a/src/PostgREST/Plan.hs +++ b/src/PostgREST/Plan.hs @@ -357,7 +357,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 @@ -371,7 +371,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 @@ -382,14 +382,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 @@ -403,8 +404,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 95566576c..bdfe28fb0 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 @@ -45,8 +43,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 615eda2c9..2a47a0e1d 100644 --- a/src/PostgREST/SchemaCache/Relationship.hs +++ b/src/PostgREST/SchemaCache/Relationship.hs @@ -55,8 +55,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 a4593b951..2c9882d84 100644 --- a/test/spec/fixtures/schema.sql +++ b/test/spec/fixtures/schema.sql @@ -2996,3 +2996,24 @@ CREATE TABLE public.tb ( CREATE VIEW test.va AS SELECT a1 FROM public.ta; CREATE VIEW test.vb AS SELECT b1 FROM public.tb; + +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; +$$;