From 744390e7f825f747ac4d362c650275cc3ed95f7b Mon Sep 17 00:00:00 2001 From: rakeshkky Date: Wed, 22 May 2019 21:34:33 +0530 Subject: [PATCH 1/5] do not consider foreign keys are dropped only if constraint oid changes, fix #2238 --- server/src-lib/Hasura/RQL/DDL/Relationship.hs | 35 +++--- server/src-lib/Hasura/RQL/DDL/Schema/Diff.hs | 74 +++--------- server/src-lib/Hasura/RQL/Types/Catalog.hs | 17 +-- server/src-lib/Hasura/RQL/Types/Common.hs | 15 +++ server/src-rsr/catalog_metadata.sql | 1 + server/src-rsr/table_meta.sql | 109 ++++++++++++++++++ .../nested_select_with_foreign_key_alter.yaml | 47 ++++++++ server/tests-py/test_graphql_queries.py | 4 + 8 files changed, 212 insertions(+), 90 deletions(-) create mode 100644 server/src-rsr/table_meta.sql create mode 100644 server/tests-py/queries/graphql_query/basic/nested_select_with_foreign_key_alter.yaml diff --git a/server/src-lib/Hasura/RQL/DDL/Relationship.hs b/server/src-lib/Hasura/RQL/DDL/Relationship.hs index 6b56484559261..14bf0dc118b84 100644 --- a/server/src-lib/Hasura/RQL/DDL/Relationship.hs +++ b/server/src-lib/Hasura/RQL/DDL/Relationship.hs @@ -23,7 +23,6 @@ import Hasura.RQL.DDL.Deps import Hasura.RQL.DDL.Permission (purgePerm) import Hasura.RQL.DDL.Relationship.Types import Hasura.RQL.Types -import Hasura.RQL.Types.Catalog import Hasura.SQL.Types import Data.Aeson.Types @@ -106,7 +105,7 @@ createObjRelP1 (WithTable qt rd) = do objRelP2Setup :: (QErrM m, CacheRWM m) - => QualifiedTable -> HS.HashSet CatalogFKey -> RelDef ObjRelUsing -> m () + => QualifiedTable -> HS.HashSet ForeignKey -> RelDef ObjRelUsing -> m () objRelP2Setup qt fkeys (RelDef rn ru _) = do (relInfo, deps) <- case ru of RUManual (ObjRelManualConfig rm) -> do @@ -117,8 +116,8 @@ objRelP2Setup qt fkeys (RelDef rn ru _) = do return (RelInfo rn ObjRel (zip lCols rCols) refqt True, deps) RUFKeyOn cn -> do -- TODO: validation should account for this too - CatalogFKey _ refqt consName colMap <- - getRequiredFkey cn fkeys $ \fk -> _cfkTable fk == qt + ForeignKey _ refqt _ consName colMap <- + getRequiredFkey cn fkeys $ \fk -> _fkTable fk == qt let deps = [ SchemaDependency (SOTableObj qt $ TOCons consName) "fkey" , SchemaDependency (SOTableObj qt $ TOCol cn) "using_col" @@ -181,7 +180,7 @@ validateArrRel qt (RelDef rn ru _) = do arrRelP2Setup :: (QErrM m, CacheRWM m) - => QualifiedTable -> HS.HashSet CatalogFKey -> ArrRelDef -> m () + => QualifiedTable -> HS.HashSet ForeignKey -> ArrRelDef -> m () arrRelP2Setup qt fkeys (RelDef rn ru _) = do (relInfo, deps) <- case ru of RUManual (ArrRelManualConfig rm) -> do @@ -192,8 +191,8 @@ arrRelP2Setup qt fkeys (RelDef rn ru _) = do return (RelInfo rn ArrRel (zip lCols rCols) refqt True, deps) RUFKeyOn (ArrRelUsingFKeyOn refqt refCol) -> do -- TODO: validation should account for this too - CatalogFKey _ _ consName colMap <- getRequiredFkey refCol fkeys $ - \fk -> _cfkTable fk == refqt && _cfkRefTable fk == qt + ForeignKey _ _ _ consName colMap <- getRequiredFkey refCol fkeys $ + \fk -> _fkTable fk == refqt && _fkRefTable fk == qt let deps = [ SchemaDependency (SOTableObj refqt $ TOCons consName) "remote_fkey" , SchemaDependency (SOTableObj refqt $ TOCol refCol) "using_col" -- we don't need to necessarily track the remote table like we did in @@ -312,9 +311,9 @@ setRelComment (SetRelComment (QualifiedObject sn tn) rn comment) = getRequiredFkey :: (QErrM m) => PGCol - -> HS.HashSet CatalogFKey - -> (CatalogFKey -> Bool) - -> m CatalogFKey + -> HS.HashSet ForeignKey + -> (ForeignKey -> Bool) + -> m ForeignKey getRequiredFkey col fkeySet preCondition = case filterFkeys of [] -> throw400 ConstraintError @@ -324,32 +323,34 @@ getRequiredFkey col fkeySet preCondition = "more than one foreign key constraint exists on the given column" where filterFkeys = HS.toList $ HS.filter filterFn fkeySet - filterFn k = preCondition k && HM.keys (_cfkColumnMapping k) == [col] + filterFn k = preCondition k && HM.keys (_fkColumnMapping k) == [col] -fetchTableFkeys :: QualifiedTable -> Q.TxE QErr (HS.HashSet CatalogFKey) +fetchTableFkeys :: QualifiedTable -> Q.TxE QErr (HS.HashSet ForeignKey) fetchTableFkeys qt@(QualifiedObject sn tn) = do r <- Q.listQE defaultTxErrorHandler [Q.sql| SELECT f.constraint_name, f.ref_table_table_schema, f.ref_table, + f.constraint_oid, f.column_mapping FROM hdb_catalog.hdb_foreign_key_constraint f WHERE f.table_schema = $1 AND f.table_name = $2 |] (sn, tn) True fmap HS.fromList $ - forM r $ \(constr, refsn, reftn, Q.AltJ colMapping) -> - return $ CatalogFKey qt (QualifiedObject refsn reftn) constr colMapping + forM r $ \(constr, refsn, reftn, oid, Q.AltJ colMapping) -> + return $ ForeignKey qt (QualifiedObject refsn reftn) oid constr colMapping -fetchFkeysAsRemoteTable :: QualifiedTable -> Q.TxE QErr (HS.HashSet CatalogFKey) +fetchFkeysAsRemoteTable :: QualifiedTable -> Q.TxE QErr (HS.HashSet ForeignKey) fetchFkeysAsRemoteTable rqt@(QualifiedObject rsn rtn) = do r <- Q.listQE defaultTxErrorHandler [Q.sql| SELECT f.table_schema, f.table_name, f.constraint_name, + f.constraint_oid, f.column_mapping FROM hdb_catalog.hdb_foreign_key_constraint f WHERE f.ref_table_table_schema = $1 AND f.ref_table = $2 |] (rsn, rtn) True fmap HS.fromList $ - forM r $ \(sn, tn, constr, Q.AltJ colMapping) -> - return $ CatalogFKey (QualifiedObject sn tn) rqt constr colMapping + forM r $ \(sn, tn, constr, oid, Q.AltJ colMapping) -> + return $ ForeignKey (QualifiedObject sn tn) rqt oid constr colMapping diff --git a/server/src-lib/Hasura/RQL/DDL/Schema/Diff.hs b/server/src-lib/Hasura/RQL/DDL/Schema/Diff.hs index d3338298458a1..701351d842be2 100644 --- a/server/src-lib/Hasura/RQL/DDL/Schema/Diff.hs +++ b/server/src-lib/Hasura/RQL/DDL/Schema/Diff.hs @@ -61,64 +61,15 @@ data TableMeta , tmTable :: !QualifiedTable , tmColumns :: ![PGColMeta] , tmConstraints :: ![ConstraintMeta] + , tmForeignKeys :: ![ForeignKey] } deriving (Show, Eq) fetchTableMeta :: Q.Tx [TableMeta] fetchTableMeta = do - res <- Q.listQ [Q.sql| - SELECT - t.table_schema, - t.table_name, - t.table_oid, - coalesce(c.columns, '[]') as columns, - coalesce(f.constraints, '[]') as constraints - FROM - (SELECT - c.oid as table_oid, - c.relname as table_name, - n.nspname as table_schema - FROM - pg_catalog.pg_class c - JOIN - pg_catalog.pg_namespace as n - ON - c.relnamespace = n.oid - ) t - LEFT OUTER JOIN - (SELECT - table_schema, - table_name, - json_agg((SELECT r FROM (SELECT column_name, udt_name AS data_type, ordinal_position, is_nullable::boolean) r)) as columns - FROM - information_schema.columns - GROUP BY - table_schema, table_name) c - ON (t.table_schema = c.table_schema AND t.table_name = c.table_name) - LEFT OUTER JOIN - (SELECT - tc.table_schema, - tc.table_name, - json_agg( - json_build_object( - 'name', tc.constraint_name, - 'oid', r.oid::integer, - 'type', tc.constraint_type - ) - ) as constraints - FROM - information_schema.table_constraints tc - JOIN pg_catalog.pg_constraint r - ON tc.constraint_name = r.conname - GROUP BY - table_schema, table_name) f - ON (t.table_schema = f.table_schema AND t.table_name = f.table_name) - WHERE - t.table_schema NOT LIKE 'pg_%' - 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) + res <- Q.listQ $(Q.sqlFromFile "src-rsr/table_meta.sql") () False + forM res $ \(ts, tn, toid, cols, constrnts, fkeys) -> + return $ TableMeta toid (QualifiedObject ts tn) (Q.getAltJ cols) + (Q.getAltJ constrnts) (Q.getAltJ fkeys) getOverlap :: (Eq k, Hashable k) => (v -> k) -> [v] -> [v] -> [(v, v)] getOverlap getKey left right = @@ -171,9 +122,18 @@ getTableDiff oldtm newtm = alteredCols = flip map (filter (uncurry (/=)) existingCols) $ pcmToPci *** pcmToPci - droppedFKeyConstraints = map cmName $ - filter (isForeignKey . cmType) $ getDifference cmOid - (tmConstraints oldtm) (tmConstraints newtm) + -- foreign keys are considered dropped only if their oid + -- and (ref-table, column mapping) are changed + droppedFKeyConstraints = map _fkConstraint $ HS.toList $ + droppedFKeysWithOid `HS.intersection` droppedFKeysWithUniq + + droppedFKeysWithOid = HS.fromList $ + getDifference _fkOid (tmForeignKeys oldtm) (tmForeignKeys newtm) + + droppedFKeysWithUniq = HS.fromList $ + getDifference mkFKeyUniqId (tmForeignKeys oldtm) (tmForeignKeys newtm) + + mkFKeyUniqId (ForeignKey _ reftn _ _ colMap) = (reftn, colMap) getTableChangeDeps :: (QErrM m, CacheRWM m) diff --git a/server/src-lib/Hasura/RQL/Types/Catalog.hs b/server/src-lib/Hasura/RQL/Types/Catalog.hs index 36ad7fcf1a190..5e1072eddae63 100644 --- a/server/src-lib/Hasura/RQL/Types/Catalog.hs +++ b/server/src-lib/Hasura/RQL/Types/Catalog.hs @@ -15,8 +15,6 @@ import Data.Aeson import Data.Aeson.Casing import Data.Aeson.TH -import qualified Data.HashMap.Strict as HM - data CatalogTable = CatalogTable { _ctTable :: !QualifiedTable @@ -67,19 +65,6 @@ data CatalogFunction } deriving (Show, Eq) $(deriveJSON (aesonDrop 3 snakeCase) ''CatalogFunction) -type ColMapping = HM.HashMap PGCol PGCol - -data CatalogFKey - = CatalogFKey - { _cfkTable :: !QualifiedTable - , _cfkRefTable :: !QualifiedTable - , _cfkConstraint :: !ConstraintName - , _cfkColumnMapping :: !ColMapping - } deriving (Show, Eq, Generic) -$(deriveJSON (aesonDrop 4 snakeCase) ''CatalogFKey) - -instance Hashable CatalogFKey - data CatalogMetadata = CatalogMetadata { _cmTables :: ![CatalogTable] @@ -89,7 +74,7 @@ data CatalogMetadata , _cmEventTriggers :: ![CatalogEventTrigger] , _cmRemoteSchemas :: ![AddRemoteSchemaQuery] , _cmFunctions :: ![CatalogFunction] - , _cmForeignKeys :: ![CatalogFKey] + , _cmForeignKeys :: ![ForeignKey] , _cmAllowlistCollections :: ![CollectionDef] } deriving (Show, Eq) $(deriveJSON (aesonDrop 3 snakeCase) ''CatalogMetadata) diff --git a/server/src-lib/Hasura/RQL/Types/Common.hs b/server/src-lib/Hasura/RQL/Types/Common.hs index 82c7d631c87a7..f2f8498a99667 100644 --- a/server/src-lib/Hasura/RQL/Types/Common.hs +++ b/server/src-lib/Hasura/RQL/Types/Common.hs @@ -16,6 +16,7 @@ module Hasura.RQL.Types.Common , WithTable(..) , ColVals , MutateResp(..) + , ForeignKey(..) ) where import Hasura.Prelude @@ -147,3 +148,17 @@ data MutateResp , _mrReturningColumns :: ![ColVals] } deriving (Show, Eq) $(deriveJSON (aesonDrop 3 snakeCase) ''MutateResp) + +type ColMapping = HM.HashMap PGCol PGCol + +data ForeignKey + = ForeignKey + { _fkTable :: !QualifiedTable + , _fkRefTable :: !QualifiedTable + , _fkOid :: !Int + , _fkConstraint :: !ConstraintName + , _fkColumnMapping :: !ColMapping + } deriving (Show, Eq, Generic) +$(deriveJSON (aesonDrop 3 snakeCase) ''ForeignKey) + +instance Hashable ForeignKey diff --git a/server/src-rsr/catalog_metadata.sql b/server/src-rsr/catalog_metadata.sql index 524524fa3b897..d08976a4a57f2 100644 --- a/server/src-rsr/catalog_metadata.sql +++ b/server/src-rsr/catalog_metadata.sql @@ -179,6 +179,7 @@ from 'schema', f.ref_table_table_schema, 'name', f.ref_table ), + 'oid', f.constraint_oid, 'constraint', f.constraint_name, 'column_mapping', f.column_mapping ) as info diff --git a/server/src-rsr/table_meta.sql b/server/src-rsr/table_meta.sql new file mode 100644 index 0000000000000..b6c539c929177 --- /dev/null +++ b/server/src-rsr/table_meta.sql @@ -0,0 +1,109 @@ +SELECT + t.table_schema, + t.table_name, + t.table_oid, + coalesce(c.columns, '[]') as columns, + coalesce(f.constraints, '[]') as constraints, + coalesce(fk.fkeys, '[]') as foreign_keys +FROM + ( + SELECT + c.oid as table_oid, + c.relname as table_name, + n.nspname as table_schema + FROM + pg_catalog.pg_class c + JOIN pg_catalog.pg_namespace as n ON c.relnamespace = n.oid + ) t + LEFT OUTER JOIN ( + SELECT + table_schema, + table_name, + json_agg( + ( + SELECT + r + FROM + ( + SELECT + column_name, + udt_name AS data_type, + ordinal_position, + is_nullable :: boolean + ) r + ) + ) as columns + FROM + information_schema.columns + GROUP BY + table_schema, + table_name + ) c ON ( + t.table_schema = c.table_schema + AND t.table_name = c.table_name + ) + LEFT OUTER JOIN ( + SELECT + tc.table_schema, + tc.table_name, + json_agg( + json_build_object( + 'name', + tc.constraint_name, + 'oid', + r.oid :: integer, + 'type', + tc.constraint_type + ) + ) as constraints + FROM + information_schema.table_constraints tc + JOIN pg_catalog.pg_constraint r ON tc.constraint_name = r.conname + GROUP BY + table_schema, + table_name + ) f ON ( + t.table_schema = f.table_schema + AND t.table_name = f.table_name + ) + LEFT OUTER JOIN ( + SELECT + f.table_schema, + f.table_name, + json_agg( + json_build_object( + 'table', + json_build_object( + 'schema', + f.table_schema, + 'name', + f.table_name + ), + 'ref_table', + json_build_object( + 'schema', + f.ref_table_table_schema, + 'name', + f.ref_table + ), + 'oid', + f.constraint_oid, + 'constraint', + f.constraint_name, + 'column_mapping', + f.column_mapping + ) + ) as fkeys + FROM + hdb_catalog.hdb_foreign_key_constraint f + GROUP BY + table_schema, + table_name + ) fk ON ( + fk.table_schema = t.table_schema + AND fk.table_name = t.table_name + ) +WHERE + t.table_schema NOT LIKE 'pg_%' + AND t.table_schema <> 'information_schema' + AND t.table_schema <> 'hdb_catalog' diff --git a/server/tests-py/queries/graphql_query/basic/nested_select_with_foreign_key_alter.yaml b/server/tests-py/queries/graphql_query/basic/nested_select_with_foreign_key_alter.yaml new file mode 100644 index 0000000000000..18d530ce4bf2b --- /dev/null +++ b/server/tests-py/queries/graphql_query/basic/nested_select_with_foreign_key_alter.yaml @@ -0,0 +1,47 @@ +- description: Alter foreign key constraint on article table + url: /v1/query + status: 200 + query: + type: run_sql + args: + sql: | + ALTER TABLE article DROP CONSTRAINT article_author_id_fkey, + ADD CONSTRAINT article_author_id_fkey FOREIGN KEY (author_id) REFERENCES author(id); + +- description: Nested select on article + url: /v1/graphql + status: 200 + response: + data: + article: + - id: 1 + title: Article 1 + content: Sample article content 1 + author: + id: 1 + name: Author 1 + - id: 2 + title: Article 2 + content: Sample article content 2 + author: + id: 1 + name: Author 1 + - id: 3 + title: Article 3 + content: Sample article content 3 + author: + id: 2 + name: Author 2 + query: + query: | + query { + article { + id + title + content + author { + id + name + } + } + } diff --git a/server/tests-py/test_graphql_queries.py b/server/tests-py/test_graphql_queries.py index bb94635750e6e..aee5c3119763b 100644 --- a/server/tests-py/test_graphql_queries.py +++ b/server/tests-py/test_graphql_queries.py @@ -46,6 +46,10 @@ def test_select_query_col_not_present_err(self, hge_ctx, transport): def test_select_query_user_col_change(self, hge_ctx, transport): check_query_f(hge_ctx, self.dir() + "/select_query_user_col_change.yaml") + def test_nested_select_with_foreign_key_alter(self, hge_ctx, transport): + transport = 'http' + check_query_f(hge_ctx, self.dir() + "/nested_select_with_foreign_key_alter.yaml", transport) + @classmethod def dir(cls): return 'queries/graphql_query/basic' From 513e7ee4568839b8aa2f185f84218dd9b6f480c0 Mon Sep 17 00:00:00 2001 From: rakeshkky Date: Wed, 22 May 2019 22:50:16 +0530 Subject: [PATCH 2/5] [console] generate combined alter foreign key SQL --- .../Data/TableModify/ModifyActions.js | 70 +++++++++---------- 1 file changed, 33 insertions(+), 37 deletions(-) diff --git a/console/src/components/Services/Data/TableModify/ModifyActions.js b/console/src/components/Services/Data/TableModify/ModifyActions.js index 470bfdf2e567b..7f52ddea496f5 100644 --- a/console/src/components/Services/Data/TableModify/ModifyActions.js +++ b/console/src/components/Services/Data/TableModify/ModifyActions.js @@ -257,58 +257,54 @@ const saveForeignKeys = (index, tableSchema, columns) => { } const lcols = filteredMappings.map(cm => `"${columns[cm.column].name}"`); const rcols = filteredMappings.map(cm => `"${cm.refColumn}"`); + const migrationUp = []; if (constraintName) { + const generatedConstraintName = generateFKConstraintName( + tableName, + lcols, + tableSchema.foreign_key_constraints + ); + const migrationUpAlterFKeySql =` + alter table "${schemaName}"."${tableName}" drop constraint "${constraintName}", + add constraint "${constraintName || generatedConstraintName}" foreign key (${lcols.join( + ', ' + )}) references "${schemaName}"."${refTableName}"(${rcols.join( + ', ' + )}) on update ${onUpdate} on delete ${onDelete}; + `; + migrationUp.push({ type: 'run_sql', args: { - sql: `alter table "${schemaName}"."${tableName}" drop constraint "${constraintName}";`, + sql: migrationUpAlterFKeySql, }, }); } - const generatedConstraintName = generateFKConstraintName( - tableName, - lcols, - tableSchema.foreign_key_constraints - ); + const migrationDown = []; - migrationUp.push({ - type: 'run_sql', - args: { - sql: `alter table "${schemaName}"."${tableName}" add constraint "${constraintName || - generatedConstraintName}" foreign key (${lcols.join( - ', ' - )}) references "${schemaName}"."${refTableName}"(${rcols.join( - ', ' - )}) on update ${onUpdate} on delete ${onDelete};`, - }, - }); - const migrationDown = [ - { - type: 'run_sql', - args: { - sql: `alter table "${schemaName}"."${tableName}" drop constraint "${constraintName || - generatedConstraintName}";`, - }, - }, - ]; if (constraintName) { const oldConstraint = tableSchema.foreign_key_constraints[index]; + const migrationDownAlterFKeySql = ` + alter table "${schemaName}"."${tableName}" drop constraint "${constraintName || generatedConstraintName}", + add constraint "${constraintName}" foreign key (${Object.keys( + oldConstraint.column_mapping + ) + .map(lc => `"${lc}"`) + .join(', ')}) references "${ + oldConstraint.ref_table + }"(${Object.values(oldConstraint.column_mapping) + .map(rc => `"${rc}"`) + .join(', ')}) on update ${ + pgConfTypes[oldConstraint.on_update] + } on delete ${pgConfTypes[oldConstraint.on_delete]}; + `; + migrationDown.push({ type: 'run_sql', args: { - sql: `alter table "${schemaName}"."${tableName}" add constraint "${constraintName}" foreign key (${Object.keys( - oldConstraint.column_mapping - ) - .map(lc => `"${lc}"`) - .join(', ')}) references "${ - oldConstraint.ref_table - }"(${Object.values(oldConstraint.column_mapping) - .map(rc => `"${rc}"`) - .join(', ')}) on update ${ - pgConfTypes[oldConstraint.on_update] - } on delete ${pgConfTypes[oldConstraint.on_delete]};`, + sql: migrationDownAlterFKeySql, }, }); } From 3a64cbb563d78b362bc0bb7233fb70c6d5553624 Mon Sep 17 00:00:00 2001 From: rakeshkky Date: Fri, 24 May 2019 14:32:08 +0530 Subject: [PATCH 3/5] [console] fix create a new foreign key --- .../Data/TableModify/ModifyActions.js | 52 +++++++++++++++---- 1 file changed, 42 insertions(+), 10 deletions(-) diff --git a/console/src/components/Services/Data/TableModify/ModifyActions.js b/console/src/components/Services/Data/TableModify/ModifyActions.js index 7f52ddea496f5..54773d952f27d 100644 --- a/console/src/components/Services/Data/TableModify/ModifyActions.js +++ b/console/src/components/Services/Data/TableModify/ModifyActions.js @@ -259,18 +259,20 @@ const saveForeignKeys = (index, tableSchema, columns) => { const rcols = filteredMappings.map(cm => `"${cm.refColumn}"`); const migrationUp = []; + const generatedConstraintName = generateFKConstraintName( + tableName, + lcols, + tableSchema.foreign_key_constraints + ); + if (constraintName) { - const generatedConstraintName = generateFKConstraintName( - tableName, - lcols, - tableSchema.foreign_key_constraints - ); - const migrationUpAlterFKeySql =` + // foreign key already exists, alter the foreign key + const migrationUpAlterFKeySql = ` alter table "${schemaName}"."${tableName}" drop constraint "${constraintName}", - add constraint "${constraintName || generatedConstraintName}" foreign key (${lcols.join( - ', ' + add constraint "${constraintName}" foreign key (${lcols.join( + ', ' )}) references "${schemaName}"."${refTableName}"(${rcols.join( - ', ' + ', ' )}) on update ${onUpdate} on delete ${onDelete}; `; @@ -280,14 +282,31 @@ const saveForeignKeys = (index, tableSchema, columns) => { sql: migrationUpAlterFKeySql, }, }); + } else { + // foreign key not found, create a new one + const migrationUpCreateFKeySql = ` + alter table "${schemaName}"."${tableName}" + add constraint "${generatedConstraintName}" foreign key (${lcols.join( + ', ' + )}) references "${schemaName}"."${refTableName}"(${rcols.join( + ', ' + )}) on update ${onUpdate} on delete ${onDelete}; + `; + migrationUp.push({ + type: 'run_sql', + args: { + sql: migrationUpCreateFKeySql, + }, + }); } const migrationDown = []; if (constraintName) { + // when foreign key is altered const oldConstraint = tableSchema.foreign_key_constraints[index]; const migrationDownAlterFKeySql = ` - alter table "${schemaName}"."${tableName}" drop constraint "${constraintName || generatedConstraintName}", + alter table "${schemaName}"."${tableName}" drop constraint "${constraintName}", add constraint "${constraintName}" foreign key (${Object.keys( oldConstraint.column_mapping ) @@ -307,7 +326,20 @@ const saveForeignKeys = (index, tableSchema, columns) => { sql: migrationDownAlterFKeySql, }, }); + } else { + // when foreign key is created + const migrationDownDeleteFKeySql = ` + alter table "${schemaName}"."${tableName}" drop constraint "${generatedConstraintName}" + `; + + migrationDown.push({ + type: 'run_sql', + args: { + sql: migrationDownDeleteFKeySql, + }, + }); } + const migrationName = `set_fk_${schemaName}_${tableName}_${lcols.join( '_' )}`; From eda89acd80bc281512b62af2c9e34f85f652eb06 Mon Sep 17 00:00:00 2001 From: rikinsk Date: Fri, 31 May 2019 23:56:53 +0530 Subject: [PATCH 4/5] fix sqls --- .../Data/TableModify/ModifyActions.js | 66 +++++++++---------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/console/src/components/Services/Data/TableModify/ModifyActions.js b/console/src/components/Services/Data/TableModify/ModifyActions.js index 4d1010c34c6f9..673217c0d2ee1 100644 --- a/console/src/components/Services/Data/TableModify/ModifyActions.js +++ b/console/src/components/Services/Data/TableModify/ModifyActions.js @@ -275,11 +275,10 @@ const saveForeignKeys = (index, tableSchema, columns) => { // foreign key already exists, alter the foreign key const migrationUpAlterFKeySql = ` alter table "${schemaName}"."${tableName}" drop constraint "${constraintName}", - add constraint "${constraintName}" foreign key (${lcols.join( - ', ' - )}) references "${schemaName}"."${refTableName}"(${rcols.join( - ', ' - )}) on update ${onUpdate} on delete ${onDelete}; + add constraint "${constraintName}" + foreign key (${lcols.join(', ')}) + references "${refSchemaName}"."${refTableName}" + (${rcols.join(', ')}) on update ${onUpdate} on delete ${onDelete}; `; migrationUp.push({ @@ -292,12 +291,12 @@ const saveForeignKeys = (index, tableSchema, columns) => { // foreign key not found, create a new one const migrationUpCreateFKeySql = ` alter table "${schemaName}"."${tableName}" - add constraint "${generatedConstraintName}" foreign key (${lcols.join( - ', ' - )}) references "${schemaName}"."${refTableName}"(${rcols.join( - ', ' - )}) on update ${onUpdate} on delete ${onDelete}; + add constraint "${generatedConstraintName}" + foreign key (${lcols.join(', ')}) + references "${refSchemaName}"."${refTableName}" + (${rcols.join(', ')}) on update ${onUpdate} on delete ${onDelete}; `; + migrationUp.push({ type: 'run_sql', args: { @@ -313,17 +312,18 @@ const saveForeignKeys = (index, tableSchema, columns) => { const oldConstraint = tableSchema.foreign_key_constraints[index]; const migrationDownAlterFKeySql = ` alter table "${schemaName}"."${tableName}" drop constraint "${constraintName}", - add constraint "${constraintName}" foreign key (${Object.keys( - oldConstraint.column_mapping - ) - .map(lc => `"${lc}"`) - .join(', ')}) references "${ - oldConstraint.ref_table - }"(${Object.values(oldConstraint.column_mapping) - .map(rc => `"${rc}"`) - .join(', ')}) on update ${ - pgConfTypes[oldConstraint.on_update] - } on delete ${pgConfTypes[oldConstraint.on_delete]}; + add constraint "${constraintName}" + foreign key (${Object.keys(oldConstraint.column_mapping) + .map(lc => `"${lc}"`) + .join(', ')}) + references "${oldConstraint.ref_table_table_schema}"."${ + oldConstraint.ref_table + }" + (${Object.values(oldConstraint.column_mapping) + .map(rc => `"${rc}"`) + .join(', ')}) + on update ${pgConfTypes[oldConstraint.on_update]} + on delete ${pgConfTypes[oldConstraint.on_delete]}; `; migrationDown.push({ @@ -1205,24 +1205,24 @@ const saveColumnChangesSql = (colName, column) => { const schemaChangesUp = originalColType !== colType ? [ - { - type: 'run_sql', - args: { - sql: columnChangesUpQuery, + { + type: 'run_sql', + args: { + sql: columnChangesUpQuery, + }, }, - }, - ] + ] : []; const schemaChangesDown = originalColType !== colType ? [ - { - type: 'run_sql', - args: { - sql: columnChangesDownQuery, + { + type: 'run_sql', + args: { + sql: columnChangesDownQuery, + }, }, - }, - ] + ] : []; /* column default up/down migration */ From 60e29f20d5bcecb8261c822b5ed8d2b873d01c13 Mon Sep 17 00:00:00 2001 From: rikinsk Date: Mon, 3 Jun 2019 15:04:51 +0530 Subject: [PATCH 5/5] fix foreign key names in edit sql --- .../Data/TableModify/ModifyActions.js | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/console/src/components/Services/Data/TableModify/ModifyActions.js b/console/src/components/Services/Data/TableModify/ModifyActions.js index 673217c0d2ee1..88034fef70c9f 100644 --- a/console/src/components/Services/Data/TableModify/ModifyActions.js +++ b/console/src/components/Services/Data/TableModify/ModifyActions.js @@ -275,7 +275,7 @@ const saveForeignKeys = (index, tableSchema, columns) => { // foreign key already exists, alter the foreign key const migrationUpAlterFKeySql = ` alter table "${schemaName}"."${tableName}" drop constraint "${constraintName}", - add constraint "${constraintName}" + add constraint "${generatedConstraintName}" foreign key (${lcols.join(', ')}) references "${refSchemaName}"."${refTableName}" (${rcols.join(', ')}) on update ${onUpdate} on delete ${onDelete}; @@ -311,17 +311,17 @@ const saveForeignKeys = (index, tableSchema, columns) => { // when foreign key is altered const oldConstraint = tableSchema.foreign_key_constraints[index]; const migrationDownAlterFKeySql = ` - alter table "${schemaName}"."${tableName}" drop constraint "${constraintName}", + alter table "${schemaName}"."${tableName}" drop constraint "${generatedConstraintName}", add constraint "${constraintName}" foreign key (${Object.keys(oldConstraint.column_mapping) - .map(lc => `"${lc}"`) - .join(', ')}) + .map(lc => `"${lc}"`) + .join(', ')}) references "${oldConstraint.ref_table_table_schema}"."${ - oldConstraint.ref_table - }" + oldConstraint.ref_table +}" (${Object.values(oldConstraint.column_mapping) - .map(rc => `"${rc}"`) - .join(', ')}) + .map(rc => `"${rc}"`) + .join(', ')}) on update ${pgConfTypes[oldConstraint.on_update]} on delete ${pgConfTypes[oldConstraint.on_delete]}; `; @@ -1205,24 +1205,24 @@ const saveColumnChangesSql = (colName, column) => { const schemaChangesUp = originalColType !== colType ? [ - { - type: 'run_sql', - args: { - sql: columnChangesUpQuery, - }, + { + type: 'run_sql', + args: { + sql: columnChangesUpQuery, }, - ] + }, + ] : []; const schemaChangesDown = originalColType !== colType ? [ - { - type: 'run_sql', - args: { - sql: columnChangesDownQuery, - }, + { + type: 'run_sql', + args: { + sql: columnChangesDownQuery, }, - ] + }, + ] : []; /* column default up/down migration */