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

getBy and select should ideally also return the AutoKey #16

Open
ozataman opened this issue Nov 29, 2013 · 11 comments
Open

getBy and select should ideally also return the AutoKey #16

ozataman opened this issue Nov 29, 2013 · 11 comments

Comments

@ozataman
Copy link

With getBy defined as below, it is often inconvenient to lookup by a unique key followed by and update or a replace. If getBy also returned the AutoKey for the record in question, that can then easily be used for subsequent operations.

getBy :: (PersistEntity v, IsUniqueKey (Key v (Unique u))) => Key v (Unique u) -> m (Maybe v)

Same applies to select - it is most often better to use the more verbose project to get both the AutoKey and the constructor.

Given AutoKey is very central to groundhog's API (many ops require it), it may be better to return it by default in these kinds of operations.

Conversely, it would be useful to define a new replaceBy function that uses a unique key to perform replacement.

@lykahb
Copy link
Owner

lykahb commented Dec 2, 2013

Perhaps it is better not to change default types of getBy and select since it will break all existing code, but to define new functions which return AutoKey too. Do you have any naming suggestions?

I am fine with adding replaceBy.

@ozataman
Copy link
Author

ozataman commented Dec 3, 2013

Hmm. Maybe we can borrow the "Entity" concept from persistent for these cases - the combination of the AutoKey and the object.

So they can be something like getEntityBy and selectEntity.

Also, I think it would be great to add (I've already needed it) something like the following that replace the conditions with a unique constraint:

updateBy :: [Update (PhantomDb m) (RestrictionHolder v c)] -> Key v (Unique u) -> ...
deleteBy :: Key v (Unique u) -> ....

The basic idea being to add a "by-unique-constraint" version for every operation that otherwise indexes either by AutoKey or by conditions.

Thoughts?

@lykahb
Copy link
Owner

lykahb commented Dec 3, 2013

I like the idea of using Entity as part of the name.
What do you use instead of updateBy and deleteBy? Comparison with unique pseudofield is more handy than comparing each field individually

update myUpdates $ ArtistName ==. k where
  k :: Key Artist (Unique ArtistName)

@ozataman
Copy link
Author

ozataman commented Dec 3, 2013

Right, but that only works when the unique constraint is a single field.
Right?

The major benefit from what I was describing above is being able to use a
multi-field unique directly as the update conditions, without having to
re-specify each field name individually.

I was thinking something like:

update myUpdates (extractUnique myRecord :: Key Artist (Unique SomeFields))

On Tuesday, December 3, 2013, Boris Lykah wrote:

I like the idea of using Entity as part of the name.
What do you use instead of updateBy and deleteBy? Comparison with unique
pseudofield is more handy than comparing each field individually

update myUpdates $ ArtistName ==. k where
k :: Key Artist (Unique ArtistName)


Reply to this email directly or view it on GitHubhttps://github.com//issues/16#issuecomment-29718030
.

Ozgun Ataman

CEO

Soostone Inc.

+1 917 725 0849 (Office)
+1 734 262 0676 (Mobile)
+1 212 320 0251 (Fax)
[email protected]

@lykahb
Copy link
Owner

lykahb commented Dec 3, 2013

This works for the composite keys too like an embedded field.

@ozataman
Copy link
Author

ozataman commented Dec 3, 2013

Could you give an example?

Sent from my iPhone

On Dec 3, 2013, at 11:31 AM, Boris Lykah [email protected] wrote:

This works for the composite keys too like an embedded field.


Reply to this email directly or view it on GitHub.

@lykahb
Copy link
Owner

lykahb commented Dec 3, 2013

Here the key has two fields

{-# LANGUAGE GADTs, TypeFamilies, TemplateHaskell, QuasiQuotes, FlexibleInstances, StandaloneDeriving #-}
import Control.Monad
import Control.Monad.IO.Class (liftIO)
import Database.Groundhog.Core (UniqueMarker)
import Database.Groundhog.TH
import Database.Groundhog.Sqlite

data CompositeExample = CompositeExample { k1 :: Int, k2 :: Int, myData :: String } deriving (Eq, Show)
data ForeignKeyExample = ForeignKeyExample { foreignKey :: Key CompositeExample (Unique CompositeExampleUnique)}
deriving instance Eq ForeignKeyExample
deriving instance Show ForeignKeyExample

data CompositeExampleUnique v where
  CompositeExampleUnique :: CompositeExampleUnique (UniqueMarker CompositeExample)

mkPersist defaultCodegenConfig [groundhog|
definitions:
  - entity: CompositeExample
    autoKey: null
    keys:
      - name: CompositeExampleUnique
        default: true
    constructors:
      - name: CompositeExample
        uniques:
          - name: CompositeExampleUnique
            fields: [k1, k2]
  - entity: ForeignKeyExample
|]

main :: IO ()
main = withSqliteConn ":memory:" $ runDbConn $ do
  let compositeExample = CompositeExample 1 2 "abc"
  runMigration defaultMigrationLogger $ do
    migrate (undefined :: CompositeExample)
    migrate (undefined :: ForeignKeyExample)
  insert compositeExample
  let k = extractUnique compositeExample
  -- in some cases you may need to put type signature.
  update [MyDataField =. "abcabc"] $ CompositeExampleUnique ==. (k :: Key CompositeExample (Unique CompositeExampleUnique))
  getBy k >>= liftIO . print
  delete $ CompositeExampleUnique ==. k
  getBy k >>= liftIO . print
  return ()

@ozataman
Copy link
Author

ozataman commented Dec 4, 2013

Oh, that's great! What I didn't know was that you can do CompositeExampleUnique ==. k. Very nice. My updateBy and deleteBy suggestions above are completely unnecessary then!

Thanks!

@MichaelXavier
Copy link

Going to resurrect this thread. @lykahb it seems like this feature is still necessary. If you want to search records and be able to look them up by autokey later, I think you would have to look them back up through a projection. Have your thoughts changed on this feature?

@lykahb
Copy link
Owner

lykahb commented Aug 13, 2015

Not every entity has an autokey so I am hesitant about adding it to the getBy and select. Projection can serve as both and it is only a little more verbose than getBy. Or do you have a different use case in mind?

@MichaelXavier
Copy link

Hmm, that's fine. I actually discovered that our company had solved this in the groundhog-utils module. Not sure if this strategy should be mentioned in documentation or maybe even ported to groundhog proper in some way. Up to you.

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

No branches or pull requests

3 participants