Skip to content
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

Select first record or create a new one if there is no match #954

Closed
wants to merge 1 commit into from

Conversation

sestrella
Copy link

@sestrella sestrella commented Sep 12, 2019

Before submitting your PR, check that you've:

After submitting your PR:

  • Update the Changelog.md file with a link to your PR
  • Check that CI passes (or if it fails, for reasons unrelated to your change, like CI timeouts)

@Vlix
Copy link
Contributor

Vlix commented Sep 19, 2019

This will throw an error when the to be inserted record can't be inserted because of a uniqueness constraint, for example. Is that the intended functionality?

@sestrella
Copy link
Author

sestrella commented Sep 25, 2019

@Vlix sorry for the late reply, perhaps bringing up the use case I had in mind might help here. So far I was thinking about using this function in scenarios like seed data, where the model looks like follows:

share [mkPersist sqlSettings] [persistLowerCase|
Role
  name        Text -- assuming there is a unique constraint on this column
  description Text
|]

findOrCreateRole Role{..}@role =  selectFirstOrInsert [RoleName ==. roleName] role

However, after a second thought, I completely agree with you this operation could fail, in which case perhaps it would make more sense to replace [Filter record] with Unique record in order to make more explicit there is a unique constraint:

getUniqueOrInsert
  :: (OnlyOneUniqueKey record, ...)
  => record
  -> ReaderT backend m (Entity record)
getUniqueOrInsert rec = getUniqueByOrInsert (onlyUniqueP rec) rec

getUniqueByOrInsert
  :: (..)
  => Unique record 
  -> record
  -> ReaderT backend m (Entity record)
getUniqueByOrInsert unique record = do
  mrecord <- getBy unique
  case mrecord of
    Nothing   -> insertEntity record
    (Just rec) -> return rec

What do you think?

@Vlix
Copy link
Contributor

Vlix commented Sep 25, 2019

Honestly, it sounds like you just want to use insertBy, which takes a record and returns Either (Entity record) (Key record).
Using the record you inserted after the Right case is returned would be identical to what you're suggesting, and I don't really see why that would need to be a new function in the library...

myFunc rec = either id (const rec) <$> insertBy rec

@parsonsmatt
Copy link
Collaborator

A problem here is that the unique records may not be the same in the non-unique fields. And a problem with the suggested PR is that there's no guarantee that the entity you insert satisfies the conditions you provided.

I'm generally inclined to include helpers and convenience functions into the library if the obvious implementation isn't right or fast for some reason. maybe (insertEntity entity) pure =<< getBy unique is very short, and I can't see why it wouldn't be right.

@sestrella
Copy link
Author

@Vlix, @parsonsmatt fair enough, I appreciate all the feedback, for now, I would go with the short hard function @Vlix suggested.

@sestrella sestrella closed this Sep 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants