JSON character prefix type information

[gregwebs]

see #136 and #168

I would like to solve this issue for more than just this use case. It is true that we need to retain type information to the point of the conversion. However, there are multiple ways to carry that information, and there are big drawbacks to character prefixing.

I think a big improvement would be something like {"type":"Int","value":2}

An alternative is to send a separate schema object with data. That could be more efficient but could be more error prone.

[maxcan]

But then can you parse data without the schema? I'd guess not so we still lose interoperability with the rest of the world. What are your thoughts about my point that it the only constructor of PersistValue that matters is PersistObjectId?

[gregwebs]

So is the main mistake is that PersistObjectId is being leaked? But is that not also the case for PersistInt64 for Sql users and potentially something else for future users of a different database?

[gregwebs]

oh, PersistInt64 is a Number so it doesn't have this issue because String is the only overloaded JSON type.

[gregwebs]

So should unKey ideally be a function that unwraps the PersistValue so that the a Key does not expose a user to a PersistValue?

One note on implementation that may be related is that I would like to remove PersistObjectId and instead just use PersistDbSpecific if possible.

[maxcan]

Ok, so that's getting deeper into persistent than I can intelligently comment on. Another solution to this would be, as you suggest, to stop the PersistObjectIds from leaking out.

[gregwebs]

yeah, so that is what I am proposing. The existing type is:

newtype KeyBackend backend entity = Key { unKey ∷ PersistValue }
type Key val = KeyBackend (PersistEntityBackend val) val

@snoyberg does it make sense to have functions to be used in the place of unKey (or possibly change the type of unKey) such that the user will not get back a PersistValue from a Key? I have a helper in MongoDB already that gives back the native DB type instead.

keyToOid ∷ (PersistEntity entity, PersistEntityBackend entity ~ MongoBackend)
         ⇒ Key entity → DB.ObjectId
keyToOid (Key k) = persistObjectIdToDbOid k

[gregwebs]

I don't think I really have any usage for a PersistObjectId. I either want a Key or the underlying MongoDB data type (DB.ObjectId from above). Is there a desire for users to deal with PersistInt64 ?

[gregwebs]

I am wondering if now is the time to take a look at composite keys.

[maxcan]

you mean the embedded schemas or addressing the persist value leakage? --  max cantor Sent with Airmail

On October 10, 2013 at 10:48:41 AM, Greg Weber (notifications@github.com) wrote:

I am wondering if now is the time to take a look at composite keys.

— Reply to this email directly or view it on GitHub.

[gregwebs]

SQL users want composite keys, which may mean another API breakage to Key

[snoyberg]

Sorry, I haven't had a chance to look at this issue yet. I will try to get to it next week.

[gregwebs]

no problem, I need a little more time still to look at this in greater depth.

[gregwebs]

I guess what this is getting at is that we give back a Haskell record, not [PersistValue] for the record. What is stopping us from giving back a Haskell value for the key? Actually, we need to support composite fields, so we could give back a Haskell record, but the global record fields issue doesn't allow for that. But key fields are few and can remain unnamed so we can just give back a constructor of Haskell values. It does require a different constructor for each record, but I think KeyPerson 5 is better than Key $ PersistInt64 5. Note that key construction now becomes type-safe, whereas right now someone can stick whatever PersistValue they want into a key.

The only problem with this is the backend parameterization. I think we just need how to figure out how to make it work for the one database a user is actually using and we can later figure out how to make it possible to have keys for different backends when someone needs that.

[snoyberg]

If we're willing to give up on datatypes that work across multiple backends, then your approach makes perfect sense. But the whole purpose in using PersistValue in the first place was because of the multi-backend issue. I doubt anything's changed in such a way that it makes sense to change key handling.

[gregwebs]

Perhaps we can still have a backend agnostic type underneath. personKey = Key . PersistInt64. Rather than exporting the Key constructor we could have functions to retrieve the Haskell value. Not sure what to call that, perhaps fromPersonKey. We could have more general less type-safe functions such as keyInt.

[snoyberg]

Are you talking about having smart constructors and extractors? I'm not necessarily opposed, but I'm not sure what the motivation is here.

[gregwebs]

This issue was opened up because we are exposing a PersistValue (PersistObjectId) which gets converted into JSON that was only meant for the database. After thinking about this I realized that PersistValue is an internal persistent implementation detail that a user should never be exposed to in usage. It should only be exposed for the purpose of defining a PersistField.

I am also trying to kepp in mind the possibility of increasing type safety and handling composite keys.

[gregwebs]

With composite keys or a primary key of a record field, the key type is dependent on the record. This by itself would be easy to make more type-safe, but there is also the option of the auto-generated id key depending on the backend.

Does this express what we want?

data Family KeyBackend :: * -> *
instance Family KeyBackend MongoBackend User  = DB.Oid -- auto-generated primary key
instance Family KeyBackend SqlBackend   User  = Int    -- auto-generated primary key
instance Family KeyBackend SqlBackend   User1 = Text -- primary key on e-mail
instance Family KeyBackend SqlBackend   User3 = PersistValue -- composite key with multiple types, need to use PersistList or a generated constructor

[gregwebs]

work is under way for a better Key type on #186

[gregwebs]

The change has been made on the persistent2 branch so that the Key is not just a PersistValue wrapper.

[gregwebs]

Note that we did not fix the JSON character prefix, but instead fixed where it leaked out in JSON generation for MongoDB. It should now only be visible as serialized JSON in the database (for SQL users). Changing how type prefixes operate is a backwards-incompatible change that would be good, as a MongoDB user I will leave it up to the SQL users to do that.