refactor: simplify join conditions logic

src/PostgREST/ApiRequest/Types.hs

@@ -9,7 +9,6 @@ module PostgREST.ApiRequest.Types
   , Field
   , Filter(..)
   , Hint
-  , JoinCondition(..)
   , JoinType(..)
   , JsonOperand(..)
   , JsonOperation(..)
@@ -33,8 +32,7 @@ module PostgREST.ApiRequest.Types
   ) where
 
 import PostgREST.MediaType                (MediaType (..))
-import PostgREST.SchemaCache.Identifiers  (FieldName,
+import PostgREST.SchemaCache.Identifiers  (FieldName)
-                                           QualifiedIdentifier)
 import PostgREST.SchemaCache.Proc         (ProcDescription (..))
 import PostgREST.SchemaCache.Relationship (Relationship)
 
@@ -72,12 +70,6 @@ data RangeError
 type NodeName = Text
 type Depth = Integer
 
-data JoinCondition =
-  JoinCondition
-    (QualifiedIdentifier, FieldName)
-    (QualifiedIdentifier, FieldName)
-  deriving (Eq)
-
 data OrderTerm = OrderTerm
   { otTerm      :: Field
   , otDirection :: Maybe OrderDirection

src/PostgREST/Plan.hs

@@ -95,7 +95,7 @@ readPlan :: QualifiedIdentifier -> AppConfig -> SchemaCache -> ApiRequest -> Eit
 readPlan qi@QualifiedIdentifier{..} AppConfig{configDbMaxRows} SchemaCache{dbRelationships} apiRequest  =
   mapLeft ApiRequestError $
   treeRestrictRange configDbMaxRows (iAction apiRequest) =<<
-  augmentRequestWithJoin qiSchema dbRelationships =<<
+  addRels qiSchema dbRelationships Nothing =<<
   addLogicTrees apiRequest =<<
   addRanges apiRequest =<<
   addOrders apiRequest =<<
@@ -118,7 +118,7 @@ initReadRequest rootQi rootAlias =
     rootDepth = 0
     rootSchema = qiSchema rootQi
     rootName = qiName rootQi
-    initial = Node (ReadPlan [] rootQi rootAlias [] [] [] allRange rootName Nothing Nothing Nothing Nothing rootDepth) []
+    initial = Node (ReadPlan [] rootQi rootAlias [] [] allRange rootName Nothing [] Nothing Nothing Nothing rootDepth) []
     treeEntry :: Depth -> Tree SelectItem -> ReadPlanTree -> ReadPlanTree
     treeEntry depth (Node fld@((fn, _),_,alias, hint, joinType) fldForest) (Node q rForest) =
       let nxtDepth = succ depth in
@@ -126,7 +126,7 @@ initReadRequest rootQi rootAlias =
         [] -> Node q{select=fld:select q} rForest
         _  -> Node q $
               foldr (treeEntry nxtDepth)
-              (Node (ReadPlan [] (QualifiedIdentifier rootSchema fn) Nothing [] [] [] allRange fn Nothing alias hint joinType nxtDepth) [])
+              (Node (ReadPlan [] (QualifiedIdentifier rootSchema fn) Nothing [] [] allRange fn Nothing [] alias hint joinType nxtDepth) [])
               fldForest:rForest
 
 -- | Enforces the `max-rows` config on the result
@@ -137,19 +137,18 @@ treeRestrictRange maxRows _ request = pure $ nodeRestrictRange maxRows <$> reque
     nodeRestrictRange :: Maybe Integer -> ReadPlan -> ReadPlan
     nodeRestrictRange m q@ReadPlan{range_=r} = q{range_=restrictRange m r }
 
-augmentRequestWithJoin :: Schema -> RelationshipsMap -> ReadPlanTree -> Either ApiRequestError ReadPlanTree
+-- add relationships to the nodes of the tree by traversing the forest while keeping track of the parentNode
-augmentRequestWithJoin schema allRels request =
- addJoinConditions Nothing <$> addRels schema allRels Nothing request
-
 addRels :: Schema -> RelationshipsMap -> Maybe ReadPlanTree -> ReadPlanTree -> Either ApiRequestError ReadPlanTree
 addRels schema allRels parentNode (Node query@ReadPlan{from=tbl,relName,relHint,depth} forest) =
   case parentNode of
     Just (Node ReadPlan{from=parentNodeQi, fromAlias=aliasQi} _) ->
       let newFrom r = if qiName tbl == relName then relForeignTable r else tbl
           newReadPlan = (\r ->
-            if not $ relIsSelf r -- add alias if self rel TODO consolidate aliasing in another function
+            if not $ relIsSelf r -- add alias if self rel
-              then query{from=newFrom r, relToParent=Just r}
+              then query{from=newFrom r, relToParent=Just r, relJoinConds=getJoinConditions Nothing aliasQi r}
-              else query{from=newFrom r, relToParent=Just r, fromAlias=Just (qiName (newFrom r) <> "_" <> show depth)}
+              else
+                let selfAlias = Just (qiName (newFrom r) <> "_" <> show depth) in
+                query{from=newFrom r, relToParent=Just r, fromAlias=selfAlias, relJoinConds=getJoinConditions selfAlias aliasQi r}
             ) <$> 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
@@ -163,25 +162,21 @@ addRels schema allRels parentNode (Node query@ReadPlan{from=tbl,relName,relHint,
     updateForest :: Maybe ReadPlanTree -> Either ApiRequestError [ReadPlanTree]
     updateForest rq = addRels schema allRels rq `traverse` forest
 
--- applies aliasing to join conditions TODO refactor, this should go into the querybuilder module
+getJoinConditions :: Maybe Alias -> Maybe Alias -> Relationship -> [JoinCondition]
-addJoinConditions :: Maybe Alias -> ReadPlanTree -> ReadPlanTree
+getJoinConditions _ _ ComputedRelationship{} = []
-addJoinConditions _ (Node node@ReadPlan{fromAlias=tblAlias, relToParent=Nothing} forest) = Node node (addJoinConditions tblAlias <$> forest)
+getJoinConditions tblAlias parentAlias Relationship{relTable=qi,relForeignTable=fQi,relCardinality=card} =
-addJoinConditions _ (Node node@ReadPlan{fromAlias=tblAlias, relToParent=Just ComputedRelationship{}} forest) = Node node (addJoinConditions tblAlias <$> forest)
+  case card of
-addJoinConditions previousAlias (Node query@ReadPlan{fromAlias=tblAlias, relToParent=Just Relationship{relTable=qi,relForeignTable=fQi,relCardinality=card}} forest) =
+    M2M (Junction QualifiedIdentifier{qiName=jtn} _ _ jcols1 jcols2) ->
-  Node query{joinConditions=joinConds} (addJoinConditions tblAlias <$> forest)
+      (toJoinCondition Nothing Nothing ftN jtn <$> jcols2) ++ (toJoinCondition parentAlias tblAlias tN jtn <$> jcols1)
+    O2M _ cols ->
+      toJoinCondition parentAlias tblAlias tN ftN <$> cols
+    M2O _ cols ->
+      toJoinCondition parentAlias tblAlias tN ftN <$> cols
+    O2O _ cols ->
+      toJoinCondition parentAlias tblAlias tN ftN <$> cols
   where
     QualifiedIdentifier{qiSchema=tSchema, qiName=tN} = qi
     QualifiedIdentifier{qiName=ftN} = fQi
-    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
-        O2O _ 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

src/PostgREST/Plan/ReadPlan.hs

@@ -3,14 +3,15 @@ module PostgREST.Plan.ReadPlan
   ( ReadPlanTree
   , ReadPlan(..)
   , fstFieldNames
+  , JoinCondition(..)
   ) where
 
 import Data.Tree (Tree (..))
 
 import PostgREST.ApiRequest.Types         (Alias, Depth, Hint,
-                                           JoinCondition, JoinType,
+                                           JoinType, LogicTree,
-                                           LogicTree, NodeName,
+                                           NodeName, OrderTerm,
-                                           OrderTerm, SelectItem)
+                                           SelectItem)
 import PostgREST.RangeQuery               (NonnegRange)
 import PostgREST.SchemaCache.Identifiers  (FieldName,
                                            QualifiedIdentifier)
@@ -21,21 +22,27 @@ import Protolude
 
 type ReadPlanTree = Tree ReadPlan
 
+data JoinCondition =
+  JoinCondition
+    (QualifiedIdentifier, FieldName)
+    (QualifiedIdentifier, FieldName)
+  deriving (Eq)
+
 data ReadPlan = ReadPlan
-  { select         :: [SelectItem]
+  { select       :: [SelectItem]
-  , from           :: QualifiedIdentifier
+  , from         :: QualifiedIdentifier
-  , fromAlias      :: Maybe Alias
+  , fromAlias    :: Maybe Alias
   -- ^ A table alias is used in case of self joins
-  , where_         :: [LogicTree]
+  , where_       :: [LogicTree]
-  , joinConditions :: [JoinCondition]
+  , order        :: [OrderTerm]
-  , order          :: [OrderTerm]
+  , range_       :: NonnegRange
-  , range_         :: NonnegRange
+  , relName      :: NodeName
-  , relName        :: NodeName
+  , relToParent  :: Maybe Relationship
-  , relToParent    :: Maybe Relationship
+  , relJoinConds :: [JoinCondition]
-  , relAlias       :: Maybe Alias
+  , relAlias     :: Maybe Alias
-  , relHint        :: Maybe Hint
+  , relHint      :: Maybe Hint
-  , relJoinType    :: Maybe JoinType
+  , relJoinType  :: Maybe JoinType
-  , depth          :: Depth
+  , depth        :: Depth
   }
   deriving (Eq)
 

src/PostgREST/Query/QueryBuilder.hs

@@ -39,14 +39,14 @@ import PostgREST.RangeQuery        (allRange)
 import Protolude
 
 readPlanToQuery :: ReadPlanTree -> SQL.Snippet
-readPlanToQuery (Node ReadPlan{select,from=mainQi,fromAlias,where_=logicForest,joinConditions, order, range_=readRange, relToParent} forest) =
+readPlanToQuery (Node ReadPlan{select,from=mainQi,fromAlias,where_=logicForest,order, range_=readRange, relToParent, relJoinConds} forest) =
   "SELECT " <>
   intercalateSnippet ", " ((pgFmtSelectItem qi <$> select) ++ selects) <> " " <>
   fromFrag <> " " <>
   intercalateSnippet " " joins <> " " <>
-  (if null logicForest && null joinConditions
+  (if null logicForest && null relJoinConds
     then mempty
-    else "WHERE " <> intercalateSnippet " AND " (map (pgFmtLogicTree qi) logicForest ++ map pgFmtJoinCondition joinConditions)) <> " " <>
+    else "WHERE " <> intercalateSnippet " AND " (map (pgFmtLogicTree qi) logicForest ++ map pgFmtJoinCondition relJoinConds)) <> " " <>
   orderF qi order <> " " <>
   limitOffsetF readRange
   where
@@ -225,14 +225,14 @@ callPlanToQuery (FunctionCall qi params args returnsScalar multipleCall returnin
 -- See https://github.com/PostgREST/postgrest/issues/2009#issuecomment-977473031
 -- Only for the nodes that have an INNER JOIN linked to the root level.
 readPlanToCountQuery :: ReadPlanTree -> SQL.Snippet
-readPlanToCountQuery (Node ReadPlan{from=mainQi, fromAlias=tblAlias, where_=logicForest, joinConditions=joinConditions_, relToParent=rel} forest) =
+readPlanToCountQuery (Node ReadPlan{from=mainQi, fromAlias=tblAlias, where_=logicForest, relToParent=rel, relJoinConds} forest) =
   "SELECT 1 " <> fromFrag <>
-  (if null logicForest && null joinConditions_ && null subQueries
+  (if null logicForest && null relJoinConds && null subQueries
     then mempty
     else " WHERE " ) <>
   intercalateSnippet " AND " (
     map (pgFmtLogicTree qi) logicForest ++
-    map pgFmtJoinCondition joinConditions_ ++
+    map pgFmtJoinCondition relJoinConds ++
     subQueries
   )
   where

src/PostgREST/Query/SqlFragment.hs

@@ -58,7 +58,6 @@ import Text.InterpolatedString.Perl6 (qc)
 
 import PostgREST.ApiRequest.Types        (Alias, Field, Filter (..),
                                           FtsOperator (..),
-                                          JoinCondition (..),
                                           JsonOperand (..),
                                           JsonOperation (..),
                                           JsonPath,
@@ -72,6 +71,7 @@ import PostgREST.ApiRequest.Types        (Alias, Field, Filter (..),
                                           TrileanVal (..))
 import PostgREST.MediaType               (MTPlanFormat (..),
                                           MTPlanOption (..))
+import PostgREST.Plan.ReadPlan           (JoinCondition (..))
 import PostgREST.RangeQuery              (NonnegRange, allRange,
                                           rangeLimit, rangeOffset)
 import PostgREST.SchemaCache.Identifiers (FieldName,