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

Is anyone actually using mpsGeneric ? #1204

Open
parsonsmatt opened this issue Mar 15, 2021 · 7 comments · May be fixed by #1348
Open

Is anyone actually using mpsGeneric ? #1204

parsonsmatt opened this issue Mar 15, 2021 · 7 comments · May be fixed by #1348

Comments

@parsonsmatt
Copy link
Collaborator

The mpsGeneric flag complicates persistent-template considerably and has a dubious value proposition - it allows you to reuse a model in a SQL context and a Redis context and a MongoDB context.

This doesn't seem very useful. Removing it would be great. But I don't want to break people who need this for some reason.

Unfortunately, I can't just, like, collect data on this. What I can do, however, is deprecate the field, and link to this issue. Folks that upgrade will see the deprecation notice and be able to common on whether they use it or not.

@parsonsmatt
Copy link
Collaborator Author

parsonsmatt commented Apr 20, 2021

I'm going to throw a deprecation notice on mpsGeneric for 2.13 with a link to this issue and a request for comment.

@parsonsmatt parsonsmatt added this to the 2.13 milestone Apr 20, 2021
@parsonsmatt parsonsmatt removed this from the 2.13 milestone Apr 28, 2021
@parsonsmatt
Copy link
Collaborator Author

As documented in #1266, I want to wait until persistent-2.13 has made it into a Stackage LTS before proceeding with the removal.

@parsonsmatt
Copy link
Collaborator Author

mpsGeneric is used in the test suite to allow MongoDB and the SQL databases to share tests.

The persistent-typed-db library uses the PersisteEntityBackend entity ~ backend trick to ensure you're only calling a query with a compatible backend. I'd like to preserve those safety guarantees.

@parsonsmatt
Copy link
Collaborator Author

Stackage LTS 18.0 has persistent-2.13.0.2, and was released on June 16th, 2021. Six months of lead time means that I can delete mpsGeneric in December.

Going to set a calendar reminder to do that...

@parsonsmatt
Copy link
Collaborator Author

Thinking about this more -

When mpsGeneric is flipped on, we generate something like this:

mkPersist sqlSettings { mpsGeneric = True } [persistLowerCase|
User
    name String
    age Int
|]

-- ===>

type User = UserGeneric SqlBackend

data UserGeneric backend = User
    { userName :: String
    , userAge :: Int
    }

newtype instance Key (UserGeneric backend)  = UserKey 
    { unUserKey :: BackendKey backend 
    }
    
type instance PersistEntityBackend (UserGeneric backend) = backend

Which lets us write:

user :: UserGeneric backend
user = User { userName = "matt", userAge = 33 }

mongo :: ReaderT MongoContext IO (BackendKey MongoContext)
mongo = unUserKey <$> insert user

sql :: ReaderT SqlBackend IO (BackendKey SqlBackend)
sql = unUserKey <$> insert user

The idea is that the output of mkPersist settings [persistLowerCase| ... |] can be shared between Database.Persist.MongoDB and Database.Persist.Postgresql.

But we can already share this - we have a function share that literally works like this.

share [mkPersist sqlSettings, mkPersist mongoSettings] [persistLowerCase| ... |]

But we have a problem - the types aren't the same, so we can't share or easily convert from Mongo representation to SQL representation. Also the generated Haskell datatypes would share the same name, so it would break. Adding a prefix/suffix to the MkPersistSettings would alleviate that, though 'easy conversions' would rely on DuplicateRecordFields to write, like conv User {..} = MongoUser {..}.

Another option is to splice the QQ separately from the mkPersist call.

module ModelDefinition where
    models :: [UnboundEntityDef]
    models = [persistLowerCase| ... |]

module Models.Sql where
    import ModelDefinition
    mkPersist sqlSettings models

module Models.Mongo where
    import ModelDefinition
    mkPersist mongoSettings models

For modules that perform interop, you'd need to write things qualified and do conversions manually.

This works but it means that you can't write stuff that's generic on the backend and share the code. For example, right now I can write:

insertUser :: UserGeneric backend -> ReaderT backend IO (Key (UserGeneric backend))
insertUser = insert User { userName = "Matt", userAge = 32 }

selectUsersWith :: ReaderT backend IO [Entity (UserGeneric backend)]
selectUsersWith =
    selectList [UserAge >=. 30] []

or other functions that talk about Users specifically, and then run that code on MongoContext or SqlBackend. With actually separate datatypes, I can't do that anymore. It comes down to the PersistRecordBackend type family requirement on these classes:

-- simplified slightly
class PersistStore backend where
    insert 
        :: (PersistEntity record, PersistEntityBackend record ~ backend)
        => record -> ReaderT backend IO (Key record)

When we have type PersistEntity User = SqlBackend and type PersistEntity Mongo.User = MongoContext, then we're stuck - this constraint means we can't write operations that are generic over the backend if they rely on specific tables.

🤔

What if we remove the constraint?

class PersistStore backend where
    insert 
        :: (PersistEntity record)
        => record -> ReaderT backend IO (Key record)

Well- we're essentially saying, "Give me a record and an EntityDef corresponding to that record, and I can stuff it in the database." And, in fact, a lot of stuff works for this. Everything is permitted. Which is a problem - it means that everyone is now opted in to the Generic behavior. Even if you've set mpsGeneric = False to ensure that you don't insert a Sql.User into your MongoDB, you totally can, and the EntityDef tells you how.

This also completely breaks persistent-typed-db, which relies on this.

🤔

It does seem like we'd want different EntityDef for a MongoContext and a SqlBackend, too.

🤔

It feels like my choices are:

  1. Drop the constraint, which ruins persistent-typed-db for allowing separate database schema to be used safely,
  2. Keep the constraint, which ruins persistent-test for non-SqlBackend use.
  3. Keep mpsGeneric, which is frighteningly awful
  4. Somehow make records polymorphic such that they can be used differently in different backends.

I think 4 is the right approach. But mpsGeneric is the end result of "Let's add a type parameter to User" and it's not fun.

So, we have a class for relating a type to it's backend:

class PersistEntity record where
    type PersistBackend record :: Type

instance PersistEntity Sql.User where
    type PersistBackend Sql.User = SqlBackend

instance PersistEntity Mongo.User where
    type PersistBackend Mongo.User = MongoContext

If we want to make it generic, then we need to somehow introduce a target backend type in the thing.

insert 
    :: (PersistEntity ent, PersistStoreWrite backend, PersistBackend ent ~ backend)
    => ent -> ReaderT backend IO (Key ent)

A type class that injects a bit of polymorphism might work.

class (PersistEntity a, PersistEntity b) => CrossDatabase a b where
    crossDatabase :: a -> b

type PersistGeneric a b backend = (CrossDatabase a b, PersistBackend b ~ backend)

So if I want to make a User generic, that gets me:

f 
    :: (PersistGeneric Sql.User rec backend) 
    => Sql.User -> ReaderT backend IO (Key rec)
f user = insert $ crossDatabase user

What uh does kind of suck here is that we can't have a functional dependency - since we'd want to generate a self instance and several potential other instances.

instance CrossDatabase Sql.User Sql.User where crossDatabase = id
isntance CrossDatabase Sql.User Mongo.User where crossDatabase = ...
instance CrossDatabase Sql.User Redis.User where ...

instance CrossDatabase Mongo.User Mongo.User where ...
instance CrossDatabase Mongo.User Sql.User where ...
instance CrossDatabase Mongo.User Redis.User where ...

instance CrossDatabase Redis.User Redis.User where ...
...

This ends up requiring O(number_of_backends^2) type class instances, and type inference is sure to be awful.

To make UX better, you could presumably just operate on a single database as the "blessed" representation, and then only write O(number_of_backends) instances of this conversion class.

I think - as a first step - the right thing to do here is to:

  1. Develop an alternative to mpsGeneric inside of persistent-test, and stop using mpsGeneric in favor of this new technique
  2. If the technique is good and fun, and people start asking for it, factor it into persistent, or another library.
  3. Keep the PersistBackend type family and constraint around.

@parsonsmatt
Copy link
Collaborator Author

A problem with the above approach is that Key backend becomes impossible to introspect on - you can't do anything with it aside from using it as an abstract database operation.

Right now, with Key (UserGeneric backend), I can still call unUserKey :: Key (UserGeneric backend) -> BackendKey backend and UserKey :: BackendKey backend -> Key (UserGeneric backend). With Key rec, I can't do anything at all beyond use it directly.

In a non-default ID case, like:

User
    Id     UUID
    name Text
    age Int

then the generic does let me introspect on it:

UserKey :: UUID -> Key (UserGeneric backend)
unUserKey :: Key (UserGeneric backend) -> UUID

But the type variables don't give me the ability to write these. I'd need to include that stuff in the class.

@parsonsmatt
Copy link
Collaborator Author

parsonsmatt commented Oct 1, 2021

Something that's striking me as a somewhat obvious fix here is to have, like:

module Persistent.Test.Models where

    models :: String
    models = [str|
    
    |]

module Persistent.Test.Sql where

    import Persistent.Test.Models

    mkPersist sqlSettings $(quoteExp persistLowerCase models)

module Persistent.Test.MongoDB where

    import Persistent.Test.Models

    mkPersist mongoSettings $(quoteExp persistLowerCase models)

🤔

That at least allows you to share the model definitions among the different backends, even if it doesn't let you share the tests themselves.

EDIT:

Oh wait, we don't even need that, we can just have models :: [UnboundEntityDef] and have mkPersist operate separately.

@parsonsmatt parsonsmatt linked a pull request Jan 6, 2022 that will close this issue
7 tasks
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 a pull request may close this issue.

1 participant