From a7e2d6dcca4abc2c3d6f0789f6aae0e02ca28bb8 Mon Sep 17 00:00:00 2001 From: rakeshkky <12475069+rakeshkky@users.noreply.github.com> Date: Tue, 24 Sep 2019 18:17:26 +0530 Subject: [PATCH 1/2] update custom column names on altering/dropping columns --- .../src-lib/Hasura/RQL/DDL/Schema/Rename.hs | 21 +++++++++ server/src-lib/Hasura/RQL/DDL/Schema/Table.hs | 40 +++++++++++----- .../set_table_custom_fields/alter_column.yaml | 47 +++++++++++++++++++ server/tests-py/test_v1_queries.py | 3 ++ 4 files changed, 98 insertions(+), 13 deletions(-) create mode 100644 server/tests-py/queries/v1/set_table_custom_fields/alter_column.yaml diff --git a/server/src-lib/Hasura/RQL/DDL/Schema/Rename.hs b/server/src-lib/Hasura/RQL/DDL/Schema/Rename.hs index 919a861e9d295..4f5d90eda7e5b 100644 --- a/server/src-lib/Hasura/RQL/DDL/Schema/Rename.hs +++ b/server/src-lib/Hasura/RQL/DDL/Schema/Rename.hs @@ -16,6 +16,7 @@ import qualified Hasura.RQL.DDL.EventTrigger as DS import Hasura.RQL.DDL.Permission import Hasura.RQL.DDL.Permission.Internal import Hasura.RQL.DDL.Relationship.Types +import Hasura.RQL.DDL.Schema.Catalog import Hasura.RQL.Types import Hasura.SQL.Types @@ -97,6 +98,8 @@ renameColInCatalog oCol nCol qt ti = do SOTableObj _ (TOTrigger triggerName) -> updateColInEventTriggerDef triggerName $ RenameItem qt oCol nCol d -> otherDeps errMsg d + -- Update custom column names + possiblyUpdateCustomColumnNames qt oCol nCol where errMsg = "cannot rename column " <> oCol <<> " to " <>> nCol assertFldNotExists = @@ -414,6 +417,16 @@ updateColMap fromQT toQT rnCol colMap = RenameItem qt oCol nCol = rnCol modCol colQt col = if colQt == qt && col == oCol then nCol else col +possiblyUpdateCustomColumnNames + :: MonadTx m => QualifiedTable -> PGCol -> PGCol -> m () +possiblyUpdateCustomColumnNames qt oCol nCol = do + TableConfig customRootFields customColumns <- liftTx $ getTableConfig qt + let updatedCustomColumns = + M.fromList $ flip map (M.toList customColumns) $ + \(dbCol, val) -> (, val) $ if dbCol == oCol then nCol else dbCol + when (updatedCustomColumns /= customColumns) $ + updateTableConfig qt $ TableConfig customRootFields updatedCustomColumns + -- database functions for relationships getRelDef :: QualifiedTable -> RelName -> Q.TxE QErr Value getRelDef (QualifiedObject sn tn) rn = @@ -433,3 +446,11 @@ updateRel (QualifiedObject sn tn) rn relDef = AND table_name = $3 AND rel_name = $4 |] (Q.AltJ relDef, sn , tn, rn) True + +getTableConfig :: QualifiedTable -> Q.TxE QErr TableConfig +getTableConfig (QualifiedObject sn tn) = + Q.getAltJ . runIdentity . Q.getRow <$> Q.withQE defaultTxErrorHandler + [Q.sql| + SELECT configuration::json FROM hdb_catalog.hdb_table + WHERE table_schema = $1 AND table_name = $2 + |] (sn, tn) True diff --git a/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs b/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs index f426453856863..a2542247f34bb 100644 --- a/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs +++ b/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs @@ -190,9 +190,8 @@ runSetTableCustomFieldsQV2 => SetTableCustomFields -> m EncJSON runSetTableCustomFieldsQV2 (SetTableCustomFields tableName rootFields columnNames) = do adminOnly - tableInfo <- askTabInfo tableName + void $ askTabInfo tableName let tableConfig = TableConfig rootFields columnNames - validateTableConfig tableInfo tableConfig updateTableConfig tableName tableConfig buildSchemaCacheFor (MOTable tableName) return successMsg @@ -249,30 +248,31 @@ processTableChanges ti tableDiff = do -- process dropped/added columns, because schema reload happens eventually sc <- askSchemaCache let tn = _tiName ti - withOldTabName = do + withOldTabName ccn = do replaceConstraints tn -- replace description replaceDescription tn -- for all the dropped columns procDroppedCols tn - procAddedCols tn - procAlteredCols sc tn + procAddedCols ccn tn + procAlteredCols sc ccn tn - withNewTabName newTN = do + withNewTabName ccn newTN = do let tnGQL = GS.qualObjectToName newTN defGCtx = scDefaultRemoteGCtx sc -- check for GraphQL schema conflicts on new name GS.checkConflictingNode defGCtx tnGQL - void $ procAlteredCols sc tn + void $ procAlteredCols sc ccn tn -- update new table in catalog renameTableInCatalog newTN tn return True - maybe withOldTabName withNewTabName mNewName + -- Drop custom column names for dropped columns + customColumnNames <- possiblyDropCustomColumnNames tn + maybe (withOldTabName customColumnNames) (withNewTabName customColumnNames) mNewName where TableDiff mNewName droppedCols addedCols alteredCols _ constraints descM = tableDiff - customFields = _tcCustomColumnNames $ _tiCustomConfig ti replaceConstraints tn = flip modTableInCache tn $ \tInfo -> return $ tInfo {_tiUniqOrPrimConstraints = constraints} @@ -284,7 +284,20 @@ processTableChanges ti tableDiff = do -- Drop the column from the cache delColFromCache droppedCol tn - procAddedCols tn = + possiblyDropCustomColumnNames tn = do + let TableConfig customFields customColumnNames = _tiCustomConfig ti + modifiedCustomColumnNames = foldl (flip M.delete) customColumnNames droppedCols + if modifiedCustomColumnNames == customColumnNames then + pure customColumnNames + else do + let updatedTableConfig = + TableConfig customFields modifiedCustomColumnNames + flip modTableInCache tn $ \tInfo -> + pure $ tInfo{_tiCustomConfig = updatedTableConfig} + liftTx $ updateTableConfig tn updatedTableConfig + pure modifiedCustomColumnNames + + procAddedCols customColumnNames tn = -- In the newly added columns check that there is no conflict with relationships forM_ addedCols $ \rawInfo -> do let colName = prciName rawInfo @@ -294,14 +307,14 @@ processTableChanges ti tableDiff = do <<> " in table " <> tn <<> " as a relationship with the name already exists" _ -> do - info <- processColumnInfoUsingCache tn customFields rawInfo + info <- processColumnInfoUsingCache tn customColumnNames rawInfo addColToCache colName info tn - procAlteredCols sc tn = fmap or $ forM alteredCols $ + procAlteredCols sc customColumnNames tn = fmap or $ forM alteredCols $ \( PGRawColumnInfo oldName oldType _ _ _ , newRawInfo@(PGRawColumnInfo newName newType _ _ _) ) -> do let performColumnUpdate = do - newInfo <- processColumnInfoUsingCache tn customFields newRawInfo + newInfo <- processColumnInfoUsingCache tn customColumnNames newRawInfo updColInCache newName newInfo tn if | oldName /= newName -> renameColInCatalog oldName newName tn ti $> True @@ -404,6 +417,7 @@ buildTableCache = processTableCache <=< buildRawTableCache where enumTables = M.mapMaybe _tiEnumValues rawTables + -- | “Processes” a 'PGRawColumnInfo' into a 'PGColumnInfo' by resolving its type using a map of known -- enum tables. processColumnInfo diff --git a/server/tests-py/queries/v1/set_table_custom_fields/alter_column.yaml b/server/tests-py/queries/v1/set_table_custom_fields/alter_column.yaml new file mode 100644 index 0000000000000..85a687f92974e --- /dev/null +++ b/server/tests-py/queries/v1/set_table_custom_fields/alter_column.yaml @@ -0,0 +1,47 @@ +- description: Set custom column names + url: /v1/query + status: 200 + response: + message: success + query: + type: set_table_custom_fields + version: 2 + args: + table: author + custom_root_fields: + select: Authors + custom_column_names: + id: AuthorId + name: AuthorName + +- description: "Rename column 'id' and drop column 'name'" + url: /v1/query + status: 200 + response: + result_type: CommandOk + result: null + query: + type: run_sql + args: + sql: | + ALTER TABLE author DROP COLUMN name; + ALTER TABLE author RENAME COLUMN id to author_id; + +- description: Test if custom column names are updated + url: /v1/graphql + status: 200 + response: + data: + Authors: + - AuthorId: 1 + age: 23 + - AuthorId: 2 + age: null + query: + query: | + query { + Authors{ + AuthorId + age + } + } diff --git a/server/tests-py/test_v1_queries.py b/server/tests-py/test_v1_queries.py index 9688285823aa3..ccb9e0ecce287 100644 --- a/server/tests-py/test_v1_queries.py +++ b/server/tests-py/test_v1_queries.py @@ -646,3 +646,6 @@ def test_set_and_unset(self, hge_ctx): def test_set_invalid_table(self, hge_ctx): check_query_f(hge_ctx, self.dir() + '/set_invalid_table.yaml') + + def test_alter_column(self, hge_ctx): + check_query_f(hge_ctx, self.dir() + '/alter_column.yaml') From 53b7760a5bfcc50279928b420ad93cd5b8b7701e Mon Sep 17 00:00:00 2001 From: rakeshkky <12475069+rakeshkky@users.noreply.github.com> Date: Thu, 26 Sep 2019 11:41:34 +0530 Subject: [PATCH 2/2] no-op refactor; resolve review comments As suggested by @lexi-lambda --- server/src-lib/Hasura/RQL/DDL/Schema/Catalog.hs | 9 +++++++++ server/src-lib/Hasura/RQL/DDL/Schema/Rename.hs | 10 +--------- server/src-lib/Hasura/RQL/DDL/Schema/Table.hs | 2 +- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/server/src-lib/Hasura/RQL/DDL/Schema/Catalog.hs b/server/src-lib/Hasura/RQL/DDL/Schema/Catalog.hs index 8504f97147db5..f14f4985cad9c 100644 --- a/server/src-lib/Hasura/RQL/DDL/Schema/Catalog.hs +++ b/server/src-lib/Hasura/RQL/DDL/Schema/Catalog.hs @@ -6,6 +6,7 @@ module Hasura.RQL.DDL.Schema.Catalog , updateTableIsEnumInCatalog , updateTableConfig , deleteTableFromCatalog + , getTableConfig ) where import Hasura.Prelude @@ -56,3 +57,11 @@ deleteTableFromCatalog (QualifiedObject sn tn) = liftTx $ Q.unitQE defaultTxErro DELETE FROM "hdb_catalog"."hdb_table" WHERE table_schema = $1 AND table_name = $2 |] (sn, tn) False + +getTableConfig :: MonadTx m => QualifiedTable -> m TableConfig +getTableConfig (QualifiedObject sn tn) = liftTx $ + Q.getAltJ . runIdentity . Q.getRow <$> Q.withQE defaultTxErrorHandler + [Q.sql| + SELECT configuration::json FROM hdb_catalog.hdb_table + WHERE table_schema = $1 AND table_name = $2 + |] (sn, tn) True diff --git a/server/src-lib/Hasura/RQL/DDL/Schema/Rename.hs b/server/src-lib/Hasura/RQL/DDL/Schema/Rename.hs index 4f5d90eda7e5b..84223ed427dcd 100644 --- a/server/src-lib/Hasura/RQL/DDL/Schema/Rename.hs +++ b/server/src-lib/Hasura/RQL/DDL/Schema/Rename.hs @@ -420,7 +420,7 @@ updateColMap fromQT toQT rnCol colMap = possiblyUpdateCustomColumnNames :: MonadTx m => QualifiedTable -> PGCol -> PGCol -> m () possiblyUpdateCustomColumnNames qt oCol nCol = do - TableConfig customRootFields customColumns <- liftTx $ getTableConfig qt + TableConfig customRootFields customColumns <- getTableConfig qt let updatedCustomColumns = M.fromList $ flip map (M.toList customColumns) $ \(dbCol, val) -> (, val) $ if dbCol == oCol then nCol else dbCol @@ -446,11 +446,3 @@ updateRel (QualifiedObject sn tn) rn relDef = AND table_name = $3 AND rel_name = $4 |] (Q.AltJ relDef, sn , tn, rn) True - -getTableConfig :: QualifiedTable -> Q.TxE QErr TableConfig -getTableConfig (QualifiedObject sn tn) = - Q.getAltJ . runIdentity . Q.getRow <$> Q.withQE defaultTxErrorHandler - [Q.sql| - SELECT configuration::json FROM hdb_catalog.hdb_table - WHERE table_schema = $1 AND table_name = $2 - |] (sn, tn) True diff --git a/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs b/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs index a2542247f34bb..71b1c5c9625f6 100644 --- a/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs +++ b/server/src-lib/Hasura/RQL/DDL/Schema/Table.hs @@ -286,7 +286,7 @@ processTableChanges ti tableDiff = do possiblyDropCustomColumnNames tn = do let TableConfig customFields customColumnNames = _tiCustomConfig ti - modifiedCustomColumnNames = foldl (flip M.delete) customColumnNames droppedCols + modifiedCustomColumnNames = foldl' (flip M.delete) customColumnNames droppedCols if modifiedCustomColumnNames == customColumnNames then pure customColumnNames else do