From 60acf7c081bd47394e1ec8a54b47fa76f326e8cd Mon Sep 17 00:00:00 2001 From: Rakesh Emmadi <12475069+rakeshkky@users.noreply.github.com> Date: Wed, 11 Dec 2019 04:50:55 +0530 Subject: [PATCH] fix json/jsonb columns as String values in nested returning of a mutation (fix #3365) (#3375) --- .../src-lib/Hasura/GraphQL/Resolve/Insert.hs | 10 +++-- server/src-lib/Hasura/RQL/DDL/Permission.hs | 6 +-- .../src-lib/Hasura/RQL/DDL/Schema/Rename.hs | 2 +- server/src-lib/Hasura/RQL/DML/Internal.hs | 4 +- server/src-lib/Hasura/RQL/DML/Mutation.hs | 22 ++++++---- server/src-lib/Hasura/RQL/Types/Common.hs | 8 ++-- server/src-lib/Hasura/RQL/Types/DML.hs | 4 +- server/src-lib/Hasura/SQL/Value.hs | 7 +++- .../author_returning_empty_articles.yaml | 42 +++++++++++++++++++ .../delete/basic/schema_setup.yaml | 8 +++- .../delete/basic/values_setup.yaml | 8 ++++ .../update/basic/author_set_name.yaml | 15 +++++++ .../update/basic/schema_setup.yaml | 7 +++- .../update/basic/values_setup.yaml | 7 ++++ server/tests-py/test_graphql_mutations.py | 3 ++ 15 files changed, 126 insertions(+), 27 deletions(-) create mode 100644 server/tests-py/queries/graphql_mutation/delete/basic/author_returning_empty_articles.yaml diff --git a/server/src-lib/Hasura/GraphQL/Resolve/Insert.hs b/server/src-lib/Hasura/GraphQL/Resolve/Insert.hs index d339abe8..2ae42874 100644 --- a/server/src-lib/Hasura/GraphQL/Resolve/Insert.hs +++ b/server/src-lib/Hasura/GraphQL/Resolve/Insert.hs @@ -244,7 +244,7 @@ mkInsertQ vn onConflictM insCols defVals role = do asSingleObject :: MonadError QErr m - => [ColVals] -> m (Maybe ColVals) + => [ColumnValues J.Value] -> m (Maybe (ColumnValues J.Value)) asSingleObject = \case [] -> return Nothing [a] -> return $ Just a @@ -252,7 +252,7 @@ asSingleObject = \case fetchFromColVals :: MonadError QErr m - => ColVals + => ColumnValues J.Value -> [PGColumnInfo] -> (PGColumnInfo -> a) -> m [(a, WithScalarType PGScalarValue)] @@ -268,11 +268,13 @@ mkSelCTE :: MonadError QErr m => QualifiedTable -> [PGColumnInfo] - -> Maybe ColVals + -> Maybe (ColumnValues J.Value) -> m CTEExp mkSelCTE tn allCols colValM = do - selCTE <- mkSelCTEFromColVals tn allCols $ maybe [] pure colValM + selCTE <- mkSelCTEFromColVals parseFn tn allCols $ maybe [] pure colValM return $ CTEExp selCTE Seq.Empty + where + parseFn ty val = toTxtValue <$> parsePGScalarValue ty val execCTEExp :: Bool diff --git a/server/src-lib/Hasura/RQL/DDL/Permission.hs b/server/src-lib/Hasura/RQL/DDL/Permission.hs index 170c57bd..3acbf1f7 100644 --- a/server/src-lib/Hasura/RQL/DDL/Permission.hs +++ b/server/src-lib/Hasura/RQL/DDL/Permission.hs @@ -75,7 +75,7 @@ import qualified Data.Text as T data InsPerm = InsPerm { ipCheck :: !BoolExp - , ipSet :: !(Maybe ColVals) + , ipSet :: !(Maybe (ColumnValues Value)) , ipColumns :: !(Maybe PermColSpec) } deriving (Show, Eq, Lift) @@ -111,7 +111,7 @@ dropView vn = procSetObj :: (QErrM m) - => TableInfo PGColumnInfo -> Maybe ColVals + => TableInfo PGColumnInfo -> Maybe (ColumnValues Value) -> m (PreSetColsPartial, [Text], [SchemaDependency]) procSetObj ti mObj = do (setColTups, deps) <- withPathK "set" $ @@ -298,7 +298,7 @@ instance IsPerm SelPerm where data UpdPerm = UpdPerm { ucColumns :: !PermColSpec -- Allowed columns - , ucSet :: !(Maybe ColVals) -- Preset columns + , ucSet :: !(Maybe (ColumnValues Value)) -- Preset columns , ucFilter :: !BoolExp -- Filter expression } deriving (Show, Eq, Lift) diff --git a/server/src-lib/Hasura/RQL/DDL/Schema/Rename.hs b/server/src-lib/Hasura/RQL/DDL/Schema/Rename.hs index 35cd9e0c..5c9de8a3 100644 --- a/server/src-lib/Hasura/RQL/DDL/Schema/Rename.hs +++ b/server/src-lib/Hasura/RQL/DDL/Schema/Rename.hs @@ -251,7 +251,7 @@ updateDelPermFlds refQT rename rn (DelPerm fltr) = do liftTx $ updatePermDefInCatalog PTDelete refQT rn $ DelPerm updFltr updatePreset - :: QualifiedTable -> RenameField -> ColVals -> ColVals + :: QualifiedTable -> RenameField -> (ColumnValues Value) -> (ColumnValues Value) updatePreset qt rf obj = case rf of RFCol (RenameItem opQT oCol nCol) -> diff --git a/server/src-lib/Hasura/RQL/DML/Internal.hs b/server/src-lib/Hasura/RQL/DML/Internal.hs index 7c5e36ad..e56d0fc7 100644 --- a/server/src-lib/Hasura/RQL/DML/Internal.hs +++ b/server/src-lib/Hasura/RQL/DML/Internal.hs @@ -278,6 +278,8 @@ dmlTxErrorHandler = mkTxErrorHandler $ \case toJSONableExp :: Bool -> PGColumnType -> Bool -> S.SQLExp -> S.SQLExp toJSONableExp strfyNum colTy asText expn + | asText || (isScalarColumnWhere isBigNum colTy && strfyNum) = + expn `S.SETyAnn` S.textTypeAnn | isScalarColumnWhere isGeoType colTy = S.SEFnApp "ST_AsGeoJSON" [ expn @@ -285,8 +287,6 @@ toJSONableExp strfyNum colTy asText expn , S.SEUnsafe "4" -- to print out crs ] Nothing `S.SETyAnn` S.jsonTypeAnn - | asText || (isScalarColumnWhere isBigNum colTy && strfyNum) = - expn `S.SETyAnn` S.textTypeAnn | otherwise = expn -- validate headers diff --git a/server/src-lib/Hasura/RQL/DML/Mutation.hs b/server/src-lib/Hasura/RQL/DML/Mutation.hs index 4a1613cf..8ce47fef 100644 --- a/server/src-lib/Hasura/RQL/DML/Mutation.hs +++ b/server/src-lib/Hasura/RQL/DML/Mutation.hs @@ -6,6 +6,7 @@ module Hasura.RQL.DML.Mutation ) where +import Data.Aeson import Hasura.Prelude import qualified Data.HashMap.Strict as Map @@ -48,20 +49,26 @@ mutateAndReturn (Mutation qt (cte, p) mutFlds _ strfyNum) = mutateAndSel :: Mutation -> Q.TxE QErr EncJSON mutateAndSel (Mutation qt q mutFlds allCols strfyNum) = do -- Perform mutation and fetch unique columns - MutateResp _ colVals <- mutateAndFetchCols qt allCols q strfyNum - selCTE <- mkSelCTEFromColVals qt allCols colVals + MutateResp _ columnVals <- mutateAndFetchCols qt allCols q strfyNum + selCTE <- mkSelCTEFromColVals txtEncodedToSQLExp qt allCols columnVals let selWith = mkSelWith qt selCTE mutFlds False strfyNum -- Perform select query and fetch returning fields encJFromBS . runIdentity . Q.getRow <$> Q.rawQE dmlTxErrorHandler (Q.fromBuilder $ toSQL selWith) [] True + where + txtEncodedToSQLExp colTy = pure . \case + TENull -> S.SENull + TELit textValue -> + S.withTyAnn (unsafePGColumnToRepresentation colTy) $ S.SELit textValue mutateAndFetchCols - :: QualifiedTable + :: (FromJSON a) + => QualifiedTable -> [PGColumnInfo] -> (S.CTE, DS.Seq Q.PrepArg) -> Bool - -> Q.TxE QErr MutateResp + -> Q.TxE QErr (MutateResp a) mutateAndFetchCols qt cols (cte, p) strfyNum = Q.getAltJ . runIdentity . Q.getRow <$> Q.rawQE dmlTxErrorHandler (Q.fromBuilder sql) (toList p) True @@ -90,8 +97,9 @@ mutateAndFetchCols qt cols (cte, p) strfyNum = mkSelCTEFromColVals :: (MonadError QErr m) - => QualifiedTable -> [PGColumnInfo] -> [ColVals] -> m S.CTE -mkSelCTEFromColVals qt allCols colVals = + => (PGColumnType -> a -> m S.SQLExp) + -> QualifiedTable -> [PGColumnInfo] -> [ColumnValues a] -> m S.CTE +mkSelCTEFromColVals parseFn qt allCols colVals = S.CTESelect <$> case colVals of [] -> return selNoRows _ -> do @@ -109,7 +117,7 @@ mkSelCTEFromColVals qt allCols colVals = let pgCol = pgiColumn ci val <- onNothing (Map.lookup pgCol colVal) $ throw500 $ "column " <> pgCol <<> " not found in returning values" - toTxtValue <$> parsePGScalarValue (pgiType ci) val + parseFn (pgiType ci) val selNoRows = S.mkSelect { S.selExtr = [S.selectStar] diff --git a/server/src-lib/Hasura/RQL/Types/Common.hs b/server/src-lib/Hasura/RQL/Types/Common.hs index b3a755fe..723cb329 100644 --- a/server/src-lib/Hasura/RQL/Types/Common.hs +++ b/server/src-lib/Hasura/RQL/Types/Common.hs @@ -12,7 +12,7 @@ module Hasura.RQL.Types.Common , ToAesonPairs(..) , WithTable(..) - , ColVals + , ColumnValues , MutateResp(..) , ForeignKey(..) , CustomColumnNames @@ -158,12 +158,12 @@ instance (ToAesonPairs a) => ToJSON (WithTable a) where toJSON (WithTable tn rel) = object $ ("table" .= tn):toAesonPairs rel -type ColVals = HM.HashMap PGCol Value +type ColumnValues a = HM.HashMap PGCol a -data MutateResp +data MutateResp a = MutateResp { _mrAffectedRows :: !Int - , _mrReturningColumns :: ![ColVals] + , _mrReturningColumns :: ![ColumnValues a] } deriving (Show, Eq) $(deriveJSON (aesonDrop 3 snakeCase) ''MutateResp) diff --git a/server/src-lib/Hasura/RQL/Types/DML.hs b/server/src-lib/Hasura/RQL/Types/DML.hs index cd9cf443..a59dffa1 100644 --- a/server/src-lib/Hasura/RQL/Types/DML.hs +++ b/server/src-lib/Hasura/RQL/Types/DML.hs @@ -298,7 +298,7 @@ instance ToJSON SelectQueryT where toJSON (DMLQuery qt selQ) = object $ "table" .= qt : selectGToPairs selQ -type InsObj = ColVals +type InsObj = ColumnValues Value data ConflictAction = CAIgnore @@ -353,7 +353,7 @@ data InsertTxConflictCtx } deriving (Show, Eq) $(deriveJSON (aesonDrop 3 snakeCase){omitNothingFields=True} ''InsertTxConflictCtx) -type UpdVals = ColVals +type UpdVals = ColumnValues Value data UpdateQuery = UpdateQuery diff --git a/server/src-lib/Hasura/SQL/Value.hs b/server/src-lib/Hasura/SQL/Value.hs index 794f28d1..c2fc3da9 100644 --- a/server/src-lib/Hasura/SQL/Value.hs +++ b/server/src-lib/Hasura/SQL/Value.hs @@ -4,7 +4,7 @@ module Hasura.SQL.Value , withConstructorFn , parsePGValue - , TxtEncodedPGVal + , TxtEncodedPGVal(..) , txtEncodedPGVal , binEncoder @@ -130,6 +130,11 @@ instance ToJSON TxtEncodedPGVal where TENull -> Null TELit t -> String t +instance FromJSON TxtEncodedPGVal where + parseJSON Null = pure TENull + parseJSON (String t) = pure $ TELit t + parseJSON v = AT.typeMismatch "String" v + txtEncodedPGVal :: PGScalarValue -> TxtEncodedPGVal txtEncodedPGVal colVal = case colVal of PGValInteger i -> TELit $ T.pack $ show i diff --git a/server/tests-py/queries/graphql_mutation/delete/basic/author_returning_empty_articles.yaml b/server/tests-py/queries/graphql_mutation/delete/basic/author_returning_empty_articles.yaml new file mode 100644 index 00000000..780a128a --- /dev/null +++ b/server/tests-py/queries/graphql_mutation/delete/basic/author_returning_empty_articles.yaml @@ -0,0 +1,42 @@ +description: Delete mutation on author with returning articles +url: /v1/graphql +status: 200 +response: + data: + delete_author: + affected_rows: 1 + returning: + - id: 3 + name: Author 3 + emails: [] + info: + age: 23 + location: + type: Point + crs: + type: name + properties: + name: urn:ogc:def:crs:EPSG::4326 + coordinates: + - -118.4079 + - 33.9434 + articles: [] +query: + query: | + mutation DeleteAuthor3 { + delete_author(where: {id: {_eq: 3}}){ + affected_rows + returning{ + id + name + emails + info + location + articles{ + id + title + content + } + } + } + } diff --git a/server/tests-py/queries/graphql_mutation/delete/basic/schema_setup.yaml b/server/tests-py/queries/graphql_mutation/delete/basic/schema_setup.yaml index f612d4e7..3f4265f2 100644 --- a/server/tests-py/queries/graphql_mutation/delete/basic/schema_setup.yaml +++ b/server/tests-py/queries/graphql_mutation/delete/basic/schema_setup.yaml @@ -5,9 +5,14 @@ args: - type: run_sql args: sql: | + CREATE EXTENSION IF NOT EXISTS postgis; + CREATE EXTENSION IF NOT EXISTS postgis_topology; create table author( id serial primary key, - name text unique + name text unique, + emails text[] not null default '{}'::text[], + info jsonb, + location geography ); - type: track_table args: @@ -48,4 +53,3 @@ args: foreign_key_constraint_on: table: article column: author_id - diff --git a/server/tests-py/queries/graphql_mutation/delete/basic/values_setup.yaml b/server/tests-py/queries/graphql_mutation/delete/basic/values_setup.yaml index 0274112d..8fadb4ae 100644 --- a/server/tests-py/queries/graphql_mutation/delete/basic/values_setup.yaml +++ b/server/tests-py/queries/graphql_mutation/delete/basic/values_setup.yaml @@ -8,6 +8,14 @@ args: objects: - name: Author 1 - name: Author 2 + - name: Author 3 + info: + age: 23 + location: + type: Point + coordinates: + - -118.4079 + - 33.9434 #Insert aticle table data - type: insert diff --git a/server/tests-py/queries/graphql_mutation/update/basic/author_set_name.yaml b/server/tests-py/queries/graphql_mutation/update/basic/author_set_name.yaml index 3b06911a..56cacec3 100644 --- a/server/tests-py/queries/graphql_mutation/update/basic/author_set_name.yaml +++ b/server/tests-py/queries/graphql_mutation/update/basic/author_set_name.yaml @@ -7,6 +7,18 @@ response: returning: - id: 1 name: Jane + emails: [] + info: + age: 23 + location: + type: Point + crs: + type: name + properties: + name: urn:ogc:def:crs:EPSG::4326 + coordinates: + - -118.4079 + - 33.9434 articles: [] status: 200 query: @@ -20,6 +32,9 @@ query: returning{ id name + emails + info + location articles{ id title diff --git a/server/tests-py/queries/graphql_mutation/update/basic/schema_setup.yaml b/server/tests-py/queries/graphql_mutation/update/basic/schema_setup.yaml index 77ca2530..6826f77e 100644 --- a/server/tests-py/queries/graphql_mutation/update/basic/schema_setup.yaml +++ b/server/tests-py/queries/graphql_mutation/update/basic/schema_setup.yaml @@ -5,9 +5,14 @@ args: - type: run_sql args: sql: | + CREATE EXTENSION IF NOT EXISTS postgis; + CREATE EXTENSION IF NOT EXISTS postgis_topology; create table author( id serial primary key, - name text unique + name text unique, + emails text[] not null default '{}'::text[], + info jsonb, + location geography ); - type: track_table args: diff --git a/server/tests-py/queries/graphql_mutation/update/basic/values_setup.yaml b/server/tests-py/queries/graphql_mutation/update/basic/values_setup.yaml index 8cdce0b7..4519dd53 100644 --- a/server/tests-py/queries/graphql_mutation/update/basic/values_setup.yaml +++ b/server/tests-py/queries/graphql_mutation/update/basic/values_setup.yaml @@ -7,6 +7,13 @@ args: table: author objects: - name: Author 1 + info: + age: 23 + location: + type: Point + coordinates: + - -118.4079 + - 33.9434 - name: Author 2 #Insert Person table data diff --git a/server/tests-py/test_graphql_mutations.py b/server/tests-py/test_graphql_mutations.py index 03e285fb..506d1253 100644 --- a/server/tests-py/test_graphql_mutations.py +++ b/server/tests-py/test_graphql_mutations.py @@ -371,6 +371,9 @@ class TestGraphqlDeleteBasic(DefaultTestMutations): def test_article_delete_returning_author(self, hge_ctx, transport): check_query_f(hge_ctx, self.dir() + "/article_returning_author.yaml", transport) + def test_author_returning_empty_articles(self, hge_ctx, transport): + check_query_f(hge_ctx, self.dir() + "/author_returning_empty_articles.yaml", transport) + @classmethod def dir(cls): return "queries/graphql_mutation/delete/basic"