hasura · 0x777 · Jun 3, 2019 · May 22, 2019 · May 22, 2019 · May 22, 2019
@@ -263,63 +263,89 @@ const saveForeignKeys = (index, tableSchema, columns) => {
     }
     const lcols = filteredMappings.map(cm => `"${columns[cm.column].name}"`);
     const rcols = filteredMappings.map(cm => `"${cm.refColumn}"`);
+
     const migrationUp = [];
+    const generatedConstraintName = generateFKConstraintName(
+      tableName,
+      lcols,
+      tableSchema.foreign_key_constraints
+    );
+
     if (constraintName) {
+      // foreign key already exists, alter the foreign key
+      const migrationUpAlterFKeySql = `
+             alter table "${schemaName}"."${tableName}" drop constraint "${constraintName}",
+             add constraint "${generatedConstraintName}" 
+             foreign key (${lcols.join(', ')}) 
+             references "${refSchemaName}"."${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
-    );
+    } else {
+      // foreign key not found, create a new one
+      const migrationUpCreateFKeySql = `
+           alter table "${schemaName}"."${tableName}"
+           add constraint "${generatedConstraintName}" 
+           foreign key (${lcols.join(', ')}) 
+           references "${refSchemaName}"."${refTableName}"
+           (${rcols.join(', ')}) on update ${onUpdate} on delete ${onDelete};
+      `;
 
-    migrationUp.push({
-      type: 'run_sql',
-      args: {
-        sql: `alter table "${schemaName}"."${tableName}" add constraint "${constraintName ||
-          generatedConstraintName}" foreign key (${lcols.join(
-          ', '
-        )}) references "${refSchemaName}"."${refTableName}"(${rcols.join(
-          ', '
-        )}) on update ${onUpdate} on delete ${onDelete};`,
-      },
-    });
-    const migrationDown = [
-      {
+      migrationUp.push({
         type: 'run_sql',
         args: {
-          sql: `alter table "${schemaName}"."${tableName}" drop constraint "${constraintName ||
-            generatedConstraintName}";`,
+          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 "${generatedConstraintName}",
+          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({
         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_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]};`,
+          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(
       '_'
     )}`;

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

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