refactor: simplify addJoinConditions

* remove unnecessary Either
* remove implicitJoins from ReadQuery
* remove unnecessary foldr in addJoinConditions
* move aliasing to addRels

src/PostgREST/Query/QueryBuilder.hs

@@ -24,6 +24,7 @@ import Data.Tree (Tree (..))
 import PostgREST.DbStructure.Identifiers  (QualifiedIdentifier (..))
 import PostgREST.DbStructure.Proc         (ProcParam (..))
 import PostgREST.DbStructure.Relationship (Cardinality (..),
+                                           Junction (..),
                                            Relationship (..))
 import PostgREST.Request.Preferences      (PreferResolution (..))
 
@@ -36,10 +37,10 @@ import PostgREST.Request.Types
 import Protolude
 
 readRequestToQuery :: ReadRequest -> SQL.Snippet
-readRequestToQuery (Node (Select colSelects mainQi tblAlias implJoins logicForest joinConditions_ ordts range, _) forest) =
+readRequestToQuery (Node (Select colSelects mainQi tblAlias logicForest joinConditions_ ordts range, (_, rel, _, _, _, _)) forest) =
   "SELECT " <>
   intercalateSnippet ", " ((pgFmtSelectItem qi <$> colSelects) ++ selects) <> " " <>
-  "FROM " <> SQL.sql (BS.intercalate ", " (tabl : implJs)) <> " " <>
+  "FROM " <> SQL.sql tabl <> implicitJoinF rel <> " " <>
   intercalateSnippet " " joins <> " " <>
   (if null logicForest && null joinConditions_
     then mempty
@@ -47,12 +48,12 @@ readRequestToQuery (Node (Select colSelects mainQi tblAlias implJoins logicFores
   orderF qi ordts <> " " <>
   limitOffsetF range
   where
-    implJs = fromQi <$> implJoins
     tabl = fromQi mainQi <> maybe mempty (\a -> " AS " <> pgFmtIdent a) tblAlias
     qi = maybe mainQi (QualifiedIdentifier mempty) tblAlias
     (selects, joins) = foldr getSelectsJoins ([],[]) forest
 
 getSelectsJoins :: ReadRequest -> ([SQL.Snippet], [SQL.Snippet]) -> ([SQL.Snippet], [SQL.Snippet])
+getSelectsJoins (Node (_, (_, Nothing, _, _, _, _)) _) _ = ([], [])
 getSelectsJoins rr@(Node (_, (name, Just Relationship{relCardinality=card,relTable=QualifiedIdentifier{qiName=table}}, alias, _, joinType, _)) _) (selects,joins) =
   let
     subquery = readRequestToQuery rr
@@ -74,7 +75,6 @@ getSelectsJoins rr@(Node (_, (name, Just Relationship{relCardinality=card,relTab
           ) localTableName $ if joinType == Just JTInner then SQL.sql localTableName <> " IS NOT NULL" else "TRUE")
   in
   (sel:selects, joi:joins)
-getSelectsJoins (Node (_, (_, Nothing, _, _, _, _)) _) _ = ([], [])
 
 mutateRequestToQuery :: MutateRequest -> SQL.Snippet
 mutateRequestToQuery (Insert mainQi iCols body onConflct putConditions returnings) =
@@ -218,20 +218,19 @@ requestToCallProcQuery (FunctionCall qi params args returnsScalar multipleCall r
 -- See https://github.com/PostgREST/postgrest/issues/2009#issuecomment-977473031
 -- Only for the nodes that have an INNER JOIN linked to the root level.
 readRequestToCountQuery :: ReadRequest -> SQL.Snippet
-readRequestToCountQuery (Node (Select{from=mainQi, fromAlias=tblAlias, implicitJoins=implJoins, where_=logicForest, joinConditions=joinConditions_}, _) forest) =
+readRequestToCountQuery (Node (Select{from=mainQi, fromAlias=tblAlias, where_=logicForest, joinConditions=joinConditions_}, (_, rel, _, _, _, _)) forest) =
-  "SELECT 1 FROM " <> SQL.sql (BS.intercalate ", " (tabl : implJs)) <>
+  "SELECT 1 FROM " <> SQL.sql tabl <> implicitJoinF rel <>
   (if null logicForest && null joinConditions_ && null subQueries
     then mempty
     else " WHERE " ) <>
   intercalateSnippet " AND " (
-    map (pgFmtLogicTree treeQi) logicForest ++
+    map (pgFmtLogicTree qi) logicForest ++
     map pgFmtJoinCondition joinConditions_ ++
     subQueries
   )
   where
-    treeQi = maybe mainQi (QualifiedIdentifier (qiSchema mainQi)) tblAlias
+    qi = maybe mainQi (QualifiedIdentifier mempty) tblAlias
     tabl = fromQi mainQi <> maybe mempty (\a -> " AS " <> pgFmtIdent a) tblAlias
-    implJs = fromQi <$> implJoins
     subQueries = foldr existsSubquery [] forest
     existsSubquery :: ReadRequest -> [SQL.Snippet] -> [SQL.Snippet]
     existsSubquery readReq@(Node (_, (_, _, _, _, joinType, _)) _) rest =
@@ -241,3 +240,8 @@ readRequestToCountQuery (Node (Select{from=mainQi, fromAlias=tblAlias, implicitJ
 
 limitedQuery :: SQL.Snippet -> Maybe Integer -> SQL.Snippet
 limitedQuery query maxRows = query <> SQL.sql (maybe mempty (\x -> " LIMIT " <> BS.pack (show x)) maxRows)
+
+implicitJoinF :: Maybe Relationship -> SQL.Snippet
+implicitJoinF rel = case relCardinality <$> rel of
+  Just (M2M Junction{junTable=jt}) -> ", " <> SQL.sql (fromQi jt)
+  _                                -> mempty

src/PostgREST/Request/DbRequestBuilder.hs

@@ -87,7 +87,7 @@ initReadRequest rootQi rootAlias =
     rootDepth = 0
     rootSchema = qiSchema rootQi
     rootName = qiName rootQi
-    initial = Node (Select [] rootQi rootAlias [] [] [] [] allRange, (rootName, Nothing, Nothing, Nothing, Nothing, rootDepth)) []
+    initial = Node (Select [] rootQi rootAlias [] [] [] allRange, (rootName, Nothing, Nothing, Nothing, Nothing, rootDepth)) []
     treeEntry :: Depth -> Tree SelectItem -> ReadRequest -> ReadRequest
     treeEntry depth (Node fld@((fn, _),_,alias, hint, joinType) fldForest) (Node (q, i) rForest) =
       let nxtDepth = succ depth in
@@ -95,7 +95,7 @@ initReadRequest rootQi rootAlias =
         [] -> Node (q {select=fld:select q}, i) rForest
         _  -> Node (q, i) $
               foldr (treeEntry nxtDepth)
-              (Node (Select [] (QualifiedIdentifier rootSchema fn) Nothing [] [] [] [] allRange,
+              (Node (Select [] (QualifiedIdentifier rootSchema fn) Nothing [] [] [] allRange,
                 (fn, Nothing, alias, hint, joinType, nxtDepth)) [])
               fldForest:rForest
 
@@ -109,16 +109,19 @@ treeRestrictRange maxRows _ request = pure $ nodeRestrictRange maxRows <$> reque
 
 augmentRequestWithJoin :: Schema -> RelationshipsMap -> ReadRequest -> Either ApiRequestError ReadRequest
 augmentRequestWithJoin schema allRels request =
-  addRels schema allRels Nothing request
+ addJoinConditions Nothing <$> addRels schema allRels Nothing request
-  >>= addJoinConditions Nothing
 
 addRels :: Schema -> RelationshipsMap -> Maybe ReadRequest -> ReadRequest -> Either ApiRequestError ReadRequest
 addRels schema allRels parentNode (Node (query@Select{from=tbl}, (nodeName, _, alias, hint, joinType, depth)) forest) =
   case parentNode of
     Just (Node (Select{from=parentNodeQi, fromAlias=aliasQi}, _) _) ->
       let newFrom r = if qiName tbl == nodeName then relForeignTable r else tbl
-          newReadNode = (\r -> (query{from=newFrom r}, (nodeName, Just r, alias, hint, joinType, depth))) <$> rel
+          newReadNode = (\r ->
-          origin = if depth == 1 -- Only on depth 1 we check if the parent(depth 0) has an alias so the sourceCTEName alias can be found as a relationship
+            if not $ relIsSelf r -- add alias if self rel TODO consolidate aliasing in another function
+              then (query{from=newFrom r}, (nodeName, Just r, alias, hint, joinType, depth))
+              else (query{from=newFrom r, fromAlias=Just (qiName (newFrom r) <> "_" <> show depth)}, (nodeName, Just r, alias, hint, joinType, depth))
+            ) <$> rel
+          origin = if depth == 1 -- Only on depth 1 we check if the root(depth 0) has an alias so the sourceCTEName alias can be found as a relationship
             then fromMaybe (qiName parentNodeQi) aliasQi
             else qiName parentNodeQi
           rel = findRel schema allRels origin nodeName hint
@@ -131,6 +134,27 @@ addRels schema allRels parentNode (Node (query@Select{from=tbl}, (nodeName, _, a
     updateForest :: Maybe ReadRequest -> Either ApiRequestError [ReadRequest]
     updateForest rq = addRels schema allRels rq `traverse` forest
 
+-- applies aliasing to join conditions TODO refactor, this should go into the querybuilder module
+addJoinConditions :: Maybe Alias -> ReadRequest -> ReadRequest
+addJoinConditions _ (Node node@(Select{fromAlias=tblAlias}, (_, Nothing, _, _, _, _)) forest) = Node node (addJoinConditions tblAlias <$> forest)
+addJoinConditions previousAlias (Node (query@Select{fromAlias=tblAlias}, nodeProps@(_, Just (Relationship QualifiedIdentifier{qiSchema=tSchema, qiName=tN} QualifiedIdentifier{qiName=ftN} _ card _ _), _, _, _, _)) forest) =
+  Node (query{joinConditions=joinConds}, nodeProps) (addJoinConditions tblAlias <$> forest)
+  where
+    joinConds =
+      case card of
+        M2M (Junction QualifiedIdentifier{qiName=jtn} _ _ jcols1 jcols2) ->
+           (toJoinCondition Nothing Nothing ftN jtn <$> jcols2) ++ (toJoinCondition previousAlias tblAlias tN jtn <$> jcols1)
+        O2M _ cols ->
+          toJoinCondition previousAlias tblAlias tN ftN <$> cols
+        M2O _ cols ->
+          toJoinCondition previousAlias tblAlias tN ftN <$> cols
+    toJoinCondition :: Maybe Alias -> Maybe Alias -> Text -> Text -> (FieldName, FieldName) -> JoinCondition
+    toJoinCondition prAl newAl tb ftb (c, fc) =
+      let qi1 = QualifiedIdentifier tSchema ftb
+          qi2 = QualifiedIdentifier tSchema tb in
+        JoinCondition (maybe qi1 (QualifiedIdentifier mempty) newAl, fc)
+                      (maybe qi2 (QualifiedIdentifier mempty) prAl, c)
+
 -- Finds a relationship between an origin and a target in the request:
 -- /origin?select=target(*) If more than one relationship is found then the
 -- request is ambiguous and we return an error.  In that case the request can
@@ -210,46 +234,6 @@ findRel schema allRels origin target hint =
             )
       ) $ fromMaybe mempty $ HM.lookup (QualifiedIdentifier schema origin, schema) allRels
 
--- previousAlias is only used for the case of self joins
-addJoinConditions :: Maybe Alias -> ReadRequest -> Either ApiRequestError ReadRequest
-addJoinConditions previousAlias (Node node@(query@Select{from=tbl,fromAlias=tblAlias}, nodeProps@(_, rel, _, _, _, depth)) forest) =
-  case rel of
-    Just r@Relationship{relCardinality=M2M Junction{junTable}} ->
-      let rq = augmentQuery r in
-      Node (rq{implicitJoins=junTable:implicitJoins rq}, nodeProps) <$> updatedForest
-    Just r -> Node (augmentQuery r, nodeProps) <$> updatedForest
-    Nothing -> Node node <$> updatedForest
-  where
-    newAlias = if depth == 0
-      then tblAlias -- only use the alias on the root node(depth 0) for when the sourceCTEName alias is used for joining
-      else case relIsSelf <$> rel of -- no need to apply the self reference alias on depth 0 only on the next depths
-        Just True -> Just (qiName tbl <> "_" <> show depth)
-        _         -> Nothing
-    augmentQuery r =
-      foldr
-        (\jc rq@Select{joinConditions=jcs} -> rq{joinConditions=jc:jcs})
-        query{fromAlias=newAlias}
-        (getJoinConditions previousAlias newAlias r)
-    updatedForest = addJoinConditions newAlias `traverse` forest
-
--- previousAlias and newAlias are used in the case of self joins
-getJoinConditions :: Maybe Alias -> Maybe Alias -> Relationship -> [JoinCondition]
-getJoinConditions previousAlias newAlias (Relationship QualifiedIdentifier{qiSchema=tSchema, qiName=tN} QualifiedIdentifier{qiName=ftN} _ card _ _) =
-  case card of
-    M2M (Junction QualifiedIdentifier{qiName=jtn} _ _ jcols1 jcols2) ->
-      (toJoinCondition previousAlias newAlias tN jtn <$> jcols1) ++ (toJoinCondition Nothing Nothing ftN jtn <$> jcols2)
-    O2M _ cols ->
-      toJoinCondition previousAlias newAlias tN ftN <$> cols
-    M2O _ cols ->
-      toJoinCondition previousAlias newAlias tN ftN <$> cols
-  where
-    toJoinCondition :: Maybe Alias -> Maybe Alias -> Text -> Text -> (FieldName, FieldName) -> JoinCondition
-    toJoinCondition prAl newAl tb ftb (c, fc) =
-      let qi1 = QualifiedIdentifier tSchema tb
-          qi2 = QualifiedIdentifier tSchema ftb in
-        JoinCondition (maybe qi1 (QualifiedIdentifier mempty) prAl, c)
-                      (maybe qi2 (QualifiedIdentifier mempty) newAl, fc)
-
 addFilters :: ApiRequest -> ReadRequest -> Either ApiRequestError ReadRequest
 addFilters ApiRequest{..} rReq =
   foldr addFilterToNode (Right rReq) flts

src/PostgREST/Request/ReadQuery.hs

@@ -31,10 +31,8 @@ type SelectItem = (Field, Maybe Cast, Maybe Alias, Maybe Hint, Maybe JoinType)
 data ReadQuery = Select
   { select         :: [SelectItem]
   , from           :: QualifiedIdentifier
-  -- ^ 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.
+  -- ^ A table alias is used in case of self joins
-  , implicitJoins  :: [QualifiedIdentifier]
   , where_         :: [LogicTree]
   , joinConditions :: [JoinCondition]
   , order          :: [OrderTerm]