-
Notifications
You must be signed in to change notification settings - Fork 189
Add support for foreign key constraints and foreign key actions #791
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| {-# OPTIONS_GHC -fno-warn-orphans #-} | ||
| {-# LANGUAGE GADTs #-} | ||
| {-# LANGUAGE TupleSections #-} | ||
| {-# LANGUAGE BangPatterns #-} | ||
| {-# LANGUAGE ScopedTypeVariables #-} | ||
|
|
@@ -78,6 +79,7 @@ module Database.Beam.Migrate.Actions | |
| , ensuringNot_ | ||
| , justOne_ | ||
|
|
||
| , ForeignKeySupport(..) | ||
| , createSchemaActionProvider | ||
| , createTableActionProvider | ||
| , dropTableActionProvider | ||
|
|
@@ -107,6 +109,7 @@ import Control.Monad | |
| import Control.Parallel.Strategies | ||
|
|
||
| import Data.Foldable | ||
| import qualified Data.List.NonEmpty as NE | ||
|
|
||
| import qualified Data.HashMap.Strict as HM | ||
| import qualified Data.HashSet as HS | ||
|
|
@@ -279,6 +282,16 @@ createIndexWeight, dropIndexWeight :: Int | |
| createIndexWeight = 200 | ||
| dropIndexWeight = 50 | ||
|
|
||
| -- | Witness for whether a backend's table constraint syntax supports | ||
| -- @FOREIGN KEY@ clauses in @CREATE TABLE@ statements. | ||
| data ForeignKeySupport be where | ||
| -- | The backend supports @FOREIGN KEY@ constraints in @CREATE TABLE@. | ||
| ForeignKeySupported | ||
| :: IsSql92ForeignKeyTableConstraintSyntax (BeamSqlBackendTableConstraintSyntax be) | ||
| => ForeignKeySupport be | ||
| -- | The backend does not support @FOREIGN KEY@ constraints. | ||
| ForeignKeyUnsupported :: ForeignKeySupport be | ||
|
|
||
| -- | Proceeds only if no predicate matches the given pattern. See the | ||
| -- implementation of 'dropTableActionProvider' for an example of usage. | ||
| ensuringNot_ :: Alternative m => [ a ] -> m () | ||
|
|
@@ -342,10 +355,13 @@ dropSchemaActionProvider = | |
| ("Drop schema " <> preSchemaNm) dropSchemaWeight) | ||
|
|
||
| -- | Action provider for SQL92 @CREATE TABLE@ actions. | ||
| createTableActionProvider :: forall be | ||
| . ( Typeable be, BeamMigrateOnlySqlBackend be ) | ||
| => ActionProvider be | ||
| createTableActionProvider = | ||
| createTableActionProvider | ||
| :: forall be | ||
| . ( Typeable be, BeamMigrateOnlySqlBackend be ) | ||
| => ForeignKeySupport be | ||
| -- ^ a witness of whether the backend supports FOREIGN KEY constraints | ||
| -> ActionProvider be | ||
| createTableActionProvider fkSupport = | ||
| ActionProvider provider | ||
| where | ||
| provider :: ActionProviderFn be | ||
|
|
@@ -377,9 +393,45 @@ createTableActionProvider = | |
| guard (tblNm == postTblNm) | ||
| pure (primaryKeyP, primaryKey) | ||
|
|
||
| let postConditions = [ p tblP, p primaryKeyP ] ++ concat columnsP | ||
| let (fkPs, fkConstraints) = case fkSupport of | ||
| ForeignKeySupported -> | ||
| unzip | ||
| [ ( p fkP | ||
| , foreignKeyConstraintSyntax localCols (qnameAsText refTbl) refCols onUpd onDel ) | ||
| | fkP@(TableHasForeignKey tblNm localCols refTbl refCols onUpd onDel) | ||
| <- findPostConditions | ||
| , tblNm == postTblNm | ||
| ] | ||
| ForeignKeyUnsupported -> ([], []) | ||
|
|
||
| -- For each foreign key constraint, ensure the referenced table | ||
| -- already exists in the current state. | ||
| -- This enforces a topological ordering, so that the solver schedules | ||
| -- CREATE TABLE for all referenced tables first. | ||
| -- | ||
| -- Note: this does not support cycles in the foreign key reference graph, | ||
| -- for which we would need ALTER TABLE ADD CONSTRAINT, which we don't | ||
| -- currently support. | ||
|
Comment on lines
+412
to
+414
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does the current design prevent adding this support in a later patch? What would be needed to get it working?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think the current patch would prevent that enhancement. I believe we would need to:
|
||
| case fkSupport of | ||
| ForeignKeyUnsupported -> pure () | ||
| ForeignKeySupported -> do | ||
| let refTbls = | ||
| [ refTbl | ||
| | TableHasForeignKey tblNm _ refTbl _ _ _ <- findPostConditions | ||
| , tblNm == postTblNm | ||
| , refTbl /= postTblNm | ||
| ] | ||
| for_ refTbls $ \refTbl -> do | ||
| TableExistsPredicate nm <- findPreConditions | ||
| guard (nm == refTbl) | ||
|
|
||
|
|
||
| let postConditions = [ p tblP, p primaryKeyP ] ++ concat columnsP ++ fkPs | ||
| cmd = createTableCmd (createTableSyntax Nothing (qnameAsTableName postTblNm) colsSyntax tblConstraints) | ||
| tblConstraints = if null primaryKey then [] else [ primaryKeyConstraintSyntax primaryKey ] | ||
| tblConstraints = (case NE.nonEmpty primaryKey of | ||
| Nothing -> [] | ||
| Just pkey -> [primaryKeyConstraintSyntax pkey]) | ||
| ++ fkConstraints | ||
| colsSyntax = map (\(colNm, type_, cs) -> (colNm, columnSchemaSyntax type_ Nothing cs Nothing)) columns | ||
| pure (PotentialAction mempty (HS.fromList postConditions) | ||
| (Seq.singleton (MigrationCommand cmd MigrationKeepsData)) | ||
|
|
@@ -590,17 +642,18 @@ dropIndexActionProvider = ActionProvider provider | |
| -- For default schema actions, see 'defaultSchemaActionProvider'. | ||
| defaultActionProvider :: ( Typeable be | ||
| , BeamMigrateOnlySqlBackend be ) | ||
| => ActionProvider be | ||
| defaultActionProvider = | ||
| => ForeignKeySupport be | ||
| -> ActionProvider be | ||
| defaultActionProvider fkSupport = | ||
| mconcat | ||
| [ createTableActionProvider | ||
| [ createTableActionProvider fkSupport | ||
| , dropTableActionProvider | ||
|
|
||
| , addColumnProvider | ||
| , dropColumnProvider | ||
|
|
||
| , addColumnNullProvider | ||
| , dropColumnNullProvider | ||
| , dropColumnNullProvider | ||
| ] | ||
|
|
||
| -- | Default action providers for any syntax which supports schemas. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm actually curious if there are any SQL DBMs that do not support foreign keys.
I know some backends (e.g. SQLite) may ignore these constraints, but I wouldn't say they're unsupported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sheaf I think we should drop
ForeignKeyUnsupportedThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I don't completely follow your suggestion. The purpose of the GADT was to allow users to call
createTableActionProviderwithout an instance forIsSql92ForeignKeyTableConstraintSyntax. If I remove that constructor, then I might as well add foreign key constraint syntax to theIsSql92TableConstraintSyntaxtypeclass, but that seems like a bigger breaking change. Would you be OK with that change?