Replace JSON_EXTRACT utilisation with JSON_VALUE

[Angelinsky7]

Steps to reproduce

Run the custom project to see it crashing PomeloBugJsonExtract.zip

The issue

In some places, in the code, JSON_EXTRACT is used for getting value of a JSON field when using projection or any other mean of directly getting data. But when the data is null, JSON_EXTRACT does return the string value "null" instead of the mysql null value, as intended in the spec : https://bugs.mysql.com/bug.php?id=85755 Sadly if you have a Nullable<Datetime> later in ef core when trying to convert it will use the string representation instead of the correct null value and crash with

---> System.FormatException: Couldn't interpret value as a valid DateTime: null
         at MySqlConnector.Core.Row.GetDateTime(Int32 ordinal) in /_/src/MySqlConnector/Core/Row.cs:line 336
         at MySqlConnector.MySqlDataReader.GetDateTime(Int32 ordinal) in /_/src/MySqlConnector/MySqlDataReader.cs:line 264
         at lambda_method186(Closure, QueryContext, DbDataReader, ResultContext, SingleQueryResultCoordinator)
         --- End of inner exception stack trace ---
         at lambda_method186(Closure, QueryContext, DbDataReader, ResultContext, SingleQueryResultCoordinator)
         at Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable`1.Enumerator.MoveNext()

be aware that the null indication here is not a real null value but instead a string "null" in

https://github.com/mysql-net/MySqlConnector/blob/1d508bcfd74f221e71a892bbb3086a545f1a5451/src/MySqlConnector/Core/Row.cs#L326-L342

The naive solution

If we replace all call with JSON_VALUE we get a correct null value instead of a string and "everything" seems to work correctly

Further technical details

MySQL version: 8.3.0-1.el8 Operating system: Window11 Pomelo.EntityFrameworkCore.MySql version: 8.0.2

[Angelinsky7]

As a really naive approach, i replaced all JSON_EXTRACT to JSON_VALUEand everything is working fine.

Test results: without the changes:

Réussi!  - échec :     0, réussite :    62, ignorée(s) :     0, total :    62, durée : 327 ms - Pomelo.EntityFrameworkCore.MySql.Tests.dll (net8.0)
Échoué!  - échec :     5, réussite :    32, ignorée(s) :     0, total :    37, durée : 1 s - Pomelo.EntityFrameworkCore.MySql.IntegrationTests.dll (net8.0)
Échoué!  - échec : 11437, réussite : 17625, ignorée(s) :   362, total : 29424, durée : 1 m 1 s - Pomelo.EntityFrameworkCore.MySql.FunctionalTests.dll (net8.0)

with my changes:

Réussi!  - échec :     0, réussite :    62, ignorée(s) :     0, total :    62, durée : 386 ms - Pomelo.EntityFrameworkCore.MySql.Tests.dll (net8.0)
Échoué!  - échec :     5, réussite :    32, ignorée(s) :     0, total :    37, durée : 1 s - Pomelo.EntityFrameworkCore.MySql.IntegrationTests.dll (net8.0)
Échoué!  - échec :  7961, réussite : 21096, ignorée(s) :   367, total : 29424, durée : 1 m 11 s - Pomelo.EntityFrameworkCore.MySql.FunctionalTests.dll (net8.0)

i think I've got failed test because of my mysql config (most of my failed test broke because of : System.Security.Authentication.AuthenticationException : Authentication failed because the remote party sent a TLS alert: 'IllegalParameter')...

but i would gladly make a PR... if it's something doable (i just need to know witch branch to target)

[lauxjpn]

@Angelinsky7 Thanks for making us aware of this issue.

We have implemented a workaround/fix for this issue in #1899, which is likely to be backported to 8.0.x.

[Angelinsky7]

@lauxjpn sorry for the noise, i should have read better #1899 ....

Do you think it would be possible to target 8.0.3 for this ? I've got some feature locked because of this. (i would hope not to wait month to have it) I would be very happy to do it ! Tell me which branch to target and i can try to backport the code of main on it (if it's ok for you)

Thanks !

[lauxjpn]

Do you think it would be possible to target 8.0.3 for this ?

@Angelinsky7 That is the plan.

I would be very happy to do it ! Tell me which branch to target and i can try to backport the code of main on it (if it's ok for you)

The 8.0.x branch is 8.0-maint (see Dependencies).

This issue is actually a bit more involved than we initially anticipated, because of the differences in how JSON_EXTRACT() and JSON_VALUE() are implemented between MySQL and MariaDB. The fix for 8.0.x (and probably 6.0.x) also should be backwards compatible.

While we are at it, we are going to improve the EF.Functions.JsonExtract() extension method and maybe introduce a EF.Functions.JsonValue() extension method as well.

So the complexities here are the reason for why we haven't merged #1899 yet and are still working on it.

It is probably best in this case here that I complete the works on the the fixes and improvements myself, since I am already in the middle of it. Thanks!

[Angelinsky7]

@lauxjpn tell me if i can do something to help you, i would be happy to do my part !

[Angelinsky7]

hello @lauxjpn, i hope your well ! Do you have any news for this to happen in the near future ? i really would love to be able to fully use json object in 8.0.x Of course if i can do something do help you, it would be a pleasure for me.

[Angelinsky7]

Hello @lauxjpn I'm sorry to insist but i keep hitting this bug again and again. I don't understand how anybody else don't have the same issue as me: Every date stored in json are quoted because of using JSON_EXTRACT instead of JSON_VALUE (or JSON_UNQUOTE(JSON_EXTRAC ) and i keep getting the mysql connector telling me that my dates are not in the correct format (which is true, because they have quote).... Could we at least make a patch to be able to decide if we want to use JSON_EXTRACT vs JSON_VALUE for all transformations ?! It's quick and dirty (and it's working) but at least i could continue to work. Now i'm at a stage where i think i should completely drop json from my databases because of this. Thanks for you answer and i hope that we'll be able to solve this.

[Angelinsky7]

So i made this branch: https://github.com/Angelinsky7/Pomelo.EntityFrameworkCore.MySql/tree/8.0-maint-json-value-%231897 just to show that by changing JSON_EXTRACT TO JSON_VALUE with the latest mariadb database it's working correctly.

We should add an option to users like that they could choose (for the whole connection, like _option: IMySqlOptions) We could add something like: bool UseJsonValue {get;} and set it in the connection definition.

If @lauxjpn you are OK with this approach i can do it.... (and when you'll finish refactoring all this code for 9.x.x and backport it you'll remove this)