experimental data family for keys #186

Closed
wants to merge 9 commits into
from

2 participants

@gregwebs
Yesod Web Framework member

@snoyberg take a look at this if you get a chance. The main change is in PersistEntity.hs

data KeyBackend backend record
persistValueToPersistKey :: PersistValue -> KeyBackend backend record
persistKeyToPersistValue :: KeyBackend backend record -> PersistValue

Now I need to change TH.hs to generate something like:

instance PersistEntity User where
      data KeyBackend (EntityBackend User) User = UserKey Int64

I think there should be a way to make this maintain the backend flexibility we want.

@gregwebs
Yesod Web Framework member

All tests are compiling now. I just need to generate KeyBackend, persistValueToPersistKey, persistKeyToPersistValue. What do you think?

@snoyberg
Yesod Web Framework member

I haven't had a chance to analyze this yet, but at a high level, this makes sense. My biggest concern is the breaking nature. We may want to include a few other bigger changes at the same time and call this persistent 2.0. I'll look at this commit in more detail in the next day or two and give you some more specific feedback.

@gregwebs
Yesod Web Framework member

Thanks for taking a peek, I will see if I can get the TH working today. Although I am not against including more breaking changes, I am not sure if putting them in one release makes things easier for users, particularly if they are planning on upgrading. I think putting this at 2.0 and having more breaking changes for 2.1 and 2.2 has its own advantages of intermediate working states.

@gregwebs gregwebs first stab at TH for associated Key.
seeing this error:
Illegal repeated type variable `backend'
b681a38
@gregwebs
Yesod Web Framework member

I started on the TH. I am seeing this error, not sure why

Illegal repeated type variable `backend'
gregwebs added some commits Dec 1, 2013
@gregwebs gregwebs refactor TH a285d26
@gregwebs gregwebs avoid illegal repeated type variable issue
however, this makes our backend constraint different
a779613
@gregwebs gregwebs Docekerfile: persistent user c8eda0d
@gregwebs gregwebs fill out Key data family TH
works with mpsGeneric = False
unfortunately, doesn't compile with mpsGeneric = True
178c050
@gregwebs
Yesod Web Framework member

I was able to get rid of the repeated type variable issue by removing the backend parameter to the associated Key data family. Everything works now for mpsGeneric = False. However, I am not sure how to make this work when everything is kept generic (uncomment line 638 in TH.hs)

@snoyberg
Yesod Web Framework member

I've played around with this a bit, and I think your change is basically the right one. I think it can be simplified just a bit. We can get rid of the KeyBackend concept entirely, and then have the associated datatype be:

data family Key record

In the case of generic datatypes, the record contains the backend as a parameter, and therefore we don't need the backend to be a parameter to Key. The instances for this look like:

data instance Key (PersonGeneric backend) = PersonKey { unPersonKey :: BackendKey backend }

or in the case of a non-generic data type:

data instance Key Person = PersonKey { unPersonKey :: BackendKey SqlBackend }

Unfortunately, this change is rather tied in to some of the other changes I'm working on simultaneously, so it's difficult to see. All of this is on the readert-pattern experimental branch if you want to have a look.

I'm not sure if this branch will ever see the light of day, or if it will just be an aborted experiment, but it's definitely helping me to clarify some of my ideas for simplifying the interface. I'll try to write up some of the ideas behind it either this week or next, which might make it easier to understand the branch.

@gregwebs
Yesod Web Framework member

ok, I am not sure how this interacts with your multi-param typeclasses in the branch

@snoyberg
Yesod Web Framework member

I think the comments I just made should be orthogonal. Besides some trickery with automatic deriving that I'm running into now, the approach seems like it should work on your branch as well.

@gregwebs gregwebs added the 2.0 label Jun 11, 2014
@gregwebs
Yesod Web Framework member

data family for Key is on persistent2

@gregwebs gregwebs closed this Aug 25, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment