-
Notifications
You must be signed in to change notification settings - Fork 293
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
Primary
key implies a Unique
constraint
#1383
Conversation
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.
overall very happy with how easy this was to knock out
it "checkUniqueUpdateable" $ runDb $ do | ||
let f = 3 | ||
let b = 5 | ||
let fo = Fo f b | ||
k <- insert fo | ||
Just _ <- checkUnique fo -- conflicts with itself | ||
|
||
let fo' = Fo (f + 1) b | ||
Just _ <- checkUnique fo' -- conflicts with fo | ||
Nothing <- checkUniqueUpdateable $ Entity k fo' -- but fo can be updated to fo' | ||
|
||
let fo'' = Fo (f + 1) (b + 1) | ||
insert_ fo'' | ||
Just (UniqueBar conflict) <- checkUniqueUpdateable $ Entity k fo'' -- fo can't be updated to fo'' | ||
conflict @== b + 1 |
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 think the old insertUnique
code was actually broken on this, since it would be totally fine doing an insert
based on the UniqueBar
being actually unique, but then fail with the Primary
key constraint violation.
@@ -1067,7 +1074,7 @@ takeId ps entityName texts = | |||
-- | |||
-- @since.2.13.0.0 | |||
data UnboundCompositeDef = UnboundCompositeDef | |||
{ unboundCompositeCols :: [FieldNameHS] | |||
{ unboundCompositeCols :: NonEmpty FieldNameHS |
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.
Might as well - we can't have a composite key without at least one field.
, entityConstraintDefsUniques = | ||
Just $ pure $ compositeToUniqueDef entityName defs unboundComposite |
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.
This is the real change. We're adding a UniqueDef
based on the composite primary key.
@@ -81,7 +81,7 @@ mkColumns | |||
-> BackendSpecificOverrides | |||
-> ([Column], [UniqueDef], [ForeignDef]) | |||
mkColumns allDefs t overrides = | |||
(cols, getEntityUniques t, getEntityForeignDefs t) | |||
(cols, getEntityUniquesNoPrimaryKey t, getEntityForeignDefs t) |
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.
In order to preserve migration compatibility, we don't return the composite primary key in the UniqueDef
for migrations.
where | ||
parentKeyFieldNames | ||
:: [(FieldNameHS, FieldNameDB)] | ||
:: NonEmpty (FieldNameHS, FieldNameDB) |
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.
just pushing this as far as we can.. even if it's toList
ed about two lines above.
entityUniques (unboundEntityDef user) `shouldBe` | ||
[ UniqueDef | ||
{ uniqueHaskell = | ||
ConstraintNameHS "UserPrimaryKey" | ||
, uniqueDBName = | ||
ConstraintNameDB "primary_key" | ||
, uniqueFields = | ||
pure (FieldNameHS "ref", FieldNameDB "ref") | ||
, uniqueAttrs = | ||
[] | ||
} | ||
] |
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.
super simple test for the Quasi parser
Fixes #1323 |
Fixes #985 |
Before submitting your PR, check that you've:
@since
declarations to the Haddockstylish-haskell
on any changed files..editorconfig
file for details)After submitting your PR:
(unreleased)
on the Changelog