fix json/jsonb columns as String values in nested returning of a mutation (fix #3365) (#3375)

2024-12-14 17:02:49 +03:00 · 2019-12-11 04:50:55 +05:30 · 2019-12-11 04:50:55 +05:30 · 60acf7c081
commit 60acf7c081
parent f265ab1060
15 changed files with 126 additions and 27 deletions
--- 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
--- 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)

--- 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) ->
--- 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
--- 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]
--- 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)

--- 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
--- 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
--- 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
@ -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
+          }
+        }
+      }
+    }
--- 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
-
--- 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
--- 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
--- 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:
--- 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
--- 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"