From 5bafdce9a3edf7ebb2198a26b24d535134bfd458 Mon Sep 17 00:00:00 2001 From: Rakesh Emmadi Date: Fri, 22 Mar 2019 12:38:42 +0530 Subject: [PATCH] fix delete mutation returning incorrect data (fix #1794) (fix #1763) (#1827) From `alpha-40` we've been using a `WHERE` clause to fetch required rows and generate mutation response. This has a few limitations like the requirement of a primary key/unique constraint. This also returns inconsistent data on `delete` mutation as mentioned in #1794. Now, we're using `VALUES (..)` (refer [here](https://www.postgresql.org/docs/current/sql-values.html)) expression to form virtual table rows in `SQL` to generate mutation response. Internal changes:- - Not to use primary key/unique constraint columns:- - Revert back to `ConstraintName` from `TableConstraint` in `TableInfo` type - Remove `tcCols` field in `TableConstraint` type - Modify `table_info.sql` and `fetchTableMeta` function `SQL` - A test case to perform `delete` mutation and returning relational objects. --- .../graphql/manual/api-reference/mutation.rst | 66 +------------ docs/graphql/manual/mutations/delete.rst | 6 +- docs/graphql/manual/mutations/insert.rst | 15 +-- docs/graphql/manual/mutations/update.rst | 3 +- docs/graphql/manual/mutations/upsert.rst | 4 - server/src-lib/Hasura/GraphQL/Context.hs | 10 +- .../Hasura/GraphQL/Resolve/ContextTypes.hs | 1 - .../src-lib/Hasura/GraphQL/Resolve/Insert.hs | 67 ++++--------- .../Hasura/GraphQL/Resolve/Mutation.hs | 10 +- server/src-lib/Hasura/GraphQL/Schema.hs | 97 +++++-------------- server/src-lib/Hasura/RQL/DDL/Schema/Diff.hs | 31 ++---- server/src-lib/Hasura/RQL/DDL/Schema/Table.hs | 6 +- server/src-lib/Hasura/RQL/DML/Delete.hs | 22 ++--- server/src-lib/Hasura/RQL/DML/Insert.hs | 17 ++-- server/src-lib/Hasura/RQL/DML/Mutation.hs | 64 ++++++------ server/src-lib/Hasura/RQL/DML/Update.hs | 24 +++-- .../src-lib/Hasura/RQL/Types/SchemaCache.hs | 31 +----- server/src-lib/Hasura/SQL/DML.hs | 51 ++++++---- server/src-lib/Hasura/SQL/Rewrite.hs | 7 +- server/src-rsr/table_info.sql | 38 +++----- .../basic/article_returning_author.yaml | 40 ++++++++ server/tests-py/test_graphql_mutations.py | 3 + 22 files changed, 237 insertions(+), 376 deletions(-) create mode 100644 server/tests-py/queries/graphql_mutation/delete/basic/article_returning_author.yaml diff --git a/docs/graphql/manual/api-reference/mutation.rst b/docs/graphql/manual/api-reference/mutation.rst index dcbbd6e9..d348881e 100644 --- a/docs/graphql/manual/api-reference/mutation.rst +++ b/docs/graphql/manual/api-reference/mutation.rst @@ -238,34 +238,11 @@ Mutation Response ^^^^^^^^^^^^^^^^^ .. code-block:: none - # if table has atleast one primary key or - # one unique constraint with not null columns { affected_rows returning { - col-field1 - col-field2 - .. - relation1{ - relation1-field1 - relation1-field2 - .. - } - relation2{ - relation2-field1 - relation2-field2 - .. - } - .. - } - } - - # if table has no primary key or unique constraints - { - affected_rows - returning { - col-field1 - col-field2 + response-field1 + response-field2 .. } } @@ -274,22 +251,6 @@ E.g.: .. code-block:: graphql - # if table has atleast one primary key or - # one unique constraint with not null columns - { - affected_rows - returning { - id - author_id - articles{ - id - title - content - } - } - } - - # if table has no primary key or unique constraints { affected_rows returning { @@ -305,8 +266,6 @@ E.g.: .. code-block:: none - # if table has atleast one primary key or - # one unique constraint with not null columns objects: [ { field1: value, @@ -323,23 +282,12 @@ E.g.: }, .. ] - - # if table has no primary key or unique constraints - objects: [ - { - col_field1: value, - col_field2: value - .. - }, - .. - ] + # no nested objects E.g.: .. code-block:: graphql - # if table has atleast one primary key or - # one unique constraint with not null columns objects: [ { title: "Software is eating the world", @@ -353,14 +301,6 @@ E.g.: } ] - # if table has no primary key or unique constraints - objects: [ - { - title: "Software is eating the world", - content: "This week, Hewlett-Packard..." - } - ] - .. _ConflictClause: **on_conflict** argument diff --git a/docs/graphql/manual/mutations/delete.rst b/docs/graphql/manual/mutations/delete.rst index 2e424627..29d5271c 100644 --- a/docs/graphql/manual/mutations/delete.rst +++ b/docs/graphql/manual/mutations/delete.rst @@ -35,10 +35,8 @@ See the :ref:`delete mutation API reference ` for the full specif .. note:: - - If a table is not in the ``public`` Postgres schema, the delete mutation field will be of the format - ``delete__``. - - To fetch nested objects using relationships in the mutation response, the table needs to have either a primary - key or a unique constraint with not null columns. + If a table is not in the ``public`` Postgres schema, the delete mutation field will be of the format + ``delete__``. Delete based on an object's fields ---------------------------------- diff --git a/docs/graphql/manual/mutations/insert.rst b/docs/graphql/manual/mutations/insert.rst index e8a5ef7c..8f6458ca 100644 --- a/docs/graphql/manual/mutations/insert.rst +++ b/docs/graphql/manual/mutations/insert.rst @@ -36,10 +36,8 @@ See the :ref:`insert mutation API reference ` for the full .. note:: - - If a table is not in the ``public`` Postgres schema, the insert mutation field will be of the format - ``insert__``. - - To fetch nested objects using relationships in the mutation response, the table needs to have either a primary - key or a unique constraint with not null columns. + If a table is not in the ``public`` Postgres schema, the insert mutation field will be of the format + ``insert__``. Insert a single object ---------------------- @@ -217,10 +215,6 @@ Insert an object and get a nested object in response } } -.. note:: - - For this to work, the parent table (*in this case,* ``article``) needs to have either a primary key or a - unique constraint. Insert an object and its nested object in the same mutation ----------------------------------------------------------- @@ -276,11 +270,6 @@ in the response } } -.. note:: - - For this to work, the parent table (*in this case,* ``article``) needs to have either a primary key or a - unique constraint. - Insert an object with a JSONB column ------------------------------------ **Example:** Insert a new ``author`` object with a JSONB ``address`` column diff --git a/docs/graphql/manual/mutations/update.rst b/docs/graphql/manual/mutations/update.rst index bf0b4204..6560a355 100644 --- a/docs/graphql/manual/mutations/update.rst +++ b/docs/graphql/manual/mutations/update.rst @@ -39,10 +39,9 @@ See the :ref:`update mutation API reference ` for the full specif - At least any one of ``_set``, ``_inc`` operators or the jsonb operators ``_append``, ``_prepend``, ``_delete_key``, ``_delete_elem``, ``_delete_at_path`` is required. + - If a table is not in the ``public`` Postgres schema, the update mutation field will be of the format ``update__``. - - To fetch nested objects using relationships in the mutation response, the table needs to have either a primary - key or a unique constraint with not null columns. Update based on an object's fields ---------------------------------- diff --git a/docs/graphql/manual/mutations/upsert.rst b/docs/graphql/manual/mutations/upsert.rst index db835701..c1dbc796 100644 --- a/docs/graphql/manual/mutations/upsert.rst +++ b/docs/graphql/manual/mutations/upsert.rst @@ -145,10 +145,6 @@ You can specify ``on_conflict`` clause while inserting nested objects } } -.. note:: - - For this to work, the parent table (*in this case,* ``author``) needs to have either a primary key or a - unique constraint. .. admonition:: Edge-cases diff --git a/server/src-lib/Hasura/GraphQL/Context.hs b/server/src-lib/Hasura/GraphQL/Context.hs index 273692dc..0c89954d 100644 --- a/server/src-lib/Hasura/GraphQL/Context.hs +++ b/server/src-lib/Hasura/GraphQL/Context.hs @@ -57,15 +57,15 @@ data UpdOpCtx , _uocHeaders :: ![T.Text] , _uocFilter :: !AnnBoolExpSQL , _uocPresetCols :: !PreSetCols - , _uocUniqCols :: !(Maybe [PGColInfo]) + , _uocAllCols :: ![PGColInfo] } deriving (Show, Eq) data DelOpCtx = DelOpCtx - { _docTable :: !QualifiedTable - , _docHeaders :: ![T.Text] - , _docFilter :: !AnnBoolExpSQL - , _docUniqCols :: !(Maybe [PGColInfo]) + { _docTable :: !QualifiedTable + , _docHeaders :: ![T.Text] + , _docFilter :: !AnnBoolExpSQL + , _docAllCols :: ![PGColInfo] } deriving (Show, Eq) data OpCtx diff --git a/server/src-lib/Hasura/GraphQL/Resolve/ContextTypes.hs b/server/src-lib/Hasura/GraphQL/Resolve/ContextTypes.hs index 9d90dbda..09577dba 100644 --- a/server/src-lib/Hasura/GraphQL/Resolve/ContextTypes.hs +++ b/server/src-lib/Hasura/GraphQL/Resolve/ContextTypes.hs @@ -49,7 +49,6 @@ data InsCtx , icSet :: !PreSetCols , icRelations :: !RelationInfoMap , icUpdPerm :: !(Maybe UpdPermForIns) - , icUniqCols :: !(Maybe [PGColInfo]) } deriving (Show, Eq) type InsCtxMap = Map.HashMap QualifiedTable InsCtx diff --git a/server/src-lib/Hasura/GraphQL/Resolve/Insert.hs b/server/src-lib/Hasura/GraphQL/Resolve/Insert.hs index 7b1de428..9cd18823 100644 --- a/server/src-lib/Hasura/GraphQL/Resolve/Insert.hs +++ b/server/src-lib/Hasura/GraphQL/Resolve/Insert.hs @@ -49,7 +49,6 @@ data AnnIns a , _aiView :: !QualifiedTable , _aiTableCols :: ![PGColInfo] , _aiDefVals :: !(Map.HashMap PGCol S.SQLExp) - , _aiUniqCols :: !(Maybe [PGColInfo]) } deriving (Show, Eq, Functor, Foldable, Traversable) type SingleObjIns = AnnIns AnnInsObj @@ -121,14 +120,14 @@ traverseInsObj rim (gName, annVal) defVal@(AnnInsObj cols objRels arrRels) = throw500 $ "relation " <> relName <<> " not found" let rTable = riRTable relInfo - InsCtx rtView rtCols rtDefVals rtRelInfoMap rtUpdPerm rtUniqCols <- getInsCtx rTable + InsCtx rtView rtCols rtDefVals rtRelInfoMap rtUpdPerm <- getInsCtx rTable withPathK (G.unName gName) $ case riType relInfo of ObjRel -> do dataObj <- asObject dataVal annDataObj <- mkAnnInsObj rtRelInfoMap dataObj ccM <- forM onConflictM $ parseOnConflict rTable rtUpdPerm - let singleObjIns = AnnIns annDataObj ccM rtView rtCols rtDefVals rtUniqCols + let singleObjIns = AnnIns annDataObj ccM rtView rtCols rtDefVals objRelIns = RelIns singleObjIns relInfo return (AnnInsObj cols (objRelIns:objRels) arrRels) @@ -139,7 +138,7 @@ traverseInsObj rim (gName, annVal) defVal@(AnnInsObj cols objRels arrRels) = dataObj <- asObject arrDataVal mkAnnInsObj rtRelInfoMap dataObj ccM <- forM onConflictM $ parseOnConflict rTable rtUpdPerm - let multiObjIns = AnnIns annDataObjs ccM rtView rtCols rtDefVals rtUniqCols + let multiObjIns = AnnIns annDataObjs ccM rtView rtCols rtDefVals arrRelIns = RelIns multiObjIns relInfo return (AnnInsObj cols objRels (arrRelIns:arrRels)) -- if array relation insert input data has empty objects @@ -200,7 +199,8 @@ mkInsertQ vn onConflictM insCols tableCols defVals role = do (givenCols, args) <- flip runStateT Seq.Empty $ toSQLExps insCols let sqlConflict = RI.toSQLConflict <$> onConflictM sqlExps = mkSQLRow defVals givenCols - sqlInsert = S.SQLInsert vn tableCols [sqlExps] sqlConflict $ Just S.returningStar + valueExp = S.ValuesExp [S.TupleExp sqlExps] + sqlInsert = S.SQLInsert vn tableCols valueExp sqlConflict $ Just S.returningStar adminIns = return (CTEExp (S.CTEInsert sqlInsert) args, Nothing) nonAdminInsert = do ccM <- mapM RI.extractConflictCtx onConflictM @@ -209,19 +209,6 @@ mkInsertQ vn onConflictM insCols tableCols defVals role = do bool nonAdminInsert adminIns $ isAdmin role -mkBoolExp - :: (MonadError QErr m, MonadState PrepArgs m) - => QualifiedTable -> [(PGColInfo, PGColValue)] - -> m S.BoolExp -mkBoolExp _ [] = return $ S.BELit False -mkBoolExp tn colInfoVals = - RB.toSQLBoolExp (S.mkQual tn) . BoolAnd <$> - mapM (fmap BoolFld . uncurry f) colInfoVals - where - f ci@(PGColInfo _ colTy _) colVal = - AVCol ci . pure . AEQ True <$> - prepare (AnnPGVal Nothing True colTy colVal) - asSingleObject :: MonadError QErr m => [ColVals] -> m (Maybe ColVals) @@ -250,18 +237,9 @@ mkSelCTE -> [PGColInfo] -> Maybe ColVals -> m CTEExp -mkSelCTE tn uniqCols colValM = do - (whereExp, args) <- case colValM of - Nothing -> return (S.BELit False, Seq.empty) - Just colVal -> do - colInfoWithVals <- fetchFromColVals colVal uniqCols id - flip runStateT Seq.Empty $ mkBoolExp tn colInfoWithVals - let sqlSel = S.mkSelect { S.selExtr = [S.selectStar] - , S.selFrom = Just $ S.mkSimpleFromExp tn - , S.selWhere = Just $ S.WhereFrag whereExp - } - - return $ CTEExp (S.CTESelect sqlSel) args +mkSelCTE tn allCols colValM = do + selCTE <- mkSelCTEFromColVals tn allCols $ maybe [] pure colValM + return $ CTEExp selCTE Seq.Empty execCTEExp :: Bool @@ -389,32 +367,29 @@ insertObj strfyNum role tn singleObjIns addCols = do addInsCols = mkPGColWithTypeAndVal allCols addCols finalInsCols = cols <> objRelInsCols <> addInsCols - -- prepare final returning columns - let arrDepCols = concatMap (map fst . riMapping . _riRelInfo) arrRels - arrDepColsWithInfo = getColInfos arrDepCols allCols - -- prepare insert query as with expression (CTEExp cte insPArgs, ccM) <- mkInsertQ vn onConflictM finalInsCols (map pgiName allCols) defVals role - uniqCols <- onNothing uniqColsM $ throw500 "unique columns not found in relational insert" RI.setConflictCtx ccM - MutateResp affRows colVals <- - mutateAndFetchCols tn (uniqCols `union` arrDepColsWithInfo) (cte, insPArgs) strfyNum + MutateResp affRows colVals <- mutateAndFetchCols tn allCols (cte, insPArgs) strfyNum colValM <- asSingleObject colVals - cteExp <- mkSelCTE tn uniqCols colValM + cteExp <- mkSelCTE tn allCols colValM - arrRelAffRows <- bool (withArrRels arrDepColsWithInfo colValM) (return 0) $ null arrRels + arrRelAffRows <- bool (withArrRels colValM) (return 0) $ null arrRels let totAffRows = objRelAffRows + affRows + arrRelAffRows return (totAffRows, cteExp) where - AnnIns annObj onConflictM vn allCols defVals uniqColsM = singleObjIns + AnnIns annObj onConflictM vn allCols defVals = singleObjIns AnnInsObj cols objRels arrRels = annObj - withArrRels arrDepCols colValM = do + arrRelDepCols = flip getColInfos allCols $ + concatMap (map fst . riMapping . _riRelInfo) arrRels + + withArrRels colValM = do colVal <- onNothing colValM $ throw400 NotSupported cannotInsArrRelErr - arrDepColsWithVal <- fetchFromColVals colVal arrDepCols pgiName + arrDepColsWithVal <- fetchFromColVals colVal arrRelDepCols pgiName arrInsARows <- forM arrRels $ insertArrRel strfyNum role arrDepColsWithVal @@ -438,7 +413,7 @@ insertMultipleObjects insertMultipleObjects strfyNum role tn multiObjIns addCols mutFlds errP = bool withoutRelsInsert withRelsInsert anyRelsToInsert where - AnnIns insObjs onConflictM vn tableColInfos defVals uniqCols = multiObjIns + AnnIns insObjs onConflictM vn tableColInfos defVals = multiObjIns singleObjInserts = multiToSingles multiObjIns insCols = map _aioColumns insObjs allInsObjRels = concatMap _aioObjRels insObjs @@ -460,7 +435,7 @@ insertMultipleObjects strfyNum role tn multiObjIns addCols mutFlds errP = rowsWithCol <- mapM toSQLExps withAddCols return $ map (mkSQLRow defVals) rowsWithCol - let insQP1 = RI.InsertQueryP1 tn vn tableCols sqlRows onConflictM mutFlds uniqCols + let insQP1 = RI.InsertQueryP1 tn vn tableCols sqlRows onConflictM mutFlds tableColInfos p1 = (insQP1, prepArgs) bool (RI.nonAdminInsert strfyNum p1) (RI.insertP2 strfyNum p1) $ isAdmin role @@ -502,11 +477,11 @@ convertInsert role tn fld = prefixErrPath fld $ do bool (withNonEmptyObjs annVals mutFlds) (withEmptyObjs mutFlds) $ null annVals where withNonEmptyObjs annVals mutFlds = do - InsCtx vn tableCols defValMap relInfoMap updPerm uniqCols <- getInsCtx tn + InsCtx vn tableCols defValMap relInfoMap updPerm <- getInsCtx tn annObjs <- mapM asObject annVals annInsObjs <- forM annObjs $ mkAnnInsObj relInfoMap conflictClauseM <- forM onConflictM $ parseOnConflict tn updPerm - let multiObjIns = AnnIns annInsObjs conflictClauseM vn tableCols defValMap uniqCols + let multiObjIns = AnnIns annInsObjs conflictClauseM vn tableCols defValMap strfyNum <- stringifyNum <$> asks getter return $ prefixErrPath fld $ insertMultipleObjects strfyNum role tn multiObjIns [] mutFlds "objects" diff --git a/server/src-lib/Hasura/GraphQL/Resolve/Mutation.hs b/server/src-lib/Hasura/GraphQL/Resolve/Mutation.hs index 95100ef3..45f2baf2 100644 --- a/server/src-lib/Hasura/GraphQL/Resolve/Mutation.hs +++ b/server/src-lib/Hasura/GraphQL/Resolve/Mutation.hs @@ -20,6 +20,7 @@ import qualified Hasura.RQL.DML.Update as RU import qualified Hasura.SQL.DML as S +import Hasura.EncJSON import Hasura.GraphQL.Context import Hasura.GraphQL.Resolve.BoolExp import Hasura.GraphQL.Resolve.Context @@ -30,7 +31,6 @@ import Hasura.GraphQL.Validate.Types import Hasura.RQL.Types import Hasura.SQL.Types import Hasura.SQL.Value -import Hasura.EncJSON convertMutResp :: G.NamedType -> SelSet -> Convert RR.MutFlds @@ -130,14 +130,14 @@ convertUpdate opCtx fld = do "atleast any one of _set, _inc, _append, _prepend, _delete_key, _delete_elem and " <> " _delete_at_path operator is expected" strfyNum <- stringifyNum <$> asks getter - let p1 = RU.UpdateQueryP1 tn setItems (filterExp, whereExp) mutFlds uniqCols + let p1 = RU.UpdateQueryP1 tn setItems (filterExp, whereExp) mutFlds allCols whenNonEmptyItems = return $ RU.updateQueryToTx strfyNum (p1, prepArgs) whenEmptyItems = return $ return $ buildEmptyMutResp mutFlds -- if there are not set items then do not perform -- update and return empty mutation response bool whenNonEmptyItems whenEmptyItems $ null setItems where - UpdOpCtx tn _ filterExp preSetCols uniqCols = opCtx + UpdOpCtx tn _ filterExp preSetCols allCols = opCtx args = _fArguments fld preSetItems = Map.toList preSetCols @@ -149,11 +149,11 @@ convertDelete opCtx fld = do whereExp <- withArg (_fArguments fld) "where" (parseBoolExp prepare) mutFlds <- convertMutResp (_fType fld) $ _fSelSet fld args <- get - let p1 = RD.DeleteQueryP1 tn (filterExp, whereExp) mutFlds uniqCols + let p1 = RD.DeleteQueryP1 tn (filterExp, whereExp) mutFlds allCols strfyNum <- stringifyNum <$> asks getter return $ RD.deleteQueryToTx strfyNum (p1, args) where - DelOpCtx tn _ filterExp uniqCols = opCtx + DelOpCtx tn _ filterExp allCols = opCtx -- | build mutation response for empty objects buildEmptyMutResp :: RR.MutFlds -> EncJSON diff --git a/server/src-lib/Hasura/GraphQL/Schema.hs b/server/src-lib/Hasura/GraphQL/Schema.hs index 1b1fab90..78b08a74 100644 --- a/server/src-lib/Hasura/GraphQL/Schema.hs +++ b/server/src-lib/Hasura/GraphQL/Schema.hs @@ -97,9 +97,9 @@ getValidCols = fst . validPartitionFieldInfoMap getValidRels :: FieldInfoMap -> [RelInfo] getValidRels = snd . validPartitionFieldInfoMap -mkValidConstraints :: [TableConstraint] -> [TableConstraint] +mkValidConstraints :: [ConstraintName] -> [ConstraintName] mkValidConstraints = - filter (isValidName . G.Name . getConstraintTxt . tcName) + filter (isValidName . G.Name . getConstraintTxt) isRelNullable :: FieldInfoMap -> RelInfo -> Bool isRelNullable fim ri = isNullable @@ -149,10 +149,6 @@ mkTableTy :: QualifiedTable -> G.NamedType mkTableTy = G.NamedType . qualObjectToName -mkTableColTy :: QualifiedTable -> G.NamedType -mkTableColTy tn = - G.NamedType $ qualObjectToName tn <> "_columns" - mkTableAggTy :: QualifiedTable -> G.NamedType mkTableAggTy tn = G.NamedType $ qualObjectToName tn <> "_aggregate" @@ -260,26 +256,6 @@ mkTableObj tn allowedFlds = mkRelFld allowAgg relInfo isNullable desc = G.Description $ "columns and relationships of " <>> tn -{- -type table_columns { - col1: colty1 - . - . - coln: coltyn -} --} - -mkTableColObj - :: QualifiedTable - -> [PGColInfo] - -> ObjTyInfo -mkTableColObj tn allowedCols = - mkHsraObjTyInfo (Just desc) (mkTableColTy tn) Set.empty $ - mapFromL _fiName flds - where - flds = map mkPGColFld allowedCols - desc = G.Description $ "columns of " <>> tn - {- type table_aggregate { agg: table_aggregate_fields @@ -503,9 +479,8 @@ type table_mutation_response { mkMutRespObj :: QualifiedTable -> Bool -- is sel perm defined - -> Bool -- is nested allowed -> ObjTyInfo -mkMutRespObj tn sel nestAlwd = +mkMutRespObj tn sel = mkHsraObjTyInfo (Just objDesc) (mkMutRespTy tn) Set.empty $ mapFromL _fiName $ affectedRowsFld : bool [] [returningFld] sel where @@ -518,10 +493,9 @@ mkMutRespObj tn sel nestAlwd = desc = "number of affected rows by the mutation" returningFld = mkHsraObjFldInfo (Just desc) "returning" Map.empty $ - G.toGT $ G.toNT $ G.toLT $ G.toNT retTy + G.toGT $ G.toNT $ G.toLT $ G.toNT $ mkTableTy tn where desc = "data of the affected rows by the mutation" - retTy = bool (mkTableColTy tn) (mkTableTy tn) nestAlwd -- table_bool_exp mkBoolExpInp @@ -954,11 +928,10 @@ mkInsInp :: QualifiedTable -> InsCtx -> InpObjTyInfo mkInsInp tn insCtx = mkHsraInpTyInfo (Just desc) (mkInsInpTy tn) $ fromInpValL $ - map mkPGColInp insCols <> bool [] relInps alwNestedIns + map mkPGColInp insCols <> relInps where desc = G.Description $ "input type for inserting data into table " <>> tn - alwNestedIns = isJust $ icUniqCols insCtx cols = icColumns insCtx setCols = Map.keys $ icSet insCtx insCols = flip filter cols $ \ci -> pgiName ci `notElem` setCols @@ -1243,8 +1216,6 @@ mkOnConflictTypes tn uniqueOrPrimaryCons cols = mkGCtxRole' :: QualifiedTable - -- all columns - -> [PGColInfo] -- insert perm -> Maybe (InsCtx, Bool) -- select permission @@ -1256,12 +1227,12 @@ mkGCtxRole' -- primary key columns -> [PGColInfo] -- constraints - -> [TableConstraint] + -> [ConstraintName] -> Maybe ViewInfo -- all functions -> [FunctionInfo] -> TyAgg -mkGCtxRole' tn allCols insPermM selPermM updColsM +mkGCtxRole' tn insPermM selPermM updColsM delPermM pkeyCols constraints viM funcs = TyAgg (mkTyInfoMap allTypes) fieldMap scalars ordByCtx @@ -1269,11 +1240,9 @@ mkGCtxRole' tn allCols insPermM selPermM updColsM ordByCtx = fromMaybe Map.empty ordByCtxM upsertPerm = or $ fmap snd insPermM - alwNestMutFld = isJust $ getUniqCols allCols constraints - constNames = map tcName constraints - isUpsertable = upsertable constNames upsertPerm $ isJust viM + isUpsertable = upsertable constraints upsertPerm $ isJust viM updatableCols = maybe [] (map pgiName) updColsM - onConflictTypes = mkOnConflictTypes tn constNames updatableCols isUpsertable + onConflictTypes = mkOnConflictTypes tn constraints updatableCols isUpsertable jsonOpTys = fromMaybe [] updJSONOpInpObjTysM relInsInpObjTys = maybe [] (map TIInpObj) $ mutHelper viIsInsertable relInsInpObjsM @@ -1288,7 +1257,6 @@ mkGCtxRole' tn allCols insPermM selPermM updColsM [ TIInpObj <$> boolExpInpObjM , TIInpObj <$> ordByInpObjM , TIObj <$> selObjM - , TIObj <$> selColObjM ] aggQueryTypes = map TIObj aggObjs <> map TIInpObj aggOrdByInps @@ -1306,7 +1274,6 @@ mkGCtxRole' tn allCols insPermM selPermM updColsM fieldMap = Map.unions $ catMaybes [ insInpObjFldsM, updSetInpObjFldsM , boolExpInpObjFldsM , selObjFldsM - , selColObjFldsM ] scalars = Set.unions [selByPkScalarSet, funcArgScalarSet] @@ -1372,7 +1339,7 @@ mkGCtxRole' tn allCols insPermM selPermM updColsM -- mut resp obj mutRespObjM = if isMut - then Just $ mkMutRespObj tn (isJust selFldsM) alwNestMutFld + then Just $ mkMutRespObj tn $ isJust selFldsM else Nothing isMut = (isJust insColsM || isJust updColsM || isJust delPermM) @@ -1380,10 +1347,6 @@ mkGCtxRole' tn allCols insPermM selPermM updColsM -- table obj selObjM = mkTableObj tn <$> selFldsM - -- table columns obj - selColObjM = if not alwNestMutFld then - (mkTableColObj tn . lefts) <$> selFldsM - else Nothing -- aggregate objs and order by inputs (aggObjs, aggOrdByInps) = case selPermM of @@ -1415,10 +1378,6 @@ mkGCtxRole' tn allCols insPermM selPermM updColsM in numFldsObjs <> compFldsObjs -- the fields used in table object selObjFldsM = mkFldMap (mkTableTy tn) <$> selFldsM - -- the fields used in table_columns object - selColObjFldsM = if not alwNestMutFld then - (mkColFldMap (mkTableColTy tn) . lefts) <$> selFldsM - else Nothing -- the scalar set for table_by_pk arguments selByPkScalarSet = Set.fromList $ map pgiType pkeyCols @@ -1430,7 +1389,7 @@ mkGCtxRole' tn allCols insPermM selPermM updColsM getRootFldsRole' :: QualifiedTable -> [PGCol] - -> [TableConstraint] + -> [ConstraintName] -> FieldInfoMap -> [FunctionInfo] -> Maybe ([T.Text], Bool) -- insert perm @@ -1442,6 +1401,7 @@ getRootFldsRole' getRootFldsRole' tn primCols constraints fields funcs insM selM updM delM viM = RootFlds mFlds where + allCols = getCols fields mFlds = mapFromL (either _fiName _fiName . snd) $ funcQueries <> funcAggQueries <> @@ -1461,21 +1421,19 @@ getRootFldsRole' tn primCols constraints fields funcs insM selM updM delM viM = bool Nothing (getDet <$> mutM) $ isMutable f viM colInfos = fst $ validPartitionFieldInfoMap fields - constNames = map tcName constraints - uniqCols = getUniqCols colInfos constraints getInsDet (hdrs, upsertPerm) = - let isUpsertable = upsertable constNames upsertPerm $ isJust viM + let isUpsertable = upsertable constraints upsertPerm $ isJust viM in ( OCInsert $ InsOpCtx tn $ hdrs `union` maybe [] (\(_, _, _, x) -> x) updM , Right $ mkInsMutFld tn isUpsertable ) getUpdDet (updCols, preSetCols, updFltr, hdrs) = - ( OCUpdate $ UpdOpCtx tn hdrs updFltr preSetCols uniqCols + ( OCUpdate $ UpdOpCtx tn hdrs updFltr preSetCols allCols , Right $ mkUpdMutFld tn $ getColInfos updCols colInfos ) getDelDet (delFltr, hdrs) = - ( OCDelete $ DelOpCtx tn hdrs delFltr uniqCols + ( OCDelete $ DelOpCtx tn hdrs delFltr allCols , Right $ mkDelMutFld tn ) getSelDet (selFltr, pLimit, hdrs, _) = @@ -1552,10 +1510,9 @@ mkInsCtx -> TableCache -> FieldInfoMap -> InsPermInfo - -> Maybe [PGColInfo] -> Maybe UpdPermInfo -> m InsCtx -mkInsCtx role tableCache fields insPermInfo uniqCols updPermM = do +mkInsCtx role tableCache fields insPermInfo updPermM = do relTupsM <- forM rels $ \relInfo -> do let remoteTable = riRTable relInfo relName = riName relInfo @@ -1566,7 +1523,7 @@ mkInsCtx role tableCache fields insPermInfo uniqCols updPermM = do isInsertable insPermM viewInfoM let relInfoMap = Map.fromList $ catMaybes relTupsM - return $ InsCtx iView cols setCols relInfoMap updPermForIns uniqCols + return $ InsCtx iView cols setCols relInfoMap updPermForIns where cols = getValidCols fields rels = getValidRels fields @@ -1584,9 +1541,8 @@ mkAdminInsCtx => QualifiedTable -> TableCache -> FieldInfoMap - -> Maybe [PGColInfo] -> m InsCtx -mkAdminInsCtx tn tc fields uniqCols = do +mkAdminInsCtx tn tc fields = do relTupsM <- forM rels $ \relInfo -> do let remoteTable = riRTable relInfo relName = riName relInfo @@ -1598,7 +1554,7 @@ mkAdminInsCtx tn tc fields uniqCols = do let relInfoMap = Map.fromList $ catMaybes relTupsM updPerm = UpdPermForIns (map pgiName cols) noFilter Map.empty - return $ InsCtx tn cols Map.empty relInfoMap (Just updPerm) uniqCols + return $ InsCtx tn cols Map.empty relInfoMap (Just updPerm) where cols = getValidCols fields rels = getValidRels fields @@ -1609,7 +1565,7 @@ mkGCtxRole -> QualifiedTable -> FieldInfoMap -> [PGCol] - -> [TableConstraint] + -> [ConstraintName] -> [FunctionInfo] -> Maybe ViewInfo -> RoleName @@ -1618,17 +1574,16 @@ mkGCtxRole mkGCtxRole tableCache tn fields pCols constraints funcs viM role permInfo = do selPermM <- mapM (getSelPerm tableCache fields role) $ _permSel permInfo tabInsCtxM <- forM (_permIns permInfo) $ \ipi -> do - tic <- mkInsCtx role tableCache fields ipi uniqCols $ _permUpd permInfo + tic <- mkInsCtx role tableCache fields ipi $ _permUpd permInfo return (tic, isJust $ _permUpd permInfo) let updColsM = filterColInfos . upiCols <$> _permUpd permInfo - tyAgg = mkGCtxRole' tn allCols tabInsCtxM selPermM updColsM + tyAgg = mkGCtxRole' tn tabInsCtxM selPermM updColsM (void $ _permDel permInfo) pColInfos constraints viM funcs rootFlds = getRootFldsRole tn pCols constraints fields funcs viM permInfo insCtxMap = maybe Map.empty (Map.singleton tn) $ fmap fst tabInsCtxM return (tyAgg, rootFlds, insCtxMap) where allCols = getCols fields - uniqCols = getUniqCols allCols constraints colInfos = getValidCols fields pColInfos = getColInfos pCols allCols filterColInfos allowedSet = @@ -1637,7 +1592,7 @@ mkGCtxRole tableCache tn fields pCols constraints funcs viM role permInfo = do getRootFldsRole :: QualifiedTable -> [PGCol] - -> [TableConstraint] + -> [ConstraintName] -> FieldInfoMap -> [FunctionInfo] -> Maybe ViewInfo @@ -1669,8 +1624,8 @@ mkGCtxMapTable mkGCtxMapTable tableCache funcCache tabInfo = do m <- Map.traverseWithKey (mkGCtxRole tableCache tn fields pkeyCols validConstraints tabFuncs viewInfo) rolePerms - adminInsCtx <- mkAdminInsCtx tn tableCache fields $ getUniqCols allCols constraints - let adminCtx = mkGCtxRole' tn allCols (Just (adminInsCtx, True)) + adminInsCtx <- mkAdminInsCtx tn tableCache fields + let adminCtx = mkGCtxRole' tn (Just (adminInsCtx, True)) (Just (True, selFlds)) (Just colInfos) (Just ()) pkeyColInfos validConstraints viewInfo tabFuncs adminInsCtxMap = Map.singleton tn adminInsCtx @@ -1678,7 +1633,6 @@ mkGCtxMapTable tableCache funcCache tabInfo = do where TableInfo tn _ fields rolePerms constraints pkeyCols viewInfo _ = tabInfo validConstraints = mkValidConstraints constraints - allCols = getCols fields colInfos = getValidCols fields validColNames = map pgiName colInfos pkeyColInfos = getColInfos pkeyCols colInfos @@ -1696,7 +1650,6 @@ mkGCtxMapTable tableCache funcCache tabInfo = do noFilter :: AnnBoolExpSQL noFilter = annBoolExpTrue - checkSchemaConflicts :: (MonadError QErr m) => GCtx -> GCtx -> m () diff --git a/server/src-lib/Hasura/RQL/DDL/Schema/Diff.hs b/server/src-lib/Hasura/RQL/DDL/Schema/Diff.hs index 6f39cc8c..de613c24 100644 --- a/server/src-lib/Hasura/RQL/DDL/Schema/Diff.hs +++ b/server/src-lib/Hasura/RQL/DDL/Schema/Diff.hs @@ -49,7 +49,6 @@ data ConstraintMeta { cmName :: !ConstraintName , cmOid :: !Int , cmType :: !ConstraintType - , cmCols :: ![PGCol] } deriving (Show, Eq) $(deriveJSON (aesonDrop 2 snakeCase){omitNothingFields=True} ''ConstraintMeta) @@ -101,27 +100,11 @@ fetchTableMeta = do json_build_object( 'name', tc.constraint_name, 'oid', r.oid::integer, - 'type', tc.constraint_type, - 'cols', tc.columns + 'type', tc.constraint_type ) ) as constraints FROM - ( - SELECT table_name, table_schema, - constraint_name, columns, - 'PRIMARY KEY' as constraint_type - FROM hdb_catalog.hdb_primary_key - UNION ALL - SELECT table_name, table_schema, - constraint_name, columns, - 'UNIQUE' as constraint_type - FROM hdb_catalog.hdb_unique_constraint - UNION ALL - SELECT table_name, table_schema, - constraint_name, '[]'::json as columns, - 'FOREIGN KEY' as constraint_type - FROM hdb_catalog.hdb_foreign_key_constraint - ) tc + information_schema.table_constraints tc JOIN pg_catalog.pg_constraint r ON tc.constraint_name = r.conname GROUP BY @@ -132,8 +115,8 @@ fetchTableMeta = do AND t.table_schema <> 'information_schema' AND t.table_schema <> 'hdb_catalog' |] () False - forM res $ \(ts, tn, toid, cols, constrnts) - -> return $ TableMeta toid (QualifiedObject ts tn) (Q.getAltJ cols) (Q.getAltJ constrnts) + forM res $ \(ts, tn, toid, cols, constrnts) -> + return $ TableMeta toid (QualifiedObject ts tn) (Q.getAltJ cols) (Q.getAltJ constrnts) getOverlap :: (Eq k, Hashable k) => (v -> k) -> [v] -> [v] -> [(v, v)] getOverlap getKey left right = @@ -157,7 +140,7 @@ data TableDiff -- The final list of uniq/primary constraint names -- used for generating types on_conflict clauses -- TODO: this ideally should't be part of TableDiff - , _tdUniqOrPriCons :: ![TableConstraint] + , _tdUniqOrPriCons :: ![ConstraintName] } deriving (Show, Eq) getTableDiff :: TableMeta -> TableMeta -> TableDiff @@ -170,9 +153,7 @@ getTableDiff oldtm newtm = newCols = tmColumns newtm uniqueOrPrimaryCons = - [ TableConstraint (cmType cm) (cmName cm) (cmCols cm) - | cm <- tmConstraints newtm, isUniqueOrPrimary (cmType cm) - ] + [cmName cm | cm <- tmConstraints newtm, isUniqueOrPrimary (cmType cm)] droppedCols = map pcmColumnName $ getDifference pcmOrdinalPosition oldCols newCols diff --git a/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs b/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs index 6bd5b311..6e971e29 100644 --- a/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs +++ b/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs @@ -2,9 +2,9 @@ module Hasura.RQL.DDL.Schema.Table where +import Hasura.EncJSON import Hasura.GraphQL.RemoteServer import Hasura.Prelude -import Hasura.EncJSON import Hasura.RQL.DDL.Deps import Hasura.RQL.DDL.Permission import Hasura.RQL.DDL.Permission.Internal @@ -55,8 +55,8 @@ getTableInfo qt@(QualifiedObject sn tn) isSystemDefined = do Q.listQ $(Q.sqlFromFile "src-rsr/table_info.sql")(sn, tn) True case tableData of [] -> throw400 NotExists $ "no such table/view exists in postgres : " <>> qt - [(Q.AltJ cols, Q.AltJ cons, Q.AltJ viewInfoM)] -> - return $ mkTableInfo qt isSystemDefined cons cols viewInfoM + [(Q.AltJ cols, Q.AltJ pkeyCols, Q.AltJ cons, Q.AltJ viewInfoM)] -> + return $ mkTableInfo qt isSystemDefined cons cols pkeyCols viewInfoM _ -> throw500 $ "more than one row found for: " <>> qt newtype TrackTable diff --git a/server/src-lib/Hasura/RQL/DML/Delete.hs b/server/src-lib/Hasura/RQL/DML/Delete.hs index 0e777b6c..c597fce5 100644 --- a/server/src-lib/Hasura/RQL/DML/Delete.hs +++ b/server/src-lib/Hasura/RQL/DML/Delete.hs @@ -26,10 +26,10 @@ import qualified Hasura.SQL.DML as S data DeleteQueryP1 = DeleteQueryP1 - { dqp1Table :: !QualifiedTable - , dqp1Where :: !(AnnBoolExpSQL, AnnBoolExpSQL) - , dqp1MutFlds :: !MutFlds - , dqp1UniqCols :: !(Maybe [PGColInfo]) + { dqp1Table :: !QualifiedTable + , dqp1Where :: !(AnnBoolExpSQL, AnnBoolExpSQL) + , dqp1MutFlds :: !MutFlds + , dqp1AllCols :: ![PGColInfo] } deriving (Show, Eq) mkDeleteCTE @@ -43,12 +43,11 @@ mkDeleteCTE (DeleteQueryP1 tn (fltr, wc) _ _) = getDeleteDeps :: DeleteQueryP1 -> [SchemaDependency] -getDeleteDeps (DeleteQueryP1 tn (_, wc) mutFlds uniqCols) = - mkParentDep tn : uniqColDeps <> whereDeps <> retDeps +getDeleteDeps (DeleteQueryP1 tn (_, wc) mutFlds allCols) = + mkParentDep tn : allColDeps <> whereDeps <> retDeps where whereDeps = getBoolExpDeps tn wc - uniqColDeps = map (mkColDep "on_type" tn) $ - maybe [] (map pgiName) uniqCols + allColDeps = map (mkColDep "on_type" tn . pgiName) allCols retDeps = map (mkColDep "untyped" tn . fst) $ pgColsFromMutFlds mutFlds @@ -75,8 +74,7 @@ validateDeleteQWith prepValBuilder (DeleteQuery tableName rqlBE mRetCols) = do askSelPermInfo tableInfo let fieldInfoMap = tiFieldInfoMap tableInfo - uniqCols = getUniqCols (getCols fieldInfoMap) $ - tiUniqOrPrimConstraints tableInfo + allCols = getCols fieldInfoMap -- convert the returning cols into sql returing exp mAnnRetCols <- forM mRetCols $ \retCols -> @@ -88,7 +86,7 @@ validateDeleteQWith prepValBuilder (DeleteQuery tableName rqlBE mRetCols) = do return $ DeleteQueryP1 tableName (dpiFilter delPerm, annSQLBoolExp) - (mkDefaultMutFlds mAnnRetCols) uniqCols + (mkDefaultMutFlds mAnnRetCols) allCols where selNecessaryMsg = @@ -105,7 +103,7 @@ validateDeleteQ = deleteQueryToTx :: Bool -> (DeleteQueryP1, DS.Seq Q.PrepArg) -> Q.TxE QErr EncJSON deleteQueryToTx strfyNum (u, p) = runMutation $ Mutation (dqp1Table u) (deleteCTE, p) - (dqp1MutFlds u) (dqp1UniqCols u) strfyNum + (dqp1MutFlds u) (dqp1AllCols u) strfyNum where deleteCTE = mkDeleteCTE u diff --git a/server/src-lib/Hasura/RQL/DML/Insert.hs b/server/src-lib/Hasura/RQL/DML/Insert.hs index b5a14fe4..1c709861 100644 --- a/server/src-lib/Hasura/RQL/DML/Insert.hs +++ b/server/src-lib/Hasura/RQL/DML/Insert.hs @@ -9,8 +9,8 @@ import qualified Data.HashSet as HS import qualified Data.Sequence as DS import qualified Data.Text.Lazy as LT -import Hasura.Prelude import Hasura.EncJSON +import Hasura.Prelude import Hasura.RQL.DML.Internal import Hasura.RQL.DML.Mutation import Hasura.RQL.DML.Returning @@ -40,15 +40,16 @@ data InsertQueryP1 , iqp1Tuples :: ![[S.SQLExp]] , iqp1Conflict :: !(Maybe ConflictClauseP1) , iqp1MutFlds :: !MutFlds - , iqp1UniqCols :: !(Maybe [PGColInfo]) + , iqp1AllCols :: ![PGColInfo] } deriving (Show, Eq) mkInsertCTE :: InsertQueryP1 -> S.CTE mkInsertCTE (InsertQueryP1 _ vn cols vals c _ _) = S.CTEInsert insert where + tupVals = S.ValuesExp $ map S.TupleExp vals insert = - S.SQLInsert vn cols vals (toSQLConflict <$> c) $ Just S.returningStar + S.SQLInsert vn cols tupVals (toSQLConflict <$> c) $ Just S.returningStar toSQLConflict :: ConflictClauseP1 -> S.SQLConflict toSQLConflict conflict = case conflict of @@ -148,8 +149,7 @@ buildConflictClause tableInfo inpCols (OnConflict mTCol mTCons act) = \pgCol -> askPGType fieldInfoMap pgCol "" validateConstraint c = do - let tableConsNames = map tcName $ - tiUniqOrPrimConstraints tableInfo + let tableConsNames = tiUniqOrPrimConstraints tableInfo withPathK "constraint" $ unless (c `elem` tableConsNames) $ throw400 Unexpected $ "constraint " <> getConstraintTxt c @@ -190,8 +190,6 @@ convInsertQuery objsParser prepFn (InsertQuery tableName val oC mRetCols) = do let fieldInfoMap = tiFieldInfoMap tableInfo setInsVals = ipiSet insPerm - uniqCols = getUniqCols (getCols fieldInfoMap) $ - tiUniqOrPrimConstraints tableInfo -- convert the returning cols into sql returing exp mAnnRetCols <- forM mRetCols $ \retCols -> do @@ -204,6 +202,7 @@ convInsertQuery objsParser prepFn (InsertQuery tableName val oC mRetCols) = do let mutFlds = mkDefaultMutFlds mAnnRetCols let defInsVals = mkDefValMap fieldInfoMap + allCols = getCols fieldInfoMap insCols = HM.keys defInsVals insView = ipiView insPerm @@ -220,7 +219,7 @@ convInsertQuery objsParser prepFn (InsertQuery tableName val oC mRetCols) = do buildConflictClause tableInfo inpCols c return $ InsertQueryP1 tableName insView insCols sqlExps - conflictClause mutFlds uniqCols + conflictClause mutFlds allCols where selNecessaryMsg = @@ -244,7 +243,7 @@ convInsQ = insertP2 :: Bool -> (InsertQueryP1, DS.Seq Q.PrepArg) -> Q.TxE QErr EncJSON insertP2 strfyNum (u, p) = runMutation $ Mutation (iqp1Table u) (insertCTE, p) - (iqp1MutFlds u) (iqp1UniqCols u) strfyNum + (iqp1MutFlds u) (iqp1AllCols u) strfyNum where insertCTE = mkInsertCTE u diff --git a/server/src-lib/Hasura/RQL/DML/Mutation.hs b/server/src-lib/Hasura/RQL/DML/Mutation.hs index 4c3fb336..b9790632 100644 --- a/server/src-lib/Hasura/RQL/DML/Mutation.hs +++ b/server/src-lib/Hasura/RQL/DML/Mutation.hs @@ -2,13 +2,14 @@ module Hasura.RQL.DML.Mutation ( Mutation(..) , runMutation , mutateAndFetchCols + , mkSelCTEFromColVals ) where import qualified Data.Sequence as DS -import Hasura.Prelude import Hasura.EncJSON +import Hasura.Prelude import Hasura.RQL.DML.Internal import Hasura.RQL.DML.Returning import Hasura.RQL.DML.Select @@ -26,7 +27,7 @@ data Mutation { _mTable :: !QualifiedTable , _mQuery :: !(S.CTE, DS.Seq Q.PrepArg) , _mFields :: !MutFlds - , _mUniqCols :: !(Maybe [PGColInfo]) + , _mCols :: ![PGColInfo] , _mStrfyNum :: !Bool } deriving (Show, Eq) @@ -44,36 +45,14 @@ mutateAndReturn (Mutation qt (cte, p) mutFlds _ strfyNum) = selWith = mkSelWith qt cte mutFlds False strfyNum mutateAndSel :: Mutation -> Q.TxE QErr EncJSON -mutateAndSel (Mutation qt q mutFlds mUniqCols strfyNum) = do - uniqCols <- onNothing mUniqCols $ - throw500 "uniqCols not found in mutateAndSel" - let colMap = Map.fromList $ flip map uniqCols $ - \ci -> (pgiName ci, ci) +mutateAndSel (Mutation qt q mutFlds allCols strfyNum) = do -- Perform mutation and fetch unique columns - MutateResp _ colVals <- mutateAndFetchCols qt uniqCols q strfyNum - colExps <- mapM (colValToColExp colMap) colVals - let selWhere = S.mkBoolExpWithColVal mkQIdenExp colExps - selCTE = S.CTESelect $ - S.mkSelect - { S.selExtr = [S.selectStar] - , S.selFrom = Just $ S.FromExp [S.FISimple qt Nothing] - , S.selWhere = Just $ S.WhereFrag selWhere - } - selWith = mkSelWith qt selCTE mutFlds False strfyNum + MutateResp _ colVals <- mutateAndFetchCols qt allCols q strfyNum + selCTE <- mkSelCTEFromColVals qt allCols colVals + 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 - colValToColExp colMap colVal = - fmap Map.fromList $ forM (Map.toList colVal) $ - \(pgCol, val) -> do - colInfo <- onNothing (Map.lookup pgCol colMap) $ - throw500 "colInfo not found; colValToColExp" - sqlExp <- runAesonParser (convToTxt (pgiType colInfo)) val - return (pgCol, sqlExp) - - mkQIdenExp col = - S.SEQIden $ S.QIden (S.mkQual qt) $ Iden $ getPGColTxt col mutateAndFetchCols @@ -107,3 +86,32 @@ mutateAndFetchCols qt cols (cte, p) strfyNum = } colSel = S.SESelect $ mkSQLSelect False $ AnnSelG selFlds tabFrom tabPerm noTableArgs strfyNum + +mkSelCTEFromColVals + :: MonadError QErr m + => QualifiedTable -> [PGColInfo] -> [ColVals] -> m S.CTE +mkSelCTEFromColVals qt allCols colVals = + S.CTESelect <$> case colVals of + [] -> return selNoRows + _ -> do + tuples <- mapM mkTupsFromColVal colVals + let fromItem = S.FIValues (S.ValuesExp tuples) tableAls $ Just colNames + return S.mkSelect + { S.selExtr = [S.selectStar] + , S.selFrom = Just $ S.FromExp [fromItem] + } + where + tableAls = S.Alias $ Iden $ snakeCaseQualObject qt + colNames = map pgiName allCols + mkTupsFromColVal colVal = + fmap S.TupleExp $ forM allCols $ \ci -> do + let pgCol = pgiName ci + val <- onNothing (Map.lookup pgCol colVal) $ + throw500 $ "column " <> pgCol <<> " not found in returning values" + runAesonParser (convToTxt (pgiType ci)) val + + selNoRows = + S.mkSelect { S.selExtr = [S.selectStar] + , S.selFrom = Just $ S.mkSimpleFromExp qt + , S.selWhere = Just $ S.WhereFrag $ S.BELit False + } diff --git a/server/src-lib/Hasura/RQL/DML/Update.hs b/server/src-lib/Hasura/RQL/DML/Update.hs index 0ca3cfee..498a9918 100644 --- a/server/src-lib/Hasura/RQL/DML/Update.hs +++ b/server/src-lib/Hasura/RQL/DML/Update.hs @@ -28,11 +28,11 @@ import qualified Hasura.SQL.DML as S data UpdateQueryP1 = UpdateQueryP1 - { uqp1Table :: !QualifiedTable - , uqp1SetExps :: ![(PGCol, S.SQLExp)] - , uqp1Where :: !(AnnBoolExpSQL, AnnBoolExpSQL) - , uqp1MutFlds :: !MutFlds - , uqp1UniqCols :: !(Maybe [PGColInfo]) + { uqp1Table :: !QualifiedTable + , uqp1SetExps :: ![(PGCol, S.SQLExp)] + , uqp1Where :: !(AnnBoolExpSQL, AnnBoolExpSQL) + , uqp1MutFlds :: !MutFlds + , uqp1AllCols :: ![PGColInfo] } deriving (Show, Eq) mkUpdateCTE @@ -48,12 +48,11 @@ mkUpdateCTE (UpdateQueryP1 tn setExps (permFltr, wc) _ _) = getUpdateDeps :: UpdateQueryP1 -> [SchemaDependency] -getUpdateDeps (UpdateQueryP1 tn setExps (_, wc) mutFlds uniqCols) = - mkParentDep tn : colDeps <> uniqColDeps <> whereDeps <> retDeps +getUpdateDeps (UpdateQueryP1 tn setExps (_, wc) mutFlds allCols) = + mkParentDep tn : colDeps <> allColDeps <> whereDeps <> retDeps where colDeps = map (mkColDep "on_type" tn . fst) setExps - uniqColDeps = map (mkColDep "on_type" tn) $ - maybe [] (map pgiName) uniqCols + allColDeps = map (mkColDep "on_type" tn . pgiName) allCols whereDeps = getBoolExpDeps tn wc retDeps = map (mkColDep "untyped" tn . fst) $ pgColsFromMutFlds mutFlds @@ -143,10 +142,9 @@ validateUpdateQueryWith f uq = do askSelPermInfo tableInfo let fieldInfoMap = tiFieldInfoMap tableInfo + allCols = getCols fieldInfoMap preSetObj = upiSet updPerm preSetCols = M.keys preSetObj - uniqCols = getUniqCols (getCols fieldInfoMap) $ - tiUniqOrPrimConstraints tableInfo -- convert the object to SQL set expression setItems <- withPathK "$set" $ @@ -180,7 +178,7 @@ validateUpdateQueryWith f uq = do setExpItems (upiFilter updPerm, annSQLBoolExp) (mkDefaultMutFlds mAnnRetCols) - uniqCols + allCols where mRetCols = uqReturning uq selNecessaryMsg = @@ -198,7 +196,7 @@ updateQueryToTx :: Bool -> (UpdateQueryP1, DS.Seq Q.PrepArg) -> Q.TxE QErr EncJSON updateQueryToTx strfyNum (u, p) = runMutation $ Mutation (uqp1Table u) (updateCTE, p) - (uqp1MutFlds u) (uqp1UniqCols u) strfyNum + (uqp1MutFlds u) (uqp1AllCols u) strfyNum where updateCTE = mkUpdateCTE u diff --git a/server/src-lib/Hasura/RQL/Types/SchemaCache.hs b/server/src-lib/Hasura/RQL/Types/SchemaCache.hs index 8dfb8c00..7d3ac042 100644 --- a/server/src-lib/Hasura/RQL/Types/SchemaCache.hs +++ b/server/src-lib/Hasura/RQL/Types/SchemaCache.hs @@ -7,7 +7,6 @@ module Hasura.RQL.Types.SchemaCache , emptySchemaCache , TableInfo(..) , TableConstraint(..) - , getUniqCols , ConstraintType(..) , ViewInfo(..) , isMutable @@ -310,32 +309,10 @@ data TableConstraint = TableConstraint { tcType :: !ConstraintType , tcName :: !ConstraintName - , tcCols :: ![PGCol] } deriving (Show, Eq) $(deriveJSON (aesonDrop 2 snakeCase) ''TableConstraint) -getUniqCols :: [PGColInfo] -> [TableConstraint] -> Maybe [PGColInfo] -getUniqCols allCols = travConstraints - where - colsNotNull = all (not . pgiIsNullable) - - travConstraints [] = Nothing - travConstraints (h:t) = - let cols = getColInfos (tcCols h) allCols - in case tcType h of - CTPRIMARYKEY -> Just cols - CTUNIQUE -> if colsNotNull cols then Just cols - else travConstraints t - _ -> travConstraints t - -getAllPkeyCols :: [TableConstraint] -> [PGCol] -getAllPkeyCols constraints = - flip concatMap constraints $ - \c -> case tcType c of - CTPRIMARYKEY -> tcCols c - _ -> [] - data ViewInfo = ViewInfo { viIsUpdatable :: !Bool @@ -362,7 +339,7 @@ data TableInfo , tiSystemDefined :: !Bool , tiFieldInfoMap :: !FieldInfoMap , tiRolePermInfoMap :: !RolePermInfoMap - , tiUniqOrPrimConstraints :: ![TableConstraint] + , tiUniqOrPrimConstraints :: ![ConstraintName] , tiPrimaryKeyCols :: ![PGCol] , tiViewInfo :: !(Maybe ViewInfo) , tiEventTriggerInfoMap :: !EventTriggerInfoMap @@ -373,14 +350,14 @@ $(deriveToJSON (aesonDrop 2 snakeCase) ''TableInfo) mkTableInfo :: QualifiedTable -> Bool - -> [TableConstraint] + -> [ConstraintName] -> [PGColInfo] + -> [PGCol] -> Maybe ViewInfo -> TableInfo -mkTableInfo tn isSystemDefined uniqCons cols mVI = +mkTableInfo tn isSystemDefined uniqCons cols pCols mVI = TableInfo tn isSystemDefined colMap (M.fromList []) uniqCons pCols mVI (M.fromList []) where - pCols = getAllPkeyCols uniqCons colMap = M.fromList $ map f cols f colInfo = (fromPGCol $ pgiName colInfo, FIColumn colInfo) diff --git a/server/src-lib/Hasura/SQL/DML.hs b/server/src-lib/Hasura/SQL/DML.hs index 6354398e..fc9a0e48 100644 --- a/server/src-lib/Hasura/SQL/DML.hs +++ b/server/src-lib/Hasura/SQL/DML.hs @@ -250,6 +250,14 @@ instance ToSQL CountType where toSQL (CTDistinct cols) = "DISTINCT" <-> paren (", " <+> cols) +newtype TupleExp + = TupleExp [SQLExp] + deriving (Show, Eq) + +instance ToSQL TupleExp where + toSQL (TupleExp exps) = + paren $ ", " <+> exps + data SQLExp = SEPrep !Int | SELit !T.Text @@ -267,7 +275,7 @@ data SQLExp | SEBool !BoolExp | SEExcluded !T.Text | SEArray ![SQLExp] - | SETuples ![SQLExp] + | SETuple !TupleExp | SECount !CountType deriving (Show, Eq) @@ -324,7 +332,7 @@ instance ToSQL SQLExp where <> toSQL (PGCol t) toSQL (SEArray exps) = "ARRAY" <> TB.char '[' <> (", " <+> exps) <> TB.char ']' - toSQL (SETuples exps) = paren $ ", " <+> exps + toSQL (SETuple tup) = toSQL tup toSQL (SECount ty) = "COUNT" <> paren (toSQL ty) intToSQLExp :: Int -> SQLExp @@ -393,6 +401,7 @@ data FromItem | FIIden !Iden | FIFunc !QualifiedFunction ![SQLExp] !(Maybe Alias) | FISelect !Lateral !Select !Alias + | FIValues !ValuesExp !Alias !(Maybe [PGCol]) | FIJoin !JoinExpr deriving (Show, Eq) @@ -402,6 +411,10 @@ mkSelFromItem = FISelect (Lateral False) mkLateralFromItem :: Select -> Alias -> FromItem mkLateralFromItem = FISelect (Lateral True) +toColTupExp :: [PGCol] -> SQLExp +toColTupExp = + SETuple . TupleExp . map (SEIden . Iden . getPGColTxt) + instance ToSQL FromItem where toSQL (FISimple qt mal) = toSQL qt <-> toSQL mal @@ -411,6 +424,9 @@ instance ToSQL FromItem where toSQL qf <> paren (", " <+> args) <-> toSQL mal toSQL (FISelect mla sel al) = toSQL mla <-> paren (toSQL sel) <-> toSQL al + toSQL (FIValues valsExp al mCols) = + paren (toSQL valsExp) <-> toSQL al + <-> toSQL (toColTupExp <$> mCols) toSQL (FIJoin je) = toSQL je @@ -499,19 +515,6 @@ mkExists fromItem whereFrag = , selWhere = Just $ WhereFrag whereFrag } -mkBoolExpWithColVal - :: (PGCol -> SQLExp) - -> [HM.HashMap PGCol SQLExp] - -> BoolExp -mkBoolExpWithColVal f colValMaps = - case colValMaps of - [] -> BELit False - l@(h:_) -> - let cols = map f $ HM.keys h - colTup = SETuples cols - valTups = map (SETuples . HM.elems) l - in BEIN colTup valTups - instance ToSQL BoolExp where toSQL (BELit True) = TB.text $ T.squote "true" toSQL (BELit False) = TB.text $ T.squote "false" @@ -703,25 +706,31 @@ instance ToSQL SQLConflict where <-> toSQL ct <-> "DO UPDATE" <-> toSQL set <-> toSQL whr +newtype ValuesExp + = ValuesExp [TupleExp] + deriving (Show, Eq) + +instance ToSQL ValuesExp where + toSQL (ValuesExp tuples) = + "VALUES" <-> (", " <+> tuples) + data SQLInsert = SQLInsert { siTable :: !QualifiedTable , siCols :: ![PGCol] - , siTuples :: ![[SQLExp]] + , siValues :: !ValuesExp , siConflict :: !(Maybe SQLConflict) , siRet :: !(Maybe RetExp) } deriving (Show, Eq) instance ToSQL SQLInsert where toSQL si = - let insTuples = flip map (siTuples si) $ \tupVals -> - "(" <-> (", " <+> tupVals) <-> ")" - insConflict = maybe "" toSQL + let insConflict = maybe "" toSQL in "INSERT INTO" <-> toSQL (siTable si) <-> "(" <-> (", " <+> siCols si) - <-> ") VALUES" - <-> (", " <+> insTuples) + <-> ")" + <-> toSQL (siValues si) <-> insConflict (siConflict si) <-> toSQL (siRet si) diff --git a/server/src-lib/Hasura/SQL/Rewrite.hs b/server/src-lib/Hasura/SQL/Rewrite.hs index ef419516..4a4656f6 100644 --- a/server/src-lib/Hasura/SQL/Rewrite.hs +++ b/server/src-lib/Hasura/SQL/Rewrite.hs @@ -96,6 +96,11 @@ uFromItem fromItem = case fromItem of newSel <- restoringIdens $ uSelect sel newAls <- addAlias al return $ S.FISelect isLateral newSel newAls + S.FIValues (S.ValuesExp tups) als mCols -> do + newValExp <- fmap S.ValuesExp $ + forM tups $ \(S.TupleExp ts) -> + S.TupleExp <$> mapM uSqlExp ts + return $ S.FIValues newValExp als mCols S.FIJoin joinExp -> S.FIJoin <$> uJoinExp joinExp @@ -171,7 +176,7 @@ uSqlExp = restoringIdens . \case S.SEExcluded <$> return t S.SEArray l -> S.SEArray <$> mapM uSqlExp l - S.SETuples l -> + S.SETuple (S.TupleExp l) -> S.SEArray <$> mapM uSqlExp l S.SECount cty -> return $ S.SECount cty where diff --git a/server/src-rsr/table_info.sql b/server/src-rsr/table_info.sql index 5a83147c..cf53f85c 100644 --- a/server/src-rsr/table_info.sql +++ b/server/src-rsr/table_info.sql @@ -1,5 +1,6 @@ select coalesce(columns.columns, '[]') as columns, + coalesce(pk.columns, '[]') as primary_key_columns, coalesce(constraints.constraints, '[]') as constraints, coalesce(views.view_info, 'null') as view_info from @@ -27,32 +28,25 @@ from tables.table_schema = columns.table_schema AND tables.table_name = columns.table_name ) + left outer join ( + select * from hdb_catalog.hdb_primary_key + ) pk on ( + tables.table_schema = pk.table_schema + AND tables.table_name = pk.table_name + ) left outer join ( select - cm.table_schema, - cm.table_name, - json_agg( - json_build_object( - 'type', cm.constraint_type, - 'name', cm.constraint_name, - 'cols', cm.columns - ) - ) as constraints + c.table_schema, + c.table_name, + json_agg(constraint_name) as constraints from - ( - select table_name, table_schema, - constraint_name, columns, - 'PRIMARY KEY' as constraint_type - from hdb_catalog.hdb_primary_key - union all - select table_name, table_schema, - constraint_name, columns, - 'UNIQUE' as constraint_type - from hdb_catalog.hdb_unique_constraint - ) cm + information_schema.table_constraints c + where + c.constraint_type = 'UNIQUE' + or c.constraint_type = 'PRIMARY KEY' group by - cm.table_schema, - cm.table_name + c.table_schema, + c.table_name ) constraints on ( tables.table_schema = constraints.table_schema AND tables.table_name = constraints.table_name diff --git a/server/tests-py/queries/graphql_mutation/delete/basic/article_returning_author.yaml b/server/tests-py/queries/graphql_mutation/delete/basic/article_returning_author.yaml new file mode 100644 index 00000000..b8edb10f --- /dev/null +++ b/server/tests-py/queries/graphql_mutation/delete/basic/article_returning_author.yaml @@ -0,0 +1,40 @@ +description: Delete mutation on article with returning columns and author +url: /v1alpha1/graphql +status: 200 +response: + data: + delete_article: + affected_rows: 2 + returning: + - id: 4 + title: Article 4 + content: Sample article content 4 + author: + id: 2 + name: Author 2 + - id: 5 + title: Article 5 + content: Sample article content 5 + author: + id: 2 + name: Author 2 +query: + query: | + mutation DeleteAuthor2Articles { + delete_article( + where: { + author: {id: {_eq: 2}} + } + ) { + affected_rows + returning { + id + title + content + author { + id + name + } + } + } + } diff --git a/server/tests-py/test_graphql_mutations.py b/server/tests-py/test_graphql_mutations.py index 9fd55f7a..2d79a51d 100644 --- a/server/tests-py/test_graphql_mutations.py +++ b/server/tests-py/test_graphql_mutations.py @@ -340,6 +340,9 @@ class TestGraphqlDeleteBasic(DefaultTestQueries): def test_article_delete_returning(self, hge_ctx): check_query_f(hge_ctx, self.dir() + "/article_returning.yaml") + def test_article_delete_returning_author(self, hge_ctx): + check_query_f(hge_ctx, self.dir() + "/article_returning_author.yaml") + @classmethod def dir(cls): return "queries/graphql_mutation/delete/basic"