DDC 3863 - Invalid JsonArrayType [master]

[Taluu]

Master version of the DDC-3863's fix. Basically, instead of returning an empty array() value if the value is null (or empty), we're returning another null value instead.

If you think this is a valid addition (after some corrections if needed), should I open other PRs for the other supported branches (2.3, 2.4, 2.5) ? But this looks like a BC break, even though the original behaviour is strange compared to the ArrayType's behavior...

Thanks

[doctrinebot]

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DBAL-1276

We use Jira to track the state of pull requests and the versions they got
included in.

[stof]

This is a BC break, so merging it in 2.x branches is totally out of question (BC breaks cannot happen in minor versions).

and DBAL 3.0 is not yet planned. The master branch is expected to become DBAL 2.6. So -1 on that for now.

[Taluu]

Thought so, but still, I think this is an erratic behavior that prevents the usage of this type in the case when the field can be referred by many columns.

For now, I've got to extend this type and override this mechanism with something similar to the fix I'm proposing here...

[stof]

what do you mean by referred by many columns ?

[Taluu]

e.g : I have a field declared in an joined inheritance in some subentities (not all of them). Here is (a little bit stripped...) the request that is made :

SELECT m0_.id AS id_0, 
    i1_.mimetype AS mimetype_9, i1_.thumbnails AS thumbnails_image, 
    d2_.mimetype AS mimetype_15, d2_.thumbnails AS thumbnails_docs
FROM Media m0_ 
    LEFT JOIN Image i1_ ON m0_.id = i1_.id 
    LEFT JOIN Document d2_ ON m0_.id = d2_.id;

I renamed the alias of the "thumbnail" for this to be clearer.

The result is as following :

As you can see, the first row is a Document and the other one is a Image. They both have common fields declared on their own, but these entities varies on some points, but this is not the subject here. They both have a json_array thumbnails. But as you can see, on the returned columns, they have two thumbnails_x fields. The JsonArray returns an empty array for a null value... And then, Doctrine ORM's AbstractHydrator stores the value into a sort of cache and returns that cache if the value is not null which is the case here.

So basically, the Document never has the right value, it always displays and stores an empty array... which is wrong IMO. The ArrayType, which is kind of the model of the JsonArrayType returns a null value if a null value is given, and the same goes for the other types (date, string, ... you name it). Hence the "erratic behavior"

[stof]

@Taluu I agree this was a bad architectural choice at the beginning (and even a bad name as JsonArrayType could work for any JSON values, including strings and numbers).

But changing it is not possible in 2.x for BC reasons, and the work on 3.0 is not planned yet so this cannot be merged.

What you can do in the meantime is define your own DBAL type mapping values to a json field without any special case, and use that instead of the core type.

[Taluu]

Yes that is what I have done, which does the trick. But I still thought this could be up for debate if it is really necessary to keep the BC, as I think this is more a bug than something to break.

[stof]

@Taluu the issue with this is that it is the worse kind of BC break: the kind breaking things silently, because people won't get an obvious error if they have code relying on the current behavior of the type. and the way to fix it is to run a DB migration, not to change some code (to transform empty arrays to empty arrays in JSON).

What we could do is to introduce a new json type in DBAL 2.6 and deprecate the old one. This way, switching to the better behavior would be opt-in rather than being mandatory

[Taluu]

Closing in favor of making a new type for 2.6