-
Notifications
You must be signed in to change notification settings - Fork 292
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
Type error on upsert. #885
Conversation
Weird, I am getting test suite failures locally for MongoDB:
The IDs are different. I did not get these earlier when I was refactoring the test suite, and they don't appear to show up in CI? |
$ "Generating Persistent entities now requires the 'UndecidableInstances' " | ||
<> "language extension. Please enable it in your file by copy/pasting " | ||
<> "this line into the top of your file: \n\n" | ||
<> "{-# LANGUAGE UndecidableInstances #-}" |
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 gives a pretty pleasant error message. The alternative is a bit nasty.
|
||
singleUniqueKey :: Q [Dec] | ||
singleUniqueKey = do | ||
expr <- [e|\p -> head (persistUniqueKeysP p)|] |
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 safe because we have already guaranteed that there are multiple unique keys
@@ -386,50 +386,6 @@ specsWith runDb = describe "persistent" $ do | |||
pBlue30 <- updateGet key25 [PersonAge +=. 2] | |||
pBlue30 @== Person "Updated" 30 Nothing | |||
|
|||
describe "putMany" $ do |
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.
These specs are failing for MongoDB locally for me. The keys are changing, so it isn't updating, but replacing. Strange! I moved them into upsert specs
which is pending for now.
|
||
singleUniqueKey :: Q [Dec] | ||
singleUniqueKey = do | ||
expr <- [e|\p -> head (persistUniqueKeys p)|] |
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 "safe" because the generating code has already guaranteed that we have exactly one unique key.
|
||
atLeastOneKey :: Q [Dec] | ||
atLeastOneKey = do | ||
expr <- [e|\p -> NEL.fromList (persistUniqueKeys p)|] |
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.
likewise, this is safe because we have a guarantee from the generating code that it has at least one key.
@@ -1,5 +1,6 @@ | |||
{-# OPTIONS_GHC -fno-warn-unused-binds -fno-warn-orphans -O0 #-} | |||
{-# LANGUAGE DeriveDataTypeable #-} | |||
{-# LANGUAGE UndecidableInstances #-} |
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 change requires UndecidableInstances
. I give a nice error message for upgrading.
@@ -165,3 +165,48 @@ specsWith runDb handleNull handleKey = describe "UpsertTests" $ do | |||
Just 2 | |||
Don'tUpdateNull -> | |||
Nothing | |||
|
|||
describe "putMany" $ do |
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.
Moved from the PersistentTest
module so that mongoDB
could skip them for now.
-- | Update based on a uniqueness constraint or insert: | ||
-- | ||
-- * insert the new record if it does not exist; | ||
-- * If the record exists (matched via it's uniqueness constraint), then update the existing record with the parameters which is passed on as list to the function. | ||
-- | ||
-- Throws an exception if there is more than 1 uniqueness constraint. | ||
-- |
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.
👋
Do we need to bump the major versions of the component libraries? eg |
Ah, dang. We can't do |
This PR introduces a few type classes that are required on
upsert
and related functions that have expectations on how many unique keys a record has.Instances for these classes are generated automatically, and "failing" instances get informative "TypeError"s when you misuse them.
Fixes #868
Fixes #796
Before submitting your PR, check that you've:
@since
declarations to the HaddockAfter submitting your PR: