-
Notifications
You must be signed in to change notification settings - Fork 55
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
Support jakarta.json.JsonValue as a persistence capable type #343
Comments
note it should be about |
Good catch! Fixed! |
@gavinking do you think this is something we can find agreement on among vendors within remaining timeframe for 3.1 (say 3-4w), should we try and see or should we rather postpone it to the next release right away? |
Mapping to VARCHAR should probably be fallback if the DB vendor does not support Json through its own datatype |
Ummmmm .... not sure because I've never used JSON-P or
So I dunno, I mean, it's doable, but it doesn't seem critical. If we're going to start adding new built-in types I'm not clear that this would be on the top of the list. Of course I might be missing some extra good reason to do it beyond "save users from having to write a converter". The situation with But again, I'm not sure. |
the use-case for this came from cloud native requirements (whatever that means). The idea behind is to simplify the scenario where a REST endpoint wants to persist data, or part of it, it receives to the DB. In most cases, I believe, data are sent in json format, so this can help avoiding the need to implement unnecessary converter or logic in the REST MessageReader/Writer for getting data from the endpoint through the persistence entity to the DB. JSON format as such is also quite popular and with DBs having native support for JSON data type, it may make sense to support it at the spec level. from the other point of view - simplicity of handling this via converter and the fact that JsonValue is not built-in type in JDK - are valid concerns and points which were raised during the conversation I've had with Linda before EE 8 (many years ago). At that time, there was an intent to include jsonp in the JDK, so the second concern would become invalid, but that actually has never happened and is not going to happen (at least not anytime in the foreseeable future). |
FWIW in Hibernate 6 we have the option to annotate a
and if the underlying database supports a JSON type, it would use that DDL type, otherwise use some We don't yet support JSONP types, but I is that really the natural representation one would use in an entity? I would actually expect that one either uses a proper model type If so that is the case, then maybe we just need to agree on such a |
FWIW With Ebean we have
For myself, I personally feel it is not ideal to map to Note that Ebean supports mapping JSON content to Jackson's Given those options for mapping JSON to types that keep the model independent of the applications JSON binding library should people use a Jakarta |
I did some trials and errors with EclipseLink and here are my findings: Mapping and Conversion:
There won't be problem to store and retrieve JSON data. Adding it to mapping just removed the need of any annotations on Entity attribute so it may help API users a bit. JSON in WHERE conditions:
vs.
-> Imagine you have 1st one in DB and second comes to some .setParameter("value", value) of the query statement. VARCHAR condition won't work properly. Another story is to introduce JSON path in JPQL and Criteria conditions. That would enhance JSON support a lot, but it won't work without native support on DB side. Using VARCHAR for such a thing is not possible. Also every DB would require it's own native SQL syntax for such a thing (look at PostgreSQL vs MySQL as an example). My conclusion is, that we can add mapping for jakarta.json.JsonValue to the spec. But there will be some limitations now. People shall consider it as some kind of CLOB/TEXT without using it in search conditions except DBs where native JSON type works fine. Edit:
|
This is EE4J JPA API specification and jakarta.json.JsonValue is common ancestor of all JSON values in EE4J JSON API specification. That's why |
Yup, cool.
I was just trying to say that in my experience it has been less useful in practical terms to map a type like JsonValue than it maybe first seems. Less useful than mapping
I am thinking you meant
There is a SQL/JSON standard for DB JSON path functions and expressions and improving support for that standard amongst the databases. So optimistically there is becoming less need for DB specific syntax but today internally ORM's need to "translate" those expressions to DB specific syntax in some cases.
As far as I know this is MySql specific behavior and I don't recall hitting this behavior with any other database. As I see it, this can be problematic that the content coming back is somewhat unpredictable (e.g. an app wanting to do MD5 on the json content for diff change/detection etc).
Just noting that wrt Postgres, I suspect there is a school of thought that |
I'm inclined to agree with this. As a general principle, I prefer to keep entity classes free of dependencies to these sort of "technical" library-defined types. Now, I don't think one needs to be religious about that, but as a rule of thumb it's pretty good. |
MySQL problem was just my stupidity. Postgres JSONB vs. JSON I do agree that JsonValue vs. Map<String, Object> or MyJsonPayloadRepresentation Keep in mind, that JSON is document format and it's structure is a tree. Map or some POJO or any other kind of Java Collection is just flat structure that allows to store just one level of the tree. There are use cases where flat structure is fine and maybe it makes sense to allow mapping of Java Map/List/POJO to DB JSON too. |
No in that
Noting that |
We should consider adding
jakarta.json.JsonValue
as a persistent capable type. As JSON is nothing more than formatted text, it easily maps to VARCHAR. This reduces the necessity for application developers to implement a converter to do this bit of work.Example Usecase (borrowed from eclipse-ee4j/eclipselink#1391):
The text was updated successfully, but these errors were encountered: