expression: fix unexpect invalid json text error when query with json_extract

[TszKitLo40]

What problem does this PR solve?

Issue Number: close #11883

Problem Summary:

What is changed and how it works?

What's Changed:
fix unexpect invalid json text error when query with json_extract
How it Works:
When a string can't be casted to json, use json.CreateBinary

Related changes

Check List

Tests

• Integration test

Release note

• No release note

[ti-challenge-bot]

There is no reward for this challenge pull request, so you can request a reward from @XuHuaiyu.

Details

Tip : About reward you can refs to reward-command.

Warning: None

[ichn-hu]

Also please add some test cases.

[ichn-hu]

Probabily we don't need use encoding/json to check the validity, just ParseBInaryFromString and if err is not nil we will fall back to CreateBinary?

[ichn-hu]

@TszKitLo40 Also, would you please check if your PR could fix #13710 ?

[TszKitLo40]

@TszKitLo40 Also, would you please check if your PR could fix #13710 ?

And I aslo have test #13710, it uses the same logic json.ParseBinaryFromString to parse a string to json, but it is used in the vecEvalJSON of *builtinCastStringAsJSONSig, we can fix some case in #13710.

[TszKitLo40]

Also please add some test cases.

I am not sure whether it is the best choice to fix this issue, because it make cause some test case fail. Do you think we can should cast a common string to json? And I don't know why should we check HasParseToJSONFlag here. And It seems that the string must be casted to JSON @ichn-hu

[ichn-hu]

I am not sure as well, but I do have concerns about implicit type conversion, perhaps we need to investigate more on MySQLs behavior and to find out if it really a valid fix.

Examples like

• compare with valid json
• compare with invalid json
• compare with mixed valid and invalid json

[TszKitLo40]

I am not sure as well, but I do have concerns about implicit type conversion, perhaps we need to investigate more on MySQLs behavior and to find out if it really a valid fix.

Examples like

• compare with valid json
• compare with invalid json
• compare with mixed valid and invalid json

But in this case, it seems that we must cast a string to json. I don't know why do we need to check HasParseToJSONFlag here.

[ichn-hu]

I am not sure as well, but I do have concerns about implicit type conversion, perhaps we need to investigate more on MySQLs behavior and to find out if it really a valid fix.
Examples like

• compare with valid json
• compare with invalid json
• compare with mixed valid and invalid json

But in this case, it seems that we must cast a string to json. I don't know why do we need to check HasParseToJSONFlag here.

I think it is not always the case, if there is an invalid json string in the argument lists, MySQL would not convert it into json.

create table t1(f1 json);
insert into t1(f1) values ('"asd"'),('"asdf"'),('"asasas"');
select f1 from t1 where json_extract(f1,"$") in ("asd","asasas","{asdf");
+----------+
| f1       |
+----------+
| "asd"    |
| "asasas" |
+----------+

[TszKitLo40]

I am not sure as well, but I do have concerns about implicit type conversion, perhaps we need to investigate more on MySQLs behavior and to find out if it really a valid fix.
Examples like

• compare with valid json
• compare with invalid json
• compare with mixed valid and invalid json

But in this case, it seems that we must cast a string to json. I don't know why do we need to check HasParseToJSONFlag here.

I think it is not always the case, if there is an invalid json string in the argument lists, MySQL would not convert it into json.

create table t1(f1 json);
insert into t1(f1) values ('"asd"'),('"asdf"'),('"asasas"');
select f1 from t1 where json_extract(f1,"$") in ("asd","asasas","{asdf");
+----------+
| f1       |
+----------+
| "asd"    |
| "asasas" |
+----------+

It seems that it is hard to know whether we should convert the string to json.

[ichn-hu]

Note that

drop table t1;
create table t1(f1 json);
insert into t1 values ('{"a": "asdf"}');
select json_extract(f1, '$') from t1;
select f1 from t1 where json_extract(f1, '$') in ('{"a": "asdf"}');

will produce 0 row in MySQL, and only

select f1 from t1 where json_extract(f1, '$') in (cast('{"a": "asdf"}' as json));

will do the work.

So I think the fix is not to convert string to json, instead, we can

• if both are json type, then compare them directly
• if we want to compare json with string, convert json to string if tpCode is string, otherwise return false directly.

[TszKitLo40]

Note that

drop table t1;
create table t1(f1 json);
insert into t1 values ('{"a": "asdf"}');
select json_extract(f1, '$') from t1;
select f1 from t1 where json_extract(f1, '$') in ('{"a": "asdf"}');

will produce 0 row in MySQL, and only

select f1 from t1 where json_extract(f1, '$') in (cast('{"a": "asdf"}' as json));

will do the work.

So I think the fix is not to convert string to json, instead, we can

• if both are json type, then compare them directly
• if we want to compare json with string, convert json to string if tpCode is string, otherwise return false directly.

It seems resonable, but if we do that, it seems that we need to change the execution planner?

[ichn-hu]

@TszKitLo40 Not necessarily, we could do it during runtime, just around the lines you just changed.

[TszKitLo40]

@TszKitLo40 Not necessarily, we could do it during runtime, just around the lines you just changed.

But in the lines I changed, we don't know the type of value to be compared, we need to know it so that we can decide whether we should cast it to json or keep it as string.

[ichn-hu]

@TszKitLo40 Not necessarily, we could do it during runtime, just around the lines you just changed.

But in the lines I changed, we don't know the type of value to be compared, we need to know it so that we can decide whether we should cast it to json or keep it as string.

Yeah, I found that it need to change planner. If you don't mind, I would like to take over this issue (I've almost figured out the fix), but you are welcomed to look at other issues on #20804

[TszKitLo40]

@TszKitLo40 Not necessarily, we could do it during runtime, just around the lines you just changed.

But in the lines I changed, we don't know the type of value to be compared, we need to know it so that we can decide whether we should cast it to json or keep it as string.

Yeah, I found that it need to change planner. If you don't mind, I would like to take over this issue (I've almost figured out the fix), but you are welcomed to look at other issues on #20804

OK, feel free to take over this issue. And I will close this PR.