groundhog icon indicating copy to clipboard operation
groundhog copied to clipboard

`getBy` and `select` should ideally also return the AutoKey

Open ozataman opened this issue 11 years ago • 11 comments

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.

ozataman avatar Nov 29 '13 22:11 ozataman

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.

lykahb avatar Dec 02 '13 20:12 lykahb

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?

ozataman avatar Dec 03 '13 04:12 ozataman

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)

lykahb avatar Dec 03 '13 15:12 lykahb

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/lykahb/groundhog/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]

ozataman avatar Dec 03 '13 16:12 ozataman

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

lykahb avatar Dec 03 '13 16:12 lykahb

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.

ozataman avatar Dec 03 '13 16:12 ozataman

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 ()

lykahb avatar Dec 03 '13 18:12 lykahb

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!

ozataman avatar Dec 04 '13 04:12 ozataman

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?

MichaelXavier avatar Aug 12 '15 22:08 MichaelXavier

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?

lykahb avatar Aug 13 '15 02:08 lykahb

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.

MichaelXavier avatar Aug 21 '15 02:08 MichaelXavier