paurkedal / ocaml-caqti

Cooperative-threaded access to relational data
https://paurkedal.github.io/ocaml-caqti/index.html
GNU Lesser General Public License v3.0
299 stars 36 forks source link

Postgresql json datatypes. #71

Closed mefyl closed 3 years ago

mefyl commented 3 years ago

I want to use PostgreSQL's json data types, which is not supported by default by Caqti. Issue https://github.com/paurkedal/ocaml-caqti/issues/16 suggest sending strings, however this doesn't work for me (probably because of more recent PostgreSQL versions):

ERROR: column "json" is of type json but expression is of type string

PostgreSQL wants us to explicitely cast the string, ie. ... VALUES ('{ "hello": "world"}'::json). I've been trying to define a custom Caqti_type.field to handle this, but it seems any custom field has to be based on an existing field which:

I would imagine the way to implement custom field would be to provide the actual, raw representation for the value for the driver.

Did I miss something here ? Otherwise, WDYT about the approach I suggest ? Happy to submit a PR.

paurkedal commented 3 years ago

Ouch, yet another breakage due to 5ef1d42b6b4f1c6cb1302bdf57807d5c354918de. Given the amount of trouble this commit has caused, I think the solution will be to make it an opt-in feature. If you want to pin, the (mis-)feature was introduced in 1.4.0. I hope to have a release ready by the beginning of next week.

mefyl commented 3 years ago

Looking at the commit, it looks like it only affects the PostgreSQL driver, so I don't think it can address my questions regarding _ field not being freely extensible and being a duplicate of Caqti_types.custom ? Is the fix that it would make it work with strings ? If so that would indeed fix it for me, although I think the second part of my comment still stands.

paurkedal commented 3 years ago

It's not a duplicate of custom, but otherwise you're right in that extending the field type is only relevant for drivers, as only they can implement the logic for handling the new cases. Having the type open hasn't been of any use since the three drivers I know of are all internal the the project. On the other hand, custom relies on having a way to rewrite the object into something already accepted.

paurkedal commented 3 years ago

I decided to drop the type enforcement of the [string] type, thus reverting it to pre-1.4.0 behaviour. I don't think it's that much needed, since the type inference on the database side usually does the job, otherwise I'll add a [text] type. I'll do a release this week.