Skip to content
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

JSON type without encoding/decoding #2723

Closed
Tobion opened this issue May 12, 2017 · 6 comments
Closed

JSON type without encoding/decoding #2723

Tobion opened this issue May 12, 2017 · 6 comments

Comments

@Tobion
Copy link
Contributor

Tobion commented May 12, 2017

The JSON datatype (see #2653) automatically encodes/decodes the JSON datastructure. We need a JSON datatype that does not do this for several reasons:

  • It's easier to switch existing text columns to json as we can keep using strings
  • Sometimes you get JSON and just want to save it. It doesn't make sense to decode it, just so that doctrine can encode it again when inserting.
  • More flexibility by being able to use any different serializations, e.g. using JMS serializer or json_decode($data, false) instead of json_decode($data, true) which is hard-coded into the JSON type.

So I'd propose to rename the new json type to json_decoded and introduce a new json type that uses a plain json string internally without transformation.
I chose json_decoded because you effectively work with a decoded json structure all the time (similar to old json_array). json_decoded would be more correct because it does not need to be an array. It can also be null or a string or int etc. decoded from json.

@Tobion
Copy link
Contributor Author

Tobion commented May 12, 2017

An alternative name for json_decoded would be json_data

@jvasseur
Copy link

jvasseur commented May 12, 2017

I would prefer to keep the current type named json. Maybe a non-decoded type could be named json_string.

@Ocramius
Copy link
Member

Ocramius commented May 16, 2017 via email

@Tobion
Copy link
Contributor Author

Tobion commented May 16, 2017

json_string as name does not make sense to me because it's not clear if it's a decoded string from json or a raw json string. json_array was also a decoded array.

I don't see why json_string would be in core

I gave three reasons above. Also DBAL is just an abstraction over DB access. So it's more logical to have a plain JSON type than a magic JSON column that decodes and encodes magically. Otherwise you could add alot more types with different serialization forms to the core.

@jvasseur
Copy link

jvasseur commented May 16, 2017

json_string seems logical to me, it's a string containing json. But I agree with @Ocramius not sure if it should be in core.

I'm against renaming the current json type for a few reasons:

  • it feels like the most common use case is to want the decoded json data rather than a non-decoded json string (to store free form data).
  • the PR introducing the type was merge nearly 2 year ago, there is probably some project already using it (even if it never was in a released version).
  • some package exists that introduce similar types with the name json (one of them is https://github.com/sonata-project/sonata-doctrine-extensions)
  • it is designed as a replacement for json_array except it isn't made only for storing arrays so json seems logical.
  • doctrine types always return the decoded data (ie: date types return DateTimeInterface instances instead of the date string).

@Tobion Tobion closed this as completed Apr 27, 2018
@github-actions
Copy link

github-actions bot commented Aug 2, 2022

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants