Fix #1221, embedding when having a self join

CHANGELOG.md

@@ -10,6 +10,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 ### Fixed
 
 - #1223, Fix incorrect OpenAPI externalDocs url - @steve-chavez
+- #1221, Fix embedding other resources when having a self join - @steve-chavez
 
 ## [5.2.0] - 2018-12-12
 

src/PostgREST/DbRequestBuilder.hs

@@ -76,7 +76,7 @@ readRequest maxRows allRels proc apiRequest  =
     buildReadRequest fieldTree =
       let rootDepth = 0
           rootNodeName = if action == ActionRead then rootTableName else sourceCTEName in
-      foldr (treeEntry rootDepth) (Node (Select [] rootNodeName [] [] [] [] allRange, (rootNodeName, Nothing, Nothing, Nothing, rootDepth)) []) fieldTree
+      foldr (treeEntry rootDepth) (Node (Select [] rootNodeName Nothing [] [] [] [] allRange, (rootNodeName, Nothing, Nothing, Nothing, rootDepth)) []) fieldTree
       where
         treeEntry :: Depth -> Tree SelectItem -> ReadRequest -> ReadRequest
         treeEntry depth (Node fld@((fn, _),_,alias,relationDetail) fldForest) (Node (q, i) rForest) =
@@ -84,7 +84,7 @@ readRequest maxRows allRels proc apiRequest  =
           case fldForest of
             [] -> Node (q {select=fld:select q}, i) rForest
             _  -> Node (q, i) $
-                  foldr (treeEntry nxtDepth) (Node (Select [] fn [] [] [] [] allRange, (fn, Nothing, alias, relationDetail, nxtDepth)) []) fldForest:rForest
+                  foldr (treeEntry nxtDepth) (Node (Select [] fn Nothing [] [] [] [] allRange, (fn, Nothing, alias, relationDetail, nxtDepth)) []) fldForest:rForest
 
     relations :: [Relation]
     relations = case action of
@@ -116,7 +116,7 @@ treeRestrictRange maxRows_ request = pure $ nodeRestrictRange maxRows_ `fmap` re
 augumentRequestWithJoin :: Schema ->  [Relation] ->  ReadRequest -> Either ApiRequestError ReadRequest
 augumentRequestWithJoin schema allRels request =
   addRelations schema allRels Nothing request
-  >>= addJoinConditions schema
+  >>= addJoinConditions schema Nothing
 
 addRelations :: Schema -> [Relation] -> Maybe ReadRequest -> ReadRequest -> Either ApiRequestError ReadRequest
 addRelations schema allRelations parentNode (Node (query@Select{from=tbl}, (nodeName, _, alias, relationDetail, depth)) forest) =
@@ -216,10 +216,11 @@ findRelation schema allRelations nodeTableName parentNodeTableName relationDetai
         )
   ) allRelations
 
-addJoinConditions :: Schema -> ReadRequest -> Either ApiRequestError ReadRequest
+-- previousAlias is only used for the case of self joins
-addJoinConditions schema (Node node@(query, nodeProps@(_, relation, _, _, _)) forest) =
+addJoinConditions :: Schema -> Maybe Alias -> ReadRequest -> Either ApiRequestError ReadRequest
+addJoinConditions schema previousAlias (Node node@(query@Select{from=tbl}, nodeProps@(_, relation, _, _, depth)) forest) =
   case relation of
-    Just Relation{relType=Root} -> Node node  <$> updatedForest -- this is the root node
+    Just Relation{relType=Root} -> Node node <$> updatedForest -- this is the root node
     Just rel@Relation{relType=Parent} -> Node (augmentQuery rel, nodeProps) <$> updatedForest
     Just rel@Relation{relType=Child} -> Node (augmentQuery rel, nodeProps) <$> updatedForest
     Just rel@Relation{relType=Many, relLinkTable=(Just linkTable)} ->
@@ -227,13 +228,21 @@ addJoinConditions schema (Node node@(query, nodeProps@(_, relation, _, _, _)) fo
       Node (rq{implicitJoins=tableName linkTable:implicitJoins rq}, nodeProps) <$> updatedForest
     _ -> Left UnknownRelation
   where
-    updatedForest = mapM (addJoinConditions schema) forest
+    newAlias = case isSelfJoin <$> relation of
-    augmentQuery rel = foldr addJoinCond query (getJoinConditions rel)
+      Just True
-    addJoinCond :: JoinCondition -> ReadQuery -> ReadQuery
+        | depth /= 0 -> Just (tbl <> "_" <> show depth) -- root node doesn't get aliased
-    addJoinCond jc rq@Select{joinConditions=jcs} = rq{joinConditions=jc:jcs}
+        | otherwise  -> Nothing
+      _              -> Nothing
+    augmentQuery rel =
+      foldr
+        (\jc rq@Select{joinConditions=jcs} -> rq{joinConditions=jc:jcs})
+        query{fromAlias=newAlias}
+        (getJoinConditions previousAlias newAlias rel)
+    updatedForest = mapM (addJoinConditions schema newAlias) forest
 
-getJoinConditions :: Relation -> [JoinCondition]
+-- previousAlias and newAlias are used in the case of self joins
-getJoinConditions (Relation Table{tableSchema=tSchema, tableName=tN} cols Table{tableName=ftN} fCols typ lt lc1 lc2) =
+getJoinConditions :: Maybe Alias -> Maybe Alias -> Relation -> [JoinCondition]
+getJoinConditions previousAlias newAlias (Relation Table{tableSchema=tSchema, tableName=tN} cols Table{tableName=ftN} fCols typ lt lc1 lc2) =
   if | typ == Child || typ == Parent ->
         zipWith (toJoinCondition tN ftN) cols fCols
      | typ == Many ->
@@ -243,8 +252,10 @@ getJoinConditions (Relation Table{tableSchema=tSchema, tableName=tN} cols Table{
   where
     toJoinCondition :: Text -> Text -> Column -> Column -> JoinCondition
     toJoinCondition tb ftb c fc =
-      JoinCondition (QualifiedIdentifier tSchema tb, Nothing, colName c)
+      let qi1 = QualifiedIdentifier tSchema tb
-                    (QualifiedIdentifier tSchema ftb, Nothing, colName fc)
+          qi2 = QualifiedIdentifier tSchema ftb in
+        JoinCondition (maybe qi1 (QualifiedIdentifier mempty) newAlias, colName c)
+                      (maybe qi2 (QualifiedIdentifier mempty) previousAlias, colName fc)
 
 addFiltersOrdersRanges :: ApiRequest -> Either ApiRequestError (ReadRequest -> ReadRequest)
 addFiltersOrdersRanges apiRequest = foldr1 (liftA2 (.)) [

src/PostgREST/QueryBuilder.hs

@@ -219,32 +219,21 @@ requestToCountQuery schema (DbRead (Node (Select{where_=logicForest}, (mainTbl,
    qi = removeSourceCTESchema schema mainTbl
 
 requestToQuery :: Schema -> Bool -> DbRequest -> SqlQuery
-requestToQuery schema isParent (DbRead (Node (Select colSelects tbl implJoins logicForest joinConditions_ ordts range, (_, maybeRelation, _, _, depth)) forest)) =
+requestToQuery schema isParent (DbRead (Node (Select colSelects tbl tblAlias implJoins logicForest joinConditions_ ordts range, _) forest)) =
   unwords [
     "SELECT " <> intercalate ", " (map (pgFmtSelectItem qi) colSelects ++ selects),
-    "FROM " <> intercalate ", " tables,
+    "FROM " <> intercalate ", " (tabl : implJs),
     unwords joins,
-    ("WHERE " <> intercalate " AND " (map (pgFmtLogicTree qi) logicForest ++ map pgFmtJoinCondition joinConds))
+    ("WHERE " <> intercalate " AND " (map (pgFmtLogicTree qi) logicForest ++ map pgFmtJoinCondition joinConditions_))
-      `emptyOnFalse` (null logicForest && null joinConds),
+      `emptyOnFalse` (null logicForest && null joinConditions_),
     ("ORDER BY " <> intercalate ", " (map (pgFmtOrderTerm qi) ordts)) `emptyOnFalse` null ordts,
     ("LIMIT " <> maybe "ALL" show (rangeLimit range) <> " OFFSET " <> show (rangeOffset range)) `emptyOnFalse` (isParent || range == allRange) ]
 
   where
-    tbls = tbl:implJoins
+    implJs = fromQi . QualifiedIdentifier schema <$> implJoins
-    isSelfJoin = maybe False (\r -> relType r /= Root && relTable r == relFTable r) maybeRelation
+    mainQi = removeSourceCTESchema schema tbl
-    (qi, tables, joinConds) =
+    tabl = fromQi mainQi <> maybe mempty (\a -> " AS " <> pgFmtIdent a) tblAlias
-      let depthAlias name dpth = if dpth /= 0  then name <> "_" <> show dpth else name in -- Root node doesn't get aliased
+    qi = maybe mainQi (QualifiedIdentifier mempty) tblAlias
-      if isSelfJoin
-        then (
-          QualifiedIdentifier "" (depthAlias tbl depth),
-          (\t -> fromQi (removeSourceCTESchema schema t) <> " AS " <> pgFmtIdent (depthAlias t depth)) <$> tbls,
-          (\(JoinCondition (qi1, _, c1) (qi2, _, c2)) ->
-            JoinCondition (qi1, Just $ depthAlias (qiName qi1) depth, c1)
-                          (qi2, Just $ depthAlias (qiName qi2) (depth - 1), c2)) <$> joinConditions_)
-        else (
-          removeSourceCTESchema schema tbl,
-          fromQi . removeSourceCTESchema schema <$> tbls,
-          joinConditions_)
 
     (joins, selects) = foldr getQueryParts ([],[]) forest
 
@@ -437,11 +426,9 @@ pgFmtFilter table (Filter fld (OpExpr hasNot oper)) = notOp <> " " <> case oper
      (find ((==) . toLower $ v) ["null","true","false"])
 
 pgFmtJoinCondition :: JoinCondition -> SqlFragment
-pgFmtJoinCondition (JoinCondition (qi, al1, col1) (QualifiedIdentifier schema fTable, al2, col2)) =
+pgFmtJoinCondition (JoinCondition (qi, col1) (QualifiedIdentifier schema fTable, col2)) =
-  pgFmtColumn (fromMaybe qi $ aliasToQi al1) col1 <> " = " <>
+  pgFmtColumn qi col1 <> " = " <>
-  pgFmtColumn (fromMaybe (removeSourceCTESchema schema fTable) $ aliasToQi al2) col2
+  pgFmtColumn (removeSourceCTESchema schema fTable) col2
-  where
-    aliasToQi al = QualifiedIdentifier "" <$> al
 
 pgFmtLogicTree :: QualifiedIdentifier -> LogicTree -> SqlFragment
 pgFmtLogicTree qi (Expr hasNot op forest) = notOp <> " (" <> intercalate (" " <> show op <> " ") (pgFmtLogicTree qi <$> forest) <> ")"

src/PostgREST/Types.hs

@@ -184,6 +184,9 @@ data Relation = Relation {
 , relLinkCols2 :: Maybe [Column]
 } deriving (Show, Eq)
 
+isSelfJoin :: Relation -> Bool
+isSelfJoin r = relType r /= Root && relTable r == relFTable r
+
 -- | Cached attributes of a JSON payload
 data PayloadJSON = PayloadJSON {
 -- | This is the raw ByteString that comes from the request body.
@@ -307,13 +310,15 @@ type SelectItem = (Field, Maybe Cast, Maybe Alias, Maybe RelationDetail)
 -- | Path of the embedded levels, e.g "clients.projects.name=eq.." gives Path ["clients", "projects"]
 type EmbedPath = [Text]
 data Filter = Filter { field::Field, opExpr::OpExpr } deriving (Show, Eq)
-data JoinCondition = JoinCondition (QualifiedIdentifier, Maybe Alias, FieldName)
+data JoinCondition = JoinCondition (QualifiedIdentifier, FieldName)
-                                   (QualifiedIdentifier, Maybe Alias, FieldName) deriving (Show, Eq)
+                                   (QualifiedIdentifier, FieldName) deriving (Show, Eq)
 
 data ReadQuery = Select {
     select         :: [SelectItem]
   , from           :: TableName
--- | Only used for many to many joins. Parent and Child joins use explicit joins.
+-- | A table alias is used in case of self joins
+  , fromAlias      :: Maybe Alias
+-- | Only used for Many to Many joins. Parent and Child joins use explicit joins.
   , implicitJoins  :: [TableName]
   , where_         :: [LogicTree]
   , joinConditions :: [JoinCondition]

test/Feature/QuerySpec.hs

@@ -589,6 +589,42 @@ spec = do
               ]
             }]|] { matchHeaders = [matchContentTypeJson] }
 
+        it "embeds other relations(manager) besides the self reference" $ do
+          get "/organizations?select=name,manager(name),referee(name,manager(name),auditor(name,manager(name))),auditor(name,manager(name),referee(name,manager(name)))&id=eq.5" `shouldRespondWith`
+            [json|[{
+              "name":"Cyberdyne",
+              "manager":{"name":"Cyberdyne Manager"},
+              "referee":{
+                "name":"Acme",
+                "manager":{"name":"Acme Manager"},
+                "auditor":{
+                  "name":"Auditor Org",
+                  "manager":{"name":"Auditor Manager"}}},
+              "auditor":{
+                "name":"Umbrella",
+                "manager":{"name":"Umbrella Manager"},
+                "referee":{
+                  "name":"Referee Org",
+                  "manager":{"name":"Referee Manager"}}}
+            }]|] { matchHeaders = [matchContentTypeJson] }
+
+          get "/organizations?select=name,manager(name),auditees:organizations.auditor(name,manager(name),refereeds:organizations.referee(name,manager(name)))&id=eq.2" `shouldRespondWith`
+            [json|[{
+              "name":"Auditor Org",
+              "manager":{"name":"Auditor Manager"},
+              "auditees":[
+                {"name":"Acme",
+                 "manager":{"name":"Acme Manager"},
+                 "refereeds":[
+                   {"name":"Cyberdyne",
+                    "manager":{"name":"Cyberdyne Manager"}},
+                   {"name":"Oscorp",
+                    "manager":{"name":"Oscorp Manager"}}]},
+                {"name":"Umbrella",
+                 "manager":{"name":"Umbrella Manager"},
+                 "refereeds":[]}]
+            }]|] { matchHeaders = [matchContentTypeJson] }
+
   describe "ordering response" $ do
     it "by a column asc" $
       get "/items?id=lte.2&order=id.asc"

test/fixtures/data.sql

@@ -354,11 +354,21 @@ INSERT INTO family_tree VALUES ('3', 'Kid Two', '1');
 INSERT INTO family_tree VALUES ('4', 'Grandkid One', '2');
 INSERT INTO family_tree VALUES ('5', 'Grandkid Two', '3');
 
+TRUNCATE TABLE managers CASCADE;
+INSERT INTO managers VALUES (1, 'Referee Manager');
+INSERT INTO managers VALUES (2, 'Auditor Manager');
+INSERT INTO managers VALUES (3, 'Acme Manager');
+INSERT INTO managers VALUES (4, 'Umbrella Manager');
+INSERT INTO managers VALUES (5, 'Cyberdyne Manager');
+INSERT INTO managers VALUES (6, 'Oscorp Manager');
+
 TRUNCATE TABLE organizations CASCADE;
-INSERT INTO organizations VALUES (1, 'Referee Org', null, null);
+INSERT INTO organizations VALUES (1, 'Referee Org', null, null, 1);
-INSERT INTO organizations VALUES (2, 'Auditor Org', null, null);
+INSERT INTO organizations VALUES (2, 'Auditor Org', null, null, 2);
-INSERT INTO organizations VALUES (3, 'Acme', 1, 2);
+INSERT INTO organizations VALUES (3, 'Acme', 1, 2, 3);
-INSERT INTO organizations VALUES (4, 'Umbrella', 1, 2);
+INSERT INTO organizations VALUES (4, 'Umbrella', 1, 2, 4);
+INSERT INTO organizations VALUES (5, 'Cyberdyne', 3, 4, 5);
+INSERT INTO organizations VALUES (6, 'Oscorp', 3, 4, 6);
 
 SET search_path = private, pg_catalog;
 

test/fixtures/privileges.sql

@@ -68,6 +68,7 @@ GRANT ALL ON TABLE
     , tiobe_pls
     , only_pk
     , family_tree
+    , managers
     , organizations
     , authors
     , books

test/fixtures/schema.sql

@@ -1115,8 +1115,8 @@ CREATE FUNCTION setprojects(id_l int, id_h int, name text) RETURNS SETOF project
 $_$;
 
 create table images (
-	name               text not null,
+	name text  not null,
-	img                bytea not null
+	img  bytea not null
 );
 
 create view images_base64 as (
@@ -1386,11 +1386,17 @@ create table test.family_tree (
 );
 alter table only test.family_tree add constraint pptr foreign key (parent) references test.family_tree(id);
 
+create table test.managers (
+  id integer primary key,
+  name text
+);
+
 create table test.organizations (
   id integer primary key,
   name text,
   referee integer,
-  auditor integer
+  auditor integer,
+  manager_id integer references managers(id)
 );
 alter table only test.organizations add constraint pptr1 foreign key (referee) references test.organizations(id);
 alter table only test.organizations add constraint pptr2 foreign key (auditor) references test.organizations(id);