Fix include entities from the same parent table using two different foreign keys

CHANGELOG.md

@@ -14,6 +14,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 - Omit Content-Type header for empty body - @begriffs
 - Prevent role from being changed twice - @begriffs
 - Use read-only transaction for read requests - @ruslantalpa
+- Include entities from the same parent table using two different foreign keys - @ruslantalpa
 
 ## [0.3.1.1] - 2016-03-28
 

src/PostgREST/QueryBuilder.hs

@@ -36,17 +36,17 @@ import qualified Data.Aeson              as JSON
 import           Data.Int                (Int64)
 
 import           PostgREST.RangeQuery    (NonnegRange, rangeLimit, rangeOffset)
-import           Control.Error           (note, fromMaybe, mapMaybe)
+import           Control.Error           (note, fromMaybe)
 import           Data.Functor.Contravariant (contramap)
 import qualified Data.HashMap.Strict     as HM
-import           Data.List               (find, (\\))
+import           Data.List               (find)
 import           Data.Monoid             ((<>))
 import           Data.Text               (Text, intercalate, unwords, replace, isInfixOf, toLower, split)
 import qualified Data.Text as T          (map, takeWhile)
 import qualified Data.Text.Encoding as T
 import           Data.String.Conversions (cs)
 import           Control.Applicative     ((<|>))
-import           Control.Monad           (join)
+-- import           Control.Monad           (join)
 import           Data.Tree               (Tree(..))
 import qualified Data.Vector as V
 import           PostgREST.Types
@@ -182,25 +182,18 @@ addRelations schema allRelations parentNode node@(Node readNode@(query, (name, _
       where n `colMatches` rc = (cs ("^" <> rc <> "_?(?:|[iI][dD]|[fF][kK])$") :: BS.ByteString) =~ (cs n :: BS.ByteString)
 
 addJoinConditions :: Schema -> ReadRequest -> Either Text ReadRequest
-addJoinConditions schema (Node (query, (n, r, a)) forest) =
+addJoinConditions schema (Node nn@(query, (n, r, a)) forest) =
   case r of
-    Nothing -> Node (updatedQuery, (n,r,a))  <$> updatedForest -- this is the root node
+    Nothing -> Node nn  <$> updatedForest -- this is the root node
-    Just rel@Relation{relType=Child} -> Node (addCond updatedQuery (getJoinConditions rel),(n,r,a)) <$> updatedForest
+    Just rel@Relation{relType=Child} -> Node (addCond query (getJoinConditions rel),(n,r,a)) <$> updatedForest
-    Just Relation{relType=Parent} -> Node (updatedQuery, (n,r,a)) <$> updatedForest
+    Just Relation{relType=Parent} -> Node nn <$> updatedForest
     Just rel@Relation{relType=Many, relLTable=(Just linkTable)} ->
       Node (qq, (n, r, a)) <$> updatedForest
       where
-         query' = addCond updatedQuery (getJoinConditions rel)
+         query' = addCond query (getJoinConditions rel)
          qq = query'{from=tableName linkTable : from query'}
     _ -> Left "unknown relation"
   where
-    -- add parentTable and parentJoinConditions to the query
-    updatedQuery = foldr (flip addCond) query parentJoinConditions
-      where
-        parentJoinConditions = map (getJoinConditions . snd) parents
-        parents = mapMaybe (getParents . rootLabel) forest
-        getParents (_, (tbl, Just rel@Relation{relType=Parent}, _)) = Just (tbl, rel)
-        getParents _ = Nothing
     updatedForest = mapM (addJoinConditions schema) forest
     addCond query' con = query'{flt_=con ++ flt_ query'}
 
@@ -288,8 +281,8 @@ requestToQuery schema (DbRead (Node (Select colSelects tbls conditions ord, (nod
     query = unwords [
       "SELECT ", intercalate ", " (map (pgFmtSelectItem qi) colSelects ++ selects),
       "FROM ", intercalate ", " (map (fromQi . toQi) tbls),
-      unwords (map joinStr joins),
+      unwords joins,
-      ("WHERE " <> intercalate " AND " ( map (pgFmtCondition qi ) localConditions )) `emptyOnNull` localConditions,
+      ("WHERE " <> intercalate " AND " ( map (pgFmtCondition qi ) conditions )) `emptyOnNull` conditions,
       orderF (fromMaybe [] ord)
       ]
     orderF ts =
@@ -304,17 +297,8 @@ requestToQuery schema (DbRead (Node (Select colSelects tbls conditions ord, (nod
                 <> (cs.show) (otDirection t) <> " "
                 <> maybe "" (cs.show) (otNullOrder t) <> " "
     (joins, selects) = foldr getQueryParts ([],[]) forest
-    parentTables = map snd joins
+
-    parentConditions = join $ map (( `filter` conditions ) . filterParentConditions) parentTables
+    getQueryParts :: Tree ReadNode -> ([SqlFragment], [SqlFragment]) -> ([SqlFragment], [SqlFragment])
-    localConditions = conditions \\ parentConditions
-    joinStr :: (SqlFragment, TableName) -> SqlFragment
-    joinStr (sql, t) = "LEFT OUTER JOIN " <> sql <> " ON " <>
-      intercalate " AND " ( map (pgFmtCondition qi ) joinConditions )
-      where
-        joinConditions = filter (filterParentConditions t) conditions
-    filterParentConditions parentTable (Filter _ _ (VForeignKey (QualifiedIdentifier "" t) _)) = parentTable == t
-    filterParentConditions _ _ = False
-    getQueryParts :: Tree ReadNode -> ([(SqlFragment, TableName)], [SqlFragment]) -> ([(SqlFragment,TableName)], [SqlFragment])
     getQueryParts (Node n@(_, (name, Just Relation{relType=Child,relTable=Table{tableName=table}}, alias)) forst) (j,s) = (j,sel:s)
       where
         sel = "COALESCE(("
@@ -322,10 +306,15 @@ requestToQuery schema (DbRead (Node (Select colSelects tbls conditions ord, (nod
            <> "FROM (" <> subquery <> ") " <> pgFmtIdent table
            <> "), '[]') AS " <> pgFmtIdent (fromMaybe name alias)
            where subquery = requestToQuery schema (DbRead (Node n forst))
-    getQueryParts (Node n@(_, (name, Just Relation{relType=Parent,relTable=Table{tableName=table}}, alias)) forst) (j,s) = (joi:j,sel:s)
+    getQueryParts (Node n@(_, (name, Just r@Relation{relType=Parent,relTable=Table{tableName=table}}, alias)) forst) (j,s) = (joi:j,sel:s)
       where
-        sel = "row_to_json(" <> pgFmtIdent table <> ".*) AS " <> pgFmtIdent (fromMaybe name alias)
+        node_name = fromMaybe name alias
-        joi = ("( " <> subquery <> " ) AS " <> pgFmtIdent table, table)
+        local_table_name = table <> "_" <> node_name
+        replaceTableName localTableName (Filter a b (VForeignKey (QualifiedIdentifier "" _) c)) = Filter a b (VForeignKey (QualifiedIdentifier "" localTableName) c)
+        replaceTableName _ x = x
+        sel = "row_to_json(" <> pgFmtIdent local_table_name <> ".*) AS " <> pgFmtIdent node_name
+        joi = " LEFT OUTER JOIN ( " <> subquery <> " ) AS " <> pgFmtIdent local_table_name  <>
+              " ON " <> intercalate " AND " ( map (pgFmtCondition qi . replaceTableName local_table_name) (getJoinConditions r) )
           where subquery = requestToQuery schema (DbRead (Node n forst))
     getQueryParts (Node n@(_, (name, Just Relation{relType=Many,relTable=Table{tableName=table}}, alias)) forst) (j,s) = (j,sel:s)
       where
@@ -361,7 +350,6 @@ requestToQuery schema (DbMutate (Update mainTbl (PayloadJSON (UniformObjects row
     Nothing -> undefined
   where
     qi = QualifiedIdentifier schema mainTbl
-
 requestToQuery schema (DbMutate (Delete mainTbl conditions)) =
   query
   where

test/Feature/QuerySpec.hs

@@ -216,6 +216,11 @@ spec = do
       get "/projects?id=eq.1&select=id, name, clients{*}, tasks{id, name}" `shouldRespondWith`
         [str|[{"id":1,"name":"Windows 7","clients":{"id":1,"name":"Microsoft"},"tasks":[{"id":1,"name":"Design w7"},{"id":2,"name":"Code w7"}]}]|]
 
+    it "embed data with two fk pointing to the same table" $
+      get "/orders?id=eq.1&select=id, name, billing_address_id{id}, shipping_address_id{id}" `shouldRespondWith`
+        [str|[{"id":1,"name":"order 1","billing_address_id":{"id":1},"shipping_address_id":{"id":2}}]|]
+
+
     it "requesting parents and children while renaming them" $
       get "/projects?id=eq.1&select=myId:id, name, project_client:client_id{*}, project_tasks:tasks{id, name}" `shouldRespondWith`
         [str|[{"myId":1,"name":"Windows 7","project_client":{"id":1,"name":"Microsoft"},"project_tasks":[{"id":1,"name":"Design w7"},{"id":2,"name":"Code w7"}]}]|]

test/Feature/StructureSpec.hs

@@ -17,6 +17,7 @@ spec = do
       request methodGet "/" [] ""
         `shouldRespondWith` [json| [
           {"schema":"test","name":"Escap3e;","insertable":true}
+        , {"schema":"test","name":"addresses","insertable":true}
         , {"schema":"test","name":"articleStars","insertable":true}
         , {"schema":"test","name":"articles","insertable":true}
         , {"schema":"test","name":"auto_incrementing_pk","insertable":true}
@@ -36,6 +37,7 @@ spec = do
         , {"schema":"test","name":"menagerie","insertable":true}
         , {"schema":"test","name":"no_pk","insertable":true}
         , {"schema":"test","name":"nullable_integer","insertable":true}
+        , {"schema":"test","name":"orders","insertable":true}
         , {"schema":"test","name":"projects","insertable":true}
         , {"schema":"test","name":"projects_view","insertable":true}
         , {"schema":"test","name":"simple_pk","insertable":true}

test/fixtures/data.sql

@@ -267,7 +267,20 @@ TRUNCATE TABLE "ghostBusters" CASCADE;
 INSERT INTO "ghostBusters" VALUES (1), (3), (5);
 
 TRUNCATE TABLE "withUnique" CASCADE;
-INSERT INTO "withUnique" VALUES ('nodup', 'blah')
+INSERT INTO "withUnique" VALUES ('nodup', 'blah');
+
+
+
+TRUNCATE TABLE addresses CASCADE;
+INSERT INTO addresses VALUES (1, 'address 1');
+INSERT INTO addresses VALUES (2, 'address 2');
+INSERT INTO addresses VALUES (3, 'address 3');
+INSERT INTO addresses VALUES (4, 'address 4');
+
+TRUNCATE TABLE orders CASCADE;
+INSERT INTO orders VALUES (1, 'order 1', 1, 2);
+INSERT INTO orders VALUES (2, 'order 2', 3, 4);
+
 --
 -- PostgreSQL database dump complete
 --

test/fixtures/privileges.sql

@@ -38,6 +38,8 @@ GRANT ALL ON TABLE
     , "ghostBusters"
     , "withUnique"
     , "موارد"
+    , addresses
+    , orders
 TO postgrest_test_anonymous;
 
 GRANT INSERT ON TABLE insertonly TO postgrest_test_anonymous;

test/fixtures/schema.sql

@@ -1001,6 +1001,19 @@ ALTER TABLE ONLY users_tasks
 ALTER TABLE ONLY users_tasks
     ADD CONSTRAINT users_tasks_user_id_fkey FOREIGN KEY (user_id) REFERENCES users(id);
 
+
+create table addresses (
+	id                   int not null unique,
+	address              text not null
+);
+
+create table orders (
+	id                   int not null unique,
+	name                 text not null,
+	billing_address_id   int references addresses(id),
+	shipping_address_id  int references addresses(id)
+);
+
 --
 -- PostgreSQL database dump complete
 --