Skip to content

Commit

Permalink
fix json/jsonb columns as String values in nested returning of a muta…
Browse files Browse the repository at this point in the history
…tion (fix #3365) (#3375)
  • Loading branch information
rakeshkky authored and lexi-lambda committed Dec 10, 2019
1 parent f265ab1 commit 60acf7c
Show file tree
Hide file tree
Showing 15 changed files with 126 additions and 27 deletions.
10 changes: 6 additions & 4 deletions server/src-lib/Hasura/GraphQL/Resolve/Insert.hs
Original file line number Diff line number Diff line change
Expand Up @@ -244,15 +244,15 @@ 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
_ -> throw500 "more than one row returned"

fetchFromColVals
:: MonadError QErr m
=> ColVals
=> ColumnValues J.Value
-> [PGColumnInfo]
-> (PGColumnInfo -> a)
-> m [(a, WithScalarType PGScalarValue)]
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions server/src-lib/Hasura/RQL/DDL/Permission.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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" $
Expand Down Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion server/src-lib/Hasura/RQL/DDL/Schema/Rename.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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) ->
Expand Down
4 changes: 2 additions & 2 deletions server/src-lib/Hasura/RQL/DML/Internal.hs
Original file line number Diff line number Diff line change
Expand Up @@ -278,15 +278,15 @@ 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
, S.SEUnsafe "15" -- max decimal digits
, 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
Expand Down
22 changes: 15 additions & 7 deletions server/src-lib/Hasura/RQL/DML/Mutation.hs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ module Hasura.RQL.DML.Mutation
)
where

import Data.Aeson
import Hasura.Prelude

import qualified Data.HashMap.Strict as Map
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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]
Expand Down
8 changes: 4 additions & 4 deletions server/src-lib/Hasura/RQL/Types/Common.hs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ module Hasura.RQL.Types.Common

, ToAesonPairs(..)
, WithTable(..)
, ColVals
, ColumnValues
, MutateResp(..)
, ForeignKey(..)
, CustomColumnNames
Expand Down Expand Up @@ -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)

Expand Down
4 changes: 2 additions & 2 deletions server/src-lib/Hasura/RQL/Types/DML.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion server/src-lib/Hasura/SQL/Value.hs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ module Hasura.SQL.Value
, withConstructorFn
, parsePGValue

, TxtEncodedPGVal
, TxtEncodedPGVal(..)
, txtEncodedPGVal

, binEncoder
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -48,4 +53,3 @@ args:
foreign_key_constraint_on:
table: article
column: author_id

Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -20,6 +32,9 @@ query:
returning{
id
name
emails
info
location
articles{
id
title
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions server/tests-py/test_graphql_mutations.py
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,9 @@ def test_article_delete_returning(self, hge_ctx, transport):
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"
Expand Down

0 comments on commit 60acf7c

Please sign in to comment.