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

Pass existing [EntityDef] into mkPersist #1241

Closed
parsonsmatt opened this issue Apr 21, 2021 · 4 comments · Fixed by #1256
Closed

Pass existing [EntityDef] into mkPersist #1241

parsonsmatt opened this issue Apr 21, 2021 · 4 comments · Fixed by #1256
Milestone

Comments

@parsonsmatt
Copy link
Collaborator

parsonsmatt commented Apr 21, 2021

mkPersist :: MkPersistSettings -> [EntityDef] -> Q [Dec]

A problem with this is that the generated declarations don't know about any of the EntityDefs that are not provided as part of this invocation. So if you have two blocks, one of which defines User and another defining an Organization with a user UserId field, the foreign key won't get properly invoked, since the liftAndFIxKeys machinery won't find the User in the existing map of tables.

This would fix #1073 as well. That issue has some good exploration on why this particular problem impacts foreign key generation. persistent figures that a BlahBlahId type is a reference to a BlahBlah model, and it tries to find the BlahBlah model in a Map HaskellName EntityDef that it constructs... from the [EntityDef] that is passed in.

A work-around is to... well, pass them in!

module Foo where

    share [mkPersist sqlSettings, mkEntityDefList "fooDefs"] [persistLowerCase|

        Foo
            name Text
    |]

module Bar where

    import Foo

    share
        [ mkPersist sqlSettings . mappend fooDefs
        , mkEntityDefList "barAndFooDefs"
        ] [persistLowerCase|
         
            Bar
                foo FooId
                name Text
    |]

This requires that you do that . mappend dependentDefs for each one. But it should fix the issue.

It's a bit annoying to tease these relationships together manually. And it's very easy to forget, especially since, well, everyone pretty much has. Can we do better?

@parsonsmatt
Copy link
Collaborator Author

I think we can!

reifyInstances, called like reifyInstances [''PersistEntity] [VarT (mkName "a")], should find all instances of PersistEntity that are in scope. We get back an [InstanceDef]. This is [guaranteed to be an InstanceD with an empty [Dec](https://www.stackage.org/haddock/lts-17.9/template-haskell-2.16.0.0/Language-Haskell-TH.html#t:InstanceDec).

InstanceD carries the Type for the instance - in this case, it'd look something like AppT (ConT ''PersistEntity) theType.

Given theType :: Type, we can write:

[| entityDef (Proxy :: Proxy $(pure theType)) |]

which produces an EntityDef in the spliced code. This EntityDef is going to be the one defined on the PersistEntity class, and so is the canonical one, not the one produced by the QuasiQuoter (which may be left unspecified).

Now, we don't yet have those EntityDefs in scope. It's still a Q Exp representing the list of all definitions. But we have a [EntityDef] that we're working with. How can we bridge this gap?

@parsonsmatt
Copy link
Collaborator Author

parsonsmatt commented Apr 21, 2021

Oh, duh, same way we get around persistLowerCase :: QuasiQuoter returning a String -> Q Exp - we splice it in.

mkPersist sqlSettings $ mappend $(entityDefsInScope) [persistLowerCase|

Bar
    foo  FooId
    name Text
|]

It's still annoying to need to mappend them manually.

It's also obnoxious to have to do entityDefsInScope for every module - that's going to be a linear instance lookup hit on each module that compiles, recollecting the same instances over and over again. Will definitely slow things down. Is there a way to keep track of some kind of global state? A compile-time IORef [Q Exp] that carries all the entityDefs would solve this nicely...

@parsonsmatt
Copy link
Collaborator Author

Hmm, reifyInstances doesn't have to grab all the instances. We can also call reifyInstances when we're trying to look up a foreign key, specifically for a table that matches.

Eg:

Foo
    name Text

Bar
    foo  FooId
    name Text

When working with Bar, first we check the EntityMap. If it's not there, we call reifyInstances ''PersistEntity (ConT (mkName "Foo")). If this returns anything, then we can splice in the resulting stuff. I'm not sure if this will operate at the phase level we care about, though.

That'll end up doing a reifyInstances search for every key use, which will (I think) end up being slower than just grabbing all the instances up front and stuffing them into the map.

@parsonsmatt
Copy link
Collaborator Author

wait, maybe we could do the splicing in persistLowerCase

the quoter exp is basically pure . parseReferences ps, so we coudl make it

quoteExp = \str -> do
  es <- discoverEntities
  new <- parseReferences ps str
  pure (new <> es) 

but then we need some check or lookup in mkPersist to differentiate - shouldn't be hard to verify if A is an instance of PersistEntity already or not, but that's just another lookup.

(really the quoter should not return [EntityDef] it should return something else...)

data EntityQuoteResult = EntityQuoteResult
    { eqrDiscoveredEntities :: Map EntityNameHS EntityDef
    , eqrParsedEntities :: Map EntityNameHS ParsedEntityDef
    } 

then mkPersist would have no trouble differentiating the two.

@parsonsmatt parsonsmatt added this to the 2.13 milestone Apr 23, 2021
@parsonsmatt parsonsmatt linked a pull request May 4, 2021 that will close this issue
8 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