fix: aliasing on computed rels

CHANGELOG.md

@@ -9,6 +9,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 
  - #2401, #2444, Fix SIGUSR1 to fully flush connections pool, remove `db-pool-timeout`. - @robx
  - #2348, Add `db-pool-acquisition-timeout` configuration option, time in seconds to wait to acquire a connection. - @robx
+ - #2428, Fix opening an empty transaction on failed resource embedding - @steve-chavez
+ - #2455, Fix embedding the same table multiple times - @steve-chavez
 
 ### Deprecated
 

src/PostgREST/Plan.hs

@@ -126,23 +126,24 @@ treeRestrictRange maxRows _ request = pure $ nodeRestrictRange maxRows <$> reque
     nodeRestrictRange :: Maybe Integer -> ReadPlan -> ReadPlan
     nodeRestrictRange m q@ReadPlan{range_=r} = q{range_=restrictRange m r }
 
--- add relationships to the nodes of the tree by traversing the forest while keeping track of the parentNode
+-- add relationships to the nodes of the tree by traversing the forest while keeping track of the parentNode, also adds aliasing
 addRels :: Schema -> Action -> RelationshipsMap -> Maybe ReadPlanTree -> ReadPlanTree -> Either ApiRequestError ReadPlanTree
-addRels schema action allRels parentNode (Node rPlan@ReadPlan{from=tbl,relName,relHint,depth} forest) =
+addRels schema action allRels parentNode (Node rPlan@ReadPlan{relName,relHint,depth} forest) =
   case parentNode of
     Just (Node ReadPlan{from=parentNodeQi, fromAlias} _) ->
-      let newFrom r = if qiName tbl == relName then relForeignTable r else tbl
-          newReadPlan = (\r ->
-            if not $ relIsSelf r -- add alias if self rel
-              then rPlan{from=newFrom r, relToParent=Just r, relJoinConds=getJoinConditions Nothing fromAlias r}
-              else
-                let selfAlias = Just (qiName (newFrom r) <> "_" <> show depth) in
-                rPlan{from=newFrom r, relToParent=Just r, fromAlias=selfAlias, relJoinConds=getJoinConditions selfAlias fromAlias 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) fromAlias
-            else qiName parentNodeQi
-          rel = findRel schema allRels origin relName relHint
+      let
+        newReadPlan = (\r ->
+          let newAlias = Just (qiName (relForeignTable r) <> "_" <> show depth) in
+          case r of
+            Relationship{relCardinality=M2M _} ->
+              rPlan{from=relForeignTable r, relToParent=Just r, relJoinConds=getJoinConditions Nothing fromAlias r}
+            _ ->
+              rPlan{from=relForeignTable r, relToParent=Just r, fromAlias=newAlias, relJoinConds=getJoinConditions newAlias fromAlias 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) fromAlias
+          else qiName parentNodeQi
+        rel = findRel schema allRels origin relName relHint
       in
       Node <$> newReadPlan <*> (updateForest . hush $ Node <$> newReadPlan <*> pure forest)
     Nothing -> -- root case

src/PostgREST/Plan/ReadPlan.hs

@@ -32,7 +32,6 @@ data ReadPlan = ReadPlan
   { select       :: [SelectItem]
   , from         :: QualifiedIdentifier
   , fromAlias    :: Maybe Alias
-  -- ^ A table alias is used in case of self joins
   , where_       :: [LogicTree]
   , order        :: [OrderTerm]
   , range_       :: NonnegRange

test/spec/Feature/Query/ComputedRelsSpec.hs

@@ -107,3 +107,12 @@ spec = describe "computed relationships" $ do
     get "/second_1?select=*,first_1(*)"
       `shouldRespondWith`
       [json|[]|] { matchHeaders = [matchContentTypeJson] }
+
+  -- https://github.com/PostgREST/postgrest/issues/2455
+  it "creates queries with the right aliasing" $ do
+    get "/fee?select=*,jsbaz(*,janedoe(*))"
+      `shouldRespondWith`
+      [json|[]|] { matchHeaders = [matchContentTypeJson] }
+    get "/fee?select=*,jsbaz(*,johnsmith(*, fee(*)))"
+      `shouldRespondWith`
+      [json|[]|] { matchHeaders = [matchContentTypeJson] }

test/spec/fixtures/privileges.sql

@@ -210,6 +210,10 @@ GRANT ALL ON TABLE
     , second
     , first_1
     , second_1
+    , fee
+    , baz
+    , janedoe
+    , johnsmith
 TO postgrest_test_anonymous;
 
 GRANT INSERT ON TABLE insertonly TO postgrest_test_anonymous;

test/spec/fixtures/schema.sql

@@ -2836,3 +2836,29 @@ $$ LANGUAGE sql STABLE ROWS 1;
 CREATE FUNCTION test.first_1(test.second_1) RETURNS SETOF test.first_1 AS $$
   SELECT * FROM test.first_1 WHERE second_id_1 = $1.id;
 $$ LANGUAGE sql STABLE ROWS 1;
+
+
+create table fee (
+  fee_id int primary key
+);
+create table baz (
+  baz_id int primary key
+);
+
+create table janedoe (
+  janedoe_id int primary key,
+  baz_id int references baz(baz_id)
+);
+
+create table johnsmith (
+  johnsmith_id int primary key,
+  fee_id int references fee(fee_id),
+  baz_id int references baz(baz_id)
+);
+
+create or replace function jsbaz(fee) returns setof baz as $$
+    select b.*
+    from baz b
+    join johnsmith js on js.baz_id = b.baz_id
+    where js.fee_id = $1.fee_id
+$$ stable language sql;