Type json_array is not consistent with NULL values

[TerjeBr]

Fields of type json_array are created as "TEXT NOT NULL", because they are not nullable by default.

If you have an existing table, and you add this field, all existing records will get an empty string as the value of the field and not NULL.
If you try to store a NULL value in that field, the database will convert it to an empty string.

The "convertToPHPValue" method for that type only checks for NULL when it converts NULL to array(), it does not check for an empty string.

The test must be changed to also test for the empty string, or else it behaves differently for when the column is set as nullable or not. And for the most common case, when the default vaule of letting the column be not nullable is used, this "feature" of converting blank values to empty arrays is not working.

[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-1038

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

[guilhermeblanco]

Hi,

This was reported a couple of times already, but here is the explanation why it cannot be accepted as is:

Many projects consume this data type and already deal currently with the array() case. Modifying the behavior now is a potential BC break and cannot be done for 2.X series.
My recommendation is that it comes as a new type, so we can incorporate as part of Doctrine and also properly deal with null values. I'd also ask for commenting that in the class, so more people don't get frustrated and submit same PR again.

[]s,

[TerjeBr]

This is different from #538 and #655
This for when you want to keep the "feature" of transforming null to empty array.
I am saying that you must do so for empty string too. Or else it behaves diffrently when the table column is nullable or not. Since the default is that a column is not nullable, when a table is altered to include a column of this type, all existing records will get a '' value in that column instead of a NULL value. So for its intended usage this feature is broken.

[Ocramius]

This particular change makes sense to me, and is consistent with current behavior.

It needs additional test cases and/or data-provider cases before merge though.

[deeky666]

agree. We need the following test in Doctrine\Tests\DBAL\Types\JsonArrayTest:

public function testJsonEmptyStringConvertsToPHPValue()
{
    $this->assertSame(array(), $this->type->convertToPHPValue('', $this->platform));
}

[TerjeBr]

Ok, I have added a unit test.

Any more tests that needs to be added?

[TerjeBr]

Should this be "back ported" to earlier versions of dbal?

[Ocramius]

@TerjeBr it may be backported, yes

[TerjeBr]

What is the earliest version it should apply to?

[Ocramius]

Likely 2.4.4. Right now, it only causes a warning because of json_decode() being unable to convert the value, right?

[TerjeBr]

What do you mean? what warning?

[TerjeBr]

I just figured that JsonArray was inluded in v2.5.0-BETA2 with commit 25e695f

[TerjeBr]

If you are talking about the Travis CI build failing, that failure is unrelated to this change.

[Ocramius]

What do you mean? what warning?

What was the behavior of this DBAL type with an empty string before the patch?

[TerjeBr]

Before the patch, an empty string in the DB was transformed into a NULL.

~$ php -r 'error_reporting(E_ALL|E_STRICT); var_dump(json_decode(""));'
NULL

No warnings generated.