On Sun, Apr 3, 2011 at 10:35 AM, Michael Snoyman wrote:

As a short term solution, I think you're right. However, I think we might want to consider a slightly more sophisticated approach... I say consider specifically, because I'm really not certain that what I'm saying is a good idea.

I think there's basically three problems with the approach of using a PersistValue inside a PersistKey:

1) It will probably hurt performance, since we'll need to do more checking. 2) The code becomes a bit more fragile. By adding this sum type to the mix, we're adding necessity for a number of checks that can fail. 3) There's nothing stopping you from inserting a value into one database (say, SQLite), getting a key, and then looking up in MongoDB. (Not that this is a flaw in the current approach as well.)

I agree on the first two points. I think the principle behind 3 is actually a feature.

So here's the idea: each database backend will have an associated type for its key datatype. Then, instead of having:

data Key entity = Int64

we'll have

data Key entity backend = BackendKey backend

(ignoring all the newtype wrappers). I think this should solve both the issue you raise about MongoDB, and the three points I mention above. However, I'm still concerned that it might lead to difficult-to-follow code. There's really only one way to find out, but I just wanted to bounce the idea around before diving in.

I am going to negate my previous email. After thinking on this a bit. I think that setting they key type at the PersistEntity is the correct approach. This would be the only way to handle keys as strings or composite keys (if we want to support either) If we want to restrict all Yesod databases to a single pre-defined key type per database, then I will implement your solution. If we want to be flexible (for legacy database support) then I think we should go with a parameterized type solution, and I have no idea how to make it efficient.

And then the other shoe drops. So let's say we've got: Author name String Entry author AuthorId And we make my little change: data Key entity backend :: * That means that the old generated code for Entry: type AuthorId = Key Author data Entry = Entry AuthorId no longer works. Instead, we now need: type AuthorId backend = Key Author backend data Entry backend = Entry (AuthorId backend) So the backend is going to need to propogate through the entire data type hierarchy. To some extent, this is appealing. On the other hand, it's probably ridiculous. I'll probably play around with this code a bit more, but I think this solution of mine is a dead end. Michael On Sun, Apr 3, 2011 at 6:52 PM, Michael Snoyman wrote:

On Sun, Apr 3, 2011 at 5:44 PM, Rick Richardson wrote:

...

On Sun, Apr 3, 2011 at 10:35 AM, Michael Snoyman wrote:

...

As a short term solution, I think you're right. However, I think we might want to consider a slightly more sophisticated approach... I say consider specifically, because I'm really not certain that what I'm saying is a good idea. I think there's basically three problems with the approach of using a PersistValue inside a PersistKey: 1) It will probably hurt performance, since we'll need to do more checking. 2) The code becomes a bit more fragile. By adding this sum type to the mix, we're adding necessity for a number of checks that can fail. 3) There's nothing stopping you from inserting a value into one database (say, SQLite), getting a key, and then looking up in MongoDB. (Not that this is a flaw in the current approach as well.)

I agree on the first two points. I think the principle behind 3 is actually a feature.

Can you elaborate here? I see it as a definite flaw in the type safety we otherwise have.

...

...

So here's the idea: each database backend will have an associated type for its key datatype. Then, instead of having:     data Key entity = Int64 we'll have     data Key entity backend = BackendKey backend (ignoring all the newtype wrappers). I think this should solve both the issue you raise about MongoDB, and the three points I mention above. However, I'm still concerned that it might lead to difficult-to-follow code. There's really only one way to find out, but I just wanted to bounce the idea around before diving in.

I am going to negate my previous email. After thinking on this a bit. I think that setting they key type at the PersistEntity is the correct approach. This would be the only way to handle keys as strings or composite keys (if we want to support either) If we want to restrict all Yesod databases to a single pre-defined key type per database, then I will implement your solution.  If we want to be flexible (for legacy database support) then I think we should go with a parameterized type solution, and I have no idea how to make it efficient.

I think the parameterized type solution *will* be efficient. What I was worried about was having a sum type where we need to perform conditionals every time we want to use it. I actually just tried out the parameterized approach, and so far the code required very minor modifications to compile. Of course, getting the tests to pass (which will involve fixing up the TH code) may be a bit more involved.

If you (or Greg) have other opinions on this, I'd be happy to hear them. I think you two are the only people who have worked on a non-SQL backend (besides my pittance attempt at Redis), so you'll likely have very good insight that I need to hear.

Michael

On Sun, Apr 3, 2011 at 12:17 PM, Greg Weber wrote:

On Sun, Apr 3, 2011 at 7:44 AM, Rick Richardson

...

wrote:

...

On Sun, Apr 3, 2011 at 10:35 AM, Michael Snoyman wrote:

...

As a short term solution, I think you're right. However, I think we might want to consider a slightly more sophisticated approach... I say consider specifically, because I'm really not certain that what I'm saying is a good idea.

I think there's basically three problems with the approach of using a PersistValue inside a PersistKey:

1) It will probably hurt performance, since we'll need to do more checking. 2) The code becomes a bit more fragile. By adding this sum type to the mix, we're adding necessity for a number of checks that can fail. 3) There's nothing stopping you from inserting a value into one database (say, SQLite), getting a key, and then looking up in MongoDB. (Not that this is a flaw in the current approach as well.)

I agree on the first two points. I think the principle behind 3 is actually a feature.

...

So here's the idea: each database backend will have an associated type for its key datatype. Then, instead of having:

data Key entity = Int64

we'll have

data Key entity backend = BackendKey backend

(ignoring all the newtype wrappers). I think this should solve both the issue you raise about MongoDB, and the three points I mention above. However, I'm still concerned that it might lead to difficult-to-follow code. There's really only one way to find out, but I just wanted to bounce the idea around before diving in.

Regarding the key type. I would like to be able to set it as part of the PersistEntity via TH, since I think keys should be allowed to be set on a per PersistEntity basis. Though this means that anything dealing with keys has to reference (Key val) which makes such things as fromPersistKey nonsensical. I also think that we could define some generic key-> datatype functions that are also set by TH, but they can't be part of the PersistEntity class, it would enable the backends to build such functions as selectKeys.

...

I am going to negate my previous email. After thinking on this a bit. I think that setting they key type at the PersistEntity is the correct approach. This would be the only way to handle keys as strings or composite keys (if we want to support either)

There are 2 use cases- relying on MongoDB to auto-generate a key, or providing one's own key. Now technically the key provided could be almost any PersistValue. But I think in practice it is likely to be an integer or an ObjectId. And I think even those use cases are likely to be the exception, not the rule. For now, to get this working, I think it is reasonable to assume that no id value will be provided by the user- that mongoDB will always be auto-generating it. So we should be able to just accommodate the key the same way as a normal db, except that it is 12 bit number. And then it should be easy to go from there to support the user supplying their own ObjectId (which is the 12 bit number that MongoDB auto-generates). And supporting inserting an integer with the overhead of assuming it is 12bits would be easy also.

I was just assuming that we would let mongo generate the values, since we didn't really want to force our customer to generate their own unique values for each insert. At the moment, I have gone the PersistValue route. I have altered the TH, PersistEntity and PersistBackend classes to support a PersistValue key. Getting it to build GenericSql required adding (PersistField (Key val)) => to most of the PersistBackend functions. I also had to muck with the selectKeys implementation. My current approach is to add a keyType= attribute for Entities in TH. If none is provided, it defaults to PersistInt64. I am not sure I like this solution, but I pretty much have everything working. I have to take off for a while. I was hoping to get this working today.