From d71d3450af1473ac893c2a2cfb38ad8d83345edd Mon Sep 17 00:00:00 2001
From: steve-chavez <stevechavezast@gmail.com>
Date: Tue, 24 Sep 2019 16:18:48 -0500
Subject: [PATCH] Fix PATCH embed when not having the id in ?select

---
 src/PostgREST/DbRequestBuilder.hs | 8 +++++++-
 test/Feature/InsertSpec.hs        | 3 +--
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/PostgREST/DbRequestBuilder.hs b/src/PostgREST/DbRequestBuilder.hs
index 3895e8ce4..ee82563a5 100644
--- a/src/PostgREST/DbRequestBuilder.hs
+++ b/src/PostgREST/DbRequestBuilder.hs
@@ -329,16 +329,22 @@ mutateRequest apiRequest tName cols pkCols readReq = mapLeft errorResponseFor $
     onlyRoot = filter (not . ( "." `isInfixOf` ) . fst)
 
 returningCols :: ReadRequest -> [FieldName]
-returningCols rr@(Node _ forest) = fstFieldNames rr ++ (colName <$> fkCols)
+returningCols rr@(Node _ forest) = returnings
   where
+    fldNames = fstFieldNames rr
     -- Without fkCols, when a mutateRequest to /projects?select=name,clients(name) occurs, the RETURNING SQL part would be
     -- `RETURNING name`(see QueryBuilder).
     -- This would make the embedding fail because the following JOIN would need the "client_id" column from projects.
     -- So this adds the foreign key columns to ensure the embedding succeeds, result would be `RETURNING name, client_id`.
     fkCols = concat $ mapMaybe (\case
         Node (_, (_, Just Relation{relFColumns=cols, relType=Parent}, _, _, _)) _ -> Just cols
+        Node (_, (_, Just Relation{relFColumns=cols, relType=Child}, _, _, _)) _  -> Just cols
         _ -> Nothing
       ) forest
+    -- However if the "client_id" is present, e.g. mutateRequest to /projects?select=client_id,name,clients(name)
+    -- we would get `RETURNING client_id, name, client_id` and then we would produce the "column reference \"client_id\" is ambiguous"
+    -- error from PostgreSQL. So we deduplicate with Set:
+    returnings = S.toList . S.fromList $ fldNames ++ (colName <$> fkCols)
 
 -- Traditional filters(e.g. id=eq.1) are added as root nodes of the LogicTree
 -- they are later concatenated with AND in the QueryBuilder
diff --git a/test/Feature/InsertSpec.hs b/test/Feature/InsertSpec.hs
index 1bd6cf600..51bad80c9 100644
--- a/test/Feature/InsertSpec.hs
+++ b/test/Feature/InsertSpec.hs
@@ -633,8 +633,7 @@ spec actualPgVersion = do
           matchHeaders = [matchContentTypeJson]
         }
 
-    it "embeds childs after update without explicitly including the id in the ?select" $ do
-      pendingWith "currently failing"
+    it "embeds childs after update without explicitly including the id in the ?select" $
       request methodPatch "/web_content?id=eq.0&select=name,web_content(name)"
               [("Prefer", "return=representation")]
         [json|{"name": "tardis-patched"}|]