diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c7afc554..e09f5d115 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### Fixed +- #1182, Fix embedding on views with composite pks - @steve-chavez +- #1180, Fix embedding on views with subselects in pg10 - @steve-chavez + ## [5.1.0] - 2018-08-31 ### Added diff --git a/src/PostgREST/DbRequestBuilder.hs b/src/PostgREST/DbRequestBuilder.hs index 005111be7..b48c23fcf 100644 --- a/src/PostgREST/DbRequestBuilder.hs +++ b/src/PostgREST/DbRequestBuilder.hs @@ -225,12 +225,12 @@ addJoinConditions schema (Node node@(query, nodeProps@(_, relation, _, _, _)) fo addJoinCond jc rq@Select{joinConditions=jcs} = rq{joinConditions=jc:jcs} getJoinConditions :: Relation -> [JoinCondition] -getJoinConditions (Relation Table{tableSchema=tSchema, tableName=tN} cols Table{tableName=ftN} fcs typ lt lc1 lc2) = +getJoinConditions (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 fcs + zipWith (toJoinCondition tN ftN) cols fCols | typ == Many -> let ltN = fromMaybe "" (tableName <$> lt) in - zipWith (toJoinCondition tN ltN) cols (fromMaybe [] lc1) ++ zipWith (toJoinCondition ftN ltN) fcs (fromMaybe [] lc2) + zipWith (toJoinCondition tN ltN) cols (fromMaybe [] lc1) ++ zipWith (toJoinCondition ftN ltN) fCols (fromMaybe [] lc2) | typ == Root -> witness where toJoinCondition :: Text -> Text -> Column -> Column -> JoinCondition diff --git a/src/PostgREST/DbStructure.hs b/src/PostgREST/DbStructure.hs index 8c2e6d3ca..614ff5ff5 100644 --- a/src/PostgREST/DbStructure.hs +++ b/src/PostgREST/DbStructure.hs @@ -41,7 +41,7 @@ getDbStructure schema pgVer = do keys <- H.statement () $ allPrimaryKeys tabs procs <- H.statement schema allProcs - let rels = addManyToManyRelations . addParentRelations $ addViewRelations syns childRels + let rels = addManyToManyRelations . addParentRelations $ addViewChildRelations syns childRels cols' = addForeignKeys rels cols keys' = addViewPrimaryKeys syns keys @@ -243,32 +243,32 @@ Having a Relation{relTable=t1, relColumns=[c1], relFTable=t2, relFColumns=[c2], t1.c1------t2.c2 -When only having a t1_view.c1 synonym, we need to add a View to Table Relation +When only having a t1_view.c1 synonym, we need to add a View to Table Child Relation t1.c1----t2.c2 t1.c1----------t2.c2 - -> --------/ + -> ________/ / t1_view.c1 t1_view.c1 -When only having a t2_view.c2 synonym, we need to add a Table to View Relation +When only having a t2_view.c2 synonym, we need to add a Table to View Child Relation t1.c1----t2.c2 t1.c1----------t2.c2 - -> \-------- + -> \________ \ t2_view.c2 t2_view.c1 -When having t1_view.c1 and a t2_view.c2 synonyms, we need to add a View to View Relation in addition to the prior +When having t1_view.c1 and a t2_view.c2 synonyms, we need to add a View to View Child Relation in addition to the prior t1.c1----t2.c2 t1.c1----------t2.c2 - -> \--------/ + -> \________/ / \ t1_view.c1 t2_view.c2 t1_view.c1-------t2_view.c1 The logic for composite pks is similar just need to make sure all the Relation columns have synonyms. -} -addViewRelations :: [Synonym] -> [Relation] -> [Relation] -addViewRelations allSyns = concatMap (\rel -> +addViewChildRelations :: [Synonym] -> [Relation] -> [Relation] +addViewChildRelations allSyns = concatMap (\rel -> rel : case rel of Relation{relType=Child, relTable, relColumns, relFTable, relFColumns} -> @@ -279,18 +279,22 @@ addViewRelations allSyns = concatMap (\rel -> fColsSyns = colSynsGroupedByView relFColumns getView :: [Synonym] -> Table getView = colTable . snd . unsafeHead - syns `allSynsOf` cols = S.fromList (fst <$> syns) == S.fromList cols in + syns `allSynsOf` cols = S.fromList (fst <$> syns) == S.fromList cols + -- Relation is dependent on the order of relColumns and relFColumns to get the join conditions right in the generated query. + -- So we need to change the order of the synonyms to match the relColumns + -- This could be avoided if the Relation type is improved with a structure that maintains the association of relColumns and relFColumns + syns `sortAccordingTo` columns = sortOn (\(k, _) -> L.lookup k $ zip columns [0::Int ..]) syns in - -- View Table Relations - [Relation (getView syns) (snd <$> syns) relFTable relFColumns Child Nothing Nothing Nothing + -- View Table Child Relations + [Relation (getView syns) (snd <$> syns `sortAccordingTo` relColumns) relFTable relFColumns Child Nothing Nothing Nothing | syns <- colsSyns, syns `allSynsOf` relColumns] ++ - -- Table View Relations - [Relation relTable relColumns (getView fSyns) (snd <$> fSyns) Child Nothing Nothing Nothing + -- Table View Child Relations + [Relation relTable relColumns (getView fSyns) (snd <$> fSyns `sortAccordingTo` relFColumns) Child Nothing Nothing Nothing | fSyns <- fColsSyns, fSyns `allSynsOf` relFColumns] ++ - -- View View Relations - [Relation (getView syns) (snd <$> syns) (getView fSyns) (snd <$> fSyns) Child Nothing Nothing Nothing + -- View View Child Relations + [Relation (getView syns) (snd <$> syns `sortAccordingTo` relColumns) (getView fSyns) (snd <$> fSyns `sortAccordingTo` relFColumns) Child Nothing Nothing Nothing | syns <- colsSyns, fSyns <- fColsSyns, syns `allSynsOf` relColumns, fSyns `allSynsOf` relFColumns] _ -> []) diff --git a/src/PostgREST/Types.hs b/src/PostgREST/Types.hs index 832207413..2a7c82b44 100644 --- a/src/PostgREST/Types.hs +++ b/src/PostgREST/Types.hs @@ -144,6 +144,9 @@ data RelationType = Child | Parent | Many | Root deriving (Show, Eq) The name 'Relation' here is used with the meaning "What is the relation between the current node and the parent node". It has nothing to do with PostgreSQL referring to tables/views as relations. + The order of the relColumns and relFColumns should be maintained to get + the join conditions right. + TODO merge relColumns and relFColumns to a tuple or Data.Bimap -} data Relation = Relation { relTable :: Table diff --git a/test/Feature/QuerySpec.hs b/test/Feature/QuerySpec.hs index f6d2a9124..ed50b200b 100644 --- a/test/Feature/QuerySpec.hs +++ b/test/Feature/QuerySpec.hs @@ -395,6 +395,36 @@ spec = do it "works when having a capitalized table name and camelCase fk column" $ get "/foos?select=*,bars(*)" `shouldRespondWith` 200 + it "works when embedding a view with a table that has a long compound pk" $ do + get "/player_view?select=id,contract(purchase_price)&id=in.(1,3,5,7)" `shouldRespondWith` + [json| + [{"id":1,"contract":[{"purchase_price":10}]}, + {"id":3,"contract":[{"purchase_price":30}]}, + {"id":5,"contract":[{"purchase_price":50}]}, + {"id":7,"contract":[]}] |] + { matchHeaders = [matchContentTypeJson] } + get "/contract?select=tournament,player_view(first_name)&limit=3" `shouldRespondWith` + [json| + [{"tournament":"tournament_1","player_view":{"first_name":"first_name_1"}}, + {"tournament":"tournament_2","player_view":{"first_name":"first_name_2"}}, + {"tournament":"tournament_3","player_view":{"first_name":"first_name_3"}}] |] + { matchHeaders = [matchContentTypeJson] } + + it "works when embedding a view with a view that referes to a table that has a long compound pk" $ do + get "/player_view?select=id,contract_view(purchase_price)&id=in.(1,3,5,7)" `shouldRespondWith` + [json| + [{"id":1,"contract_view":[{"purchase_price":10}]}, + {"id":3,"contract_view":[{"purchase_price":30}]}, + {"id":5,"contract_view":[{"purchase_price":50}]}, + {"id":7,"contract_view":[]}] |] + { matchHeaders = [matchContentTypeJson] } + get "/contract_view?select=tournament,player_view(first_name)&limit=3" `shouldRespondWith` + [json| + [{"tournament":"tournament_1","player_view":{"first_name":"first_name_1"}}, + {"tournament":"tournament_2","player_view":{"first_name":"first_name_2"}}, + {"tournament":"tournament_3","player_view":{"first_name":"first_name_3"}}] |] + { matchHeaders = [matchContentTypeJson] } + describe "path fixed" $ do it "works when requesting children 2 levels" $ get "/clients?id=eq.1&select=id,projects:projects.client_id(id,tasks(id))" `shouldRespondWith` diff --git a/test/fixtures/data.sql b/test/fixtures/data.sql index b32180833..db44ffd4e 100644 --- a/test/fixtures/data.sql +++ b/test/fixtures/data.sql @@ -250,7 +250,7 @@ INSERT INTO tsearch VALUES (to_tsvector('It''s kind of fun to do the impossible' INSERT INTO tsearch VALUES (to_tsvector('But also fun to do what is possible')); INSERT INTO tsearch VALUES (to_tsvector('Fat cats ate rats')); INSERT INTO tsearch VALUES (to_tsvector('french', 'C''est un peu amusant de faire l''impossible')); -INSERT INTO tsearch VALUES (to_tsvector('german', 'Es ist eine Art Spaß, das Unmögliche zu machen')); +INSERT INTO tsearch VALUES (to_tsvector('german', 'Es ist eine Art Spaß, das Unmögliche zu machen')); -- -- Data for Name: users_projects; Type: TABLE DATA; Schema: test; Owner: - @@ -442,3 +442,24 @@ INSERT INTO jsonb_test VALUES (1, '{ "a": {"b": 2} }'); INSERT INTO jsonb_test VALUES (2, '{ "c": [1,2,3] }'); INSERT INTO jsonb_test VALUES (3, '[{ "d": "test" }]'); INSERT INTO jsonb_test VALUES (4, '{ "e": 1 }'); + +TRUNCATE TABLE private.player CASCADE; +INSERT into private.player +SELECT + generate_series, + 'first_name_' || generate_series, + 'last_name_' || generate_series, + '2018-10-11' +FROM generate_series(1, 12); + +TRUNCATE TABLE contract CASCADE; +insert into contract +select + 'tournament_' || generate_series, + tsrange(now()::timestamp, null), + 10*generate_series, + generate_series, + 'first_name_' || generate_series, + 'last_name_' || generate_series, + '2018-10-11' +from generate_series(1, 6); diff --git a/test/fixtures/privileges.sql b/test/fixtures/privileges.sql index d3b8957f2..d2cd5d3cd 100644 --- a/test/fixtures/privileges.sql +++ b/test/fixtures/privileges.sql @@ -89,6 +89,9 @@ GRANT ALL ON TABLE , foos , bars , materialized_projects + , contract + , player_view + , contract_view TO postgrest_test_anonymous; GRANT INSERT ON TABLE insertonly TO postgrest_test_anonymous; diff --git a/test/fixtures/schema.sql b/test/fixtures/schema.sql index 2055d1ce2..9f2e6159b 100755 --- a/test/fixtures/schema.sql +++ b/test/fixtures/schema.sql @@ -1579,3 +1579,26 @@ create or replace function test."quotedFunction"("user" text, "fullName" text, " returns jsonb AS $$ select format('{"user": "%s", "fullName": "%s", "SSN": "%s"}', "user", "fullName", "SSN")::jsonb; $$ language sql; + +create table private.player ( + id integer not null, + first_name text not null, + last_name text not null, + birth_date date, + primary key (last_name, id, first_name, birth_date) -- just for testing a long compound pk +); + +create table test.contract ( + tournament text not null, + time tsrange not null, + purchase_price int not null, + id integer not null, + first_name text not null, + last_name text not null, + birth_date date, + foreign key (last_name, id, first_name, birth_date) references private.player +); + +create view test.player_view as select * from private.player; + +create view test.contract_view as select * from test.contract;