From b9fec2ce419d8f4263b97ff9134776efdebf3b73 Mon Sep 17 00:00:00 2001 From: Ruslan Talpa Date: Wed, 2 Sep 2015 11:03:34 +0300 Subject: [PATCH 1/6] Support for &select=col1,col2,col3 as suggested in issue #227 --- src/PostgREST/App.hs | 2 +- src/PostgREST/PgQuery.hs | 16 +++++++++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/PostgREST/App.hs b/src/PostgREST/App.hs index 4dae952ed..554c7cb20 100644 --- a/src/PostgREST/App.hs +++ b/src/PostgREST/App.hs @@ -72,7 +72,7 @@ app conf reqBody req = . limitT range . orderT (orderParse qq) . whereT qt qq - $ selectStar qt + $ selectT qt qq ) row <- H.maybeEx select let (tableTotal, queryTotal, body) = diff --git a/src/PostgREST/PgQuery.hs b/src/PostgREST/PgQuery.hs index b82d78015..bf3e57d5f 100644 --- a/src/PostgREST/PgQuery.hs +++ b/src/PostgREST/PgQuery.hs @@ -58,7 +58,7 @@ whereT table params q = then q else q <> B.Stmt " where " empty True <> conjunction where - cols = [ col | col <- params, fst col `notElem` ["order"] ] + cols = [ col | col <- params, fst col `notElem` ["order","select"] ] wherePredTable = wherePred table conjunction = mconcat $ L.intersperse andq (map wherePredTable cols) @@ -129,6 +129,20 @@ asJsonRow s = s { B.stmtTemplate = "row_to_json(t) from (" <> B.stmtTemplate s < selectStar :: QualifiedIdentifier -> PStmt selectStar t = B.Stmt ("select * from " <> fromQi t) empty True +selectT :: QualifiedIdentifier -> Net.Query -> PStmt +selectT table params = + if L.null cols + then selectStar table + else B.Stmt "select " empty True <> conjunction <> B.Stmt (" from " <> fromQi table ) empty True + where + selectTermTable = selectTerm table + conjunction = mconcat $ L.intersperse commaq (map selectTermTable cols) + columnsParam = fromMaybe "" $ join (lookup "select" params) + cols = filter ((>0) . T.length) $ map T.strip $ T.split (==',') $ cs columnsParam + +selectTerm :: QualifiedIdentifier -> T.Text -> PStmt +selectTerm table col = B.Stmt (pgFmtJsonbPath table (cs col)) empty True + returningStarT :: StatementT returningStarT s = s { B.stmtTemplate = B.stmtTemplate s <> " RETURNING *" } From 40d4fb0d759714c80d0120494f2f82e929a60ec5 Mon Sep 17 00:00:00 2001 From: Ruslan Talpa Date: Wed, 2 Sep 2015 12:41:02 +0300 Subject: [PATCH 2/6] Support for extracting fields from json columns --- src/PostgREST/PgQuery.hs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/PostgREST/PgQuery.hs b/src/PostgREST/PgQuery.hs index bf3e57d5f..a6d61be42 100644 --- a/src/PostgREST/PgQuery.hs +++ b/src/PostgREST/PgQuery.hs @@ -141,7 +141,11 @@ selectT table params = cols = filter ((>0) . T.length) $ map T.strip $ T.split (==',') $ cs columnsParam selectTerm :: QualifiedIdentifier -> T.Text -> PStmt -selectTerm table col = B.Stmt (pgFmtJsonbPath table (cs col)) empty True +selectTerm table col = B.Stmt (pgFmtJsonbPath table (cs col) <> asT jsonbPath) empty True + where + jsonbPath = parseJsonbPath $ cs col + asT (Just (DoubleArrow _ (KeyIdentifier key))) = " AS " <> pgFmtIdent key + asT _ = "" returningStarT :: StatementT returningStarT s = s { B.stmtTemplate = B.stmtTemplate s <> " RETURNING *" } From 6c16395dfb9903c569005f034482f338965317b3 Mon Sep 17 00:00:00 2001 From: Ruslan Talpa Date: Fri, 4 Sep 2015 19:04:39 +0300 Subject: [PATCH 3/6] change fn name from selectT to select --- src/PostgREST/App.hs | 6 +++--- src/PostgREST/PgQuery.hs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/PostgREST/App.hs b/src/PostgREST/App.hs index 554c7cb20..05bf8e822 100644 --- a/src/PostgREST/App.hs +++ b/src/PostgREST/App.hs @@ -64,7 +64,7 @@ app conf reqBody req = else do let qt = qualify table from = fromMaybe 0 $ rangeOffset <$> range - select = B.Stmt "select " V.empty True <> + query = B.Stmt "select " V.empty True <> parentheticT ( whereT qt qq $ countRows qt ) <> commaq <> ( @@ -72,9 +72,9 @@ app conf reqBody req = . limitT range . orderT (orderParse qq) . whereT qt qq - $ selectT qt qq + $ select qt qq ) - row <- H.maybeEx select + row <- H.maybeEx query let (tableTotal, queryTotal, body) = fromMaybe (0, 0, Just "" :: Maybe Text) row to = from+queryTotal-1 diff --git a/src/PostgREST/PgQuery.hs b/src/PostgREST/PgQuery.hs index a6d61be42..ecac65768 100644 --- a/src/PostgREST/PgQuery.hs +++ b/src/PostgREST/PgQuery.hs @@ -129,8 +129,8 @@ asJsonRow s = s { B.stmtTemplate = "row_to_json(t) from (" <> B.stmtTemplate s < selectStar :: QualifiedIdentifier -> PStmt selectStar t = B.Stmt ("select * from " <> fromQi t) empty True -selectT :: QualifiedIdentifier -> Net.Query -> PStmt -selectT table params = +select :: QualifiedIdentifier -> Net.Query -> PStmt +select table params = if L.null cols then selectStar table else B.Stmt "select " empty True <> conjunction <> B.Stmt (" from " <> fromQi table ) empty True From 9c8ac2a48983d057bd7e50a4bd864f2d290b5a3c Mon Sep 17 00:00:00 2001 From: Ruslan Talpa Date: Sat, 5 Sep 2015 15:05:53 +0300 Subject: [PATCH 4/6] tests for &select= feature and support for casting columns (usefull when extracting subfields from json columns) --- src/PostgREST/PgQuery.hs | 8 ++++++-- test/Feature/QuerySpec.hs | 34 +++++++++++++++++++++++++++++++++- test/Feature/StructureSpec.hs | 1 + test/SpecHelper.hs | 13 +++++++++++++ test/fixtures/schema.sql | 28 ++++++++++++++++++++++------ 5 files changed, 75 insertions(+), 9 deletions(-) diff --git a/src/PostgREST/PgQuery.hs b/src/PostgREST/PgQuery.hs index ecac65768..450a13eaf 100644 --- a/src/PostgREST/PgQuery.hs +++ b/src/PostgREST/PgQuery.hs @@ -141,9 +141,13 @@ select table params = cols = filter ((>0) . T.length) $ map T.strip $ T.split (==',') $ cs columnsParam selectTerm :: QualifiedIdentifier -> T.Text -> PStmt -selectTerm table col = B.Stmt (pgFmtJsonbPath table (cs col) <> asT jsonbPath) empty True +selectTerm table col = + case T.splitOn "::" col of + [colName,castTo] -> B.Stmt ("CAST (" <> pgFmtJsonbPath table (cs colName) <> " AS " <> castTo <> " )" <> asT (jsonbPath colName)) empty True + _-> B.Stmt (pgFmtJsonbPath table (cs col) <> asT (jsonbPath col)) empty True where - jsonbPath = parseJsonbPath $ cs col + jsonbPath :: T.Text -> Maybe JsonbPath + jsonbPath c = parseJsonbPath $ cs c asT (Just (DoubleArrow _ (KeyIdentifier key))) = " AS " <> pgFmtIdent key asT _ = "" diff --git a/test/Feature/QuerySpec.hs b/test/Feature/QuerySpec.hs index 8de67bcee..e351b6c4c 100644 --- a/test/Feature/QuerySpec.hs +++ b/test/Feature/QuerySpec.hs @@ -11,13 +11,14 @@ import SpecHelper spec :: Spec spec = beforeAll (clearTable "items" >> createItems 15) + . beforeAll (clearTable "complex_items" >> createComplexItems) . beforeAll (clearTable "nullable_integer" >> createNullInteger) . beforeAll ( clearTable "no_pk" >> createNulls 2 >> createLikableStrings >> createJsonData) - . afterAll_ (clearTable "items" >> clearTable "no_pk" >> clearTable "simple_pk") + . afterAll_ (clearTable "items" >> clearTable "complex_items" >> clearTable "no_pk" >> clearTable "simple_pk") . around withApp $ do describe "Querying a table with a column called count" $ @@ -129,6 +130,37 @@ spec = get "/items?always_true=eq.true" `shouldRespondWith` [json| [{"id":1},{"id":2},{"id":3},{"id":4},{"id":5},{"id":6},{"id":7},{"id":8},{"id":9},{"id":10},{"id":11},{"id":12},{"id":13},{"id":14},{"id":15}] |] + describe "Shaping response with select parameter" $ do + + it "selectStar works in absense of parameter" $ + get "/complex_items?id=eq.3" `shouldRespondWith` + "[{\"id\":3,\"name\":\"Three\",\"settings\":{\"foo\":{\"int\":1,\"bar\":\"baz\"}}}]" + + it "one simple column" $ + get "/complex_items?select=id" `shouldRespondWith` + [json| [{"id":1},{"id":2},{"id":3}] |] + + it "one simple column with casting (text)" $ + get "/complex_items?select=id::text" `shouldRespondWith` + [json| [{"id":"1"},{"id":"2"},{"id":"3"}] |] + + it "json column" $ + get "/complex_items?id=eq.1&select=settings" `shouldRespondWith` + [json| [{"settings":{"foo":{"int":1,"bar":"baz"}}}] |] + + it "json subfield one level with casting (json)" $ + get "/complex_items?id=eq.1&select=settings->>foo::json" `shouldRespondWith` + [json| [{"foo":{"int":1,"bar":"baz"}}] |] -- the value of foo here is of type "text" + + it "json subfield two levels (string)" $ + get "/complex_items?id=eq.1&select=settings->foo->>bar" `shouldRespondWith` + [json| [{"bar":"baz"}] |] + + it "json subfield two levels with casting (int)" $ + get "/complex_items?id=eq.1&select=settings->foo->>int::integer" `shouldRespondWith` + [json| [{"int":1}] |] -- the value in the db is an int, but here we expect a string for now + + describe "ordering response" $ do it "by a column asc" $ get "/items?id=lte.2&order=id.asc" diff --git a/test/Feature/StructureSpec.hs b/test/Feature/StructureSpec.hs index 867361360..f63628e8a 100644 --- a/test/Feature/StructureSpec.hs +++ b/test/Feature/StructureSpec.hs @@ -15,6 +15,7 @@ spec = around withApp $ do request methodGet "/" [] "" `shouldRespondWith` [json| [ {"schema":"1","name":"auto_incrementing_pk","insertable":true} + , {"schema":"1","name":"complex_items","insertable":true} , {"schema":"1","name":"compound_pk","insertable":true} , {"schema":"1","name":"has_count_column","insertable":false} , {"schema":"1","name":"has_fk","insertable":true} diff --git a/test/SpecHelper.hs b/test/SpecHelper.hs index e5ae89db8..f198cf7dc 100644 --- a/test/SpecHelper.hs +++ b/test/SpecHelper.hs @@ -13,6 +13,7 @@ import Data.Monoid import Data.Text hiding (map) import qualified Data.Vector as V import Control.Monad (void) +import Control.Applicative import Network.HTTP.Types.Header (Header, ByteRange, renderByteRange, hRange, hAuthorization, hAccept) @@ -118,6 +119,18 @@ createItems n = do txn = mapM_ H.unitEx stmts stmts = map [H.stmt|insert into "1".items (id) values (?)|] [1..n] +createComplexItems :: IO () +createComplexItems = do + pool <- testPool + void . liftIO $ H.session pool $ H.tx Nothing txn + where + txn = mapM_ H.unitEx stmts + stmts = getZipList $ [H.stmt|insert into "1".complex_items (id, name, settings) values (?,?,?)|] + <$> ZipList ([1..3]::[Int]) + <*> ZipList (["One", "Two", "Three"]::[Text]) + <*> ZipList ([jobj,jobj,jobj]) + jobj = (J.object [("foo", J.object [("int", J.Number 1),("bar", J.String "baz")])]) + createNulls :: Int -> IO () createNulls n = do pool <- testPool diff --git a/test/fixtures/schema.sql b/test/fixtures/schema.sql index 6796d2167..0f5cb5068 100755 --- a/test/fixtures/schema.sql +++ b/test/fixtures/schema.sql @@ -47,7 +47,7 @@ SET search_path = postgrest, pg_catalog; CREATE FUNCTION check_role_exists() RETURNS trigger LANGUAGE plpgsql AS $$ -begin +begin if not exists (select 1 from pg_roles as r where r.rolname = new.rolname) then raise foreign_key_violation using message = 'Cannot create user with unknown role: ' || new.rolname; return null; @@ -64,7 +64,7 @@ CREATE FUNCTION update_owner() RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN - NEW.owner = current_user; + NEW.owner = current_user; RETURN NEW; END; $$; @@ -75,8 +75,8 @@ ALTER FUNCTION postgrest.update_owner() OWNER TO postgrest_test; CREATE FUNCTION set_authors_only_owner() RETURNS trigger LANGUAGE plpgsql AS $$ -begin - NEW.owner = current_setting('user_vars.user_id'); +begin + NEW.owner = current_setting('user_vars.user_id'); RETURN NEW; end $$; @@ -170,7 +170,7 @@ ALTER TABLE "1".has_fk_id_seq OWNER TO postgrest_test; ALTER SEQUENCE has_fk_id_seq OWNED BY has_fk.id; CREATE MATERIALIZED VIEW "1".materialized_view AS - SELECT + SELECT version(); ALTER TABLE "1".materialized_view OWNER TO postgrest_test; @@ -201,6 +201,15 @@ CREATE TABLE items ( ALTER TABLE "1".items OWNER TO postgrest_test; +CREATE TABLE complex_items ( + id bigint NOT NULL, + name text, + settings json +); + + +ALTER TABLE "1".complex_items OWNER TO postgrest_test; + CREATE SEQUENCE items_id_seq START WITH 1 @@ -407,7 +416,7 @@ ALTER FUNCTION public.always_true("1".items) OWNER TO postgrest_test; ALTER TABLE ONLY authors_only ADD CONSTRAINT authors_only_pkey PRIMARY KEY (secret); - + CREATE TRIGGER insert_insertable_view_with_join INSTEAD OF INSERT ON "1".insertable_view_with_join FOR EACH ROW EXECUTE PROCEDURE "1".insert_insertable_view_with_join(); @@ -437,6 +446,8 @@ ALTER TABLE ONLY has_fk ALTER TABLE ONLY items ADD CONSTRAINT items_pkey PRIMARY KEY (id); +ALTER TABLE ONLY complex_items + ADD CONSTRAINT complex_items_pkey PRIMARY KEY (id); ALTER TABLE ONLY menagerie @@ -539,6 +550,11 @@ REVOKE ALL ON TABLE items FROM postgrest_test; GRANT ALL ON TABLE items TO postgrest_test; GRANT ALL ON TABLE items TO postgrest_anonymous; +REVOKE ALL ON TABLE complex_items FROM PUBLIC; +REVOKE ALL ON TABLE complex_items FROM postgrest_test; +GRANT ALL ON TABLE complex_items TO postgrest_test; +GRANT ALL ON TABLE complex_items TO postgrest_anonymous; + REVOKE ALL ON FUNCTION getitemrange(bigint, bigint) FROM PUBLIC; REVOKE ALL ON FUNCTION getitemrange(bigint, bigint) FROM postgrest_test; From cc418d35193d1d58c61cd4b2721791e0844034db Mon Sep 17 00:00:00 2001 From: Ruslan Talpa Date: Sat, 5 Sep 2015 15:31:36 +0300 Subject: [PATCH 5/6] aditional tests for bad casting parameters --- test/Feature/QuerySpec.hs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/Feature/QuerySpec.hs b/test/Feature/QuerySpec.hs index e351b6c4c..b1547bc81 100644 --- a/test/Feature/QuerySpec.hs +++ b/test/Feature/QuerySpec.hs @@ -152,10 +152,28 @@ spec = get "/complex_items?id=eq.1&select=settings->>foo::json" `shouldRespondWith` [json| [{"foo":{"int":1,"bar":"baz"}}] |] -- the value of foo here is of type "text" + it "fails on bad casting (data of the wrong format)" $ + get "/complex_items?select=settings->foo->>bar::integer" + `shouldRespondWith` ResponseMatcher { + matchBody = Just [json| {"hint":null,"details":null,"code":"22P02","message":"invalid input syntax for integer: \"baz\""} |] + , matchStatus = 400 + , matchHeaders = [] + } + + it "fails on bad casting (wrong cast type)" $ + get "/complex_items?select=id::fakecolumntype" + `shouldRespondWith` ResponseMatcher { + matchBody = Just [json| {"hint":null,"details":null,"code":"42704","message":"type \"fakecolumntype\" does not exist"} |] + , matchStatus = 400 + , matchHeaders = [] + } + + it "json subfield two levels (string)" $ get "/complex_items?id=eq.1&select=settings->foo->>bar" `shouldRespondWith` [json| [{"bar":"baz"}] |] + it "json subfield two levels with casting (int)" $ get "/complex_items?id=eq.1&select=settings->foo->>int::integer" `shouldRespondWith` [json| [{"int":1}] |] -- the value in the db is an int, but here we expect a string for now From 92a1d8c7e375a4c8da0f3abfe937cb640295e8c2 Mon Sep 17 00:00:00 2001 From: Ruslan Talpa Date: Sat, 5 Sep 2015 16:54:09 +0300 Subject: [PATCH 6/6] constrain cast parameters to letters only --- src/PostgREST/PgQuery.hs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/PostgREST/PgQuery.hs b/src/PostgREST/PgQuery.hs index 450a13eaf..19a0cca34 100644 --- a/src/PostgREST/PgQuery.hs +++ b/src/PostgREST/PgQuery.hs @@ -143,7 +143,8 @@ select table params = selectTerm :: QualifiedIdentifier -> T.Text -> PStmt selectTerm table col = case T.splitOn "::" col of - [colName,castTo] -> B.Stmt ("CAST (" <> pgFmtJsonbPath table (cs colName) <> " AS " <> castTo <> " )" <> asT (jsonbPath colName)) empty True + [colName,castTo] -> B.Stmt ("CAST (" <> pgFmtJsonbPath table (cs colName) <> " AS " <> castToSafe <> " )" <> asT (jsonbPath colName)) empty True + where castToSafe = T.filter ( `elem` ['a'..'z'] ) castTo _-> B.Stmt (pgFmtJsonbPath table (cs col) <> asT (jsonbPath col)) empty True where jsonbPath :: T.Text -> Maybe JsonbPath